Skip to content

[5.6] Load manifests asynchronously [cherry pick] #3940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Dec 23, 2021

Conversation

pcbeard
Copy link
Contributor

@pcbeard pcbeard commented Dec 13, 2021

This is a cherry pick of PR #3894 on to the release/5.6 branch.

DelayableAction represents an action on a target that can be deferred
until a more appropriate time. In this case, we use defer to cleanup
objects in the case of early return error handling, but we need to
delay this action when passing the target for use into an async
completion block.
The manifest parser uses SourceControl.GitShellHelper synchronously,
which would cause a hang if using the default queue for completions.
This is required for unit tests to pass.
@pcbeard pcbeard changed the title Async manifest loading5.6 Load manifests asynchronously [cherry pick] Dec 13, 2021
@tomerd
Copy link
Contributor

tomerd commented Dec 13, 2021

@swift-ci test

@tomerd tomerd added the 5.6 label Dec 13, 2021
@tomerd tomerd changed the title Load manifests asynchronously [cherry pick] [5.6] Load manifests asynchronously [cherry pick] Dec 13, 2021
@tomerd
Copy link
Contributor

tomerd commented Dec 13, 2021

@swift-ci please test macOS

motivation: with manfiest loading async, the concurrent loading tests introduce significant contention on cpu since the underlying queue in unbounded

changes: use semaphore to limit the concurrency to max active cpus
@tomerd
Copy link
Contributor

tomerd commented Dec 22, 2021

@swift-ci test

@tomerd tomerd merged commit f62e608 into swiftlang:release/5.6 Dec 23, 2021
@pcbeard pcbeard deleted the AsyncManifestLoading5.6 branch December 23, 2021 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants