[5.6] dependency resolution performance (#4007) #4012
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
motivation: improve dependency resolution performance changes: * add informational diagnostics on manifest cache hit/miss * disable expensive optimization in pubgrub resolver that has no functional value given changes from swiftlang#3985 * adjust resolver workspace delgate to be less noisy given the difference in resolution callbacks
tomerd
added
ready
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
motivation: clean output to user changes: remove debug leftover from swiftlang#4007
|
@swift-ci please test |
abertelrud
approved these changes
Jan 13, 2022
airspeedswift
approved these changes
Jan 13, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
5.6 cherry pick of #4007
Explanation:
Fixed dependency resolution issues causing failure on edge cases such with pre-release versions. as a result of that fix, the bounds computation done as part of the resolution is no longer requires which removes a significant performance bottleneck. this PR removes the bounds computation yielding 3x performance improvement on medium to large projects.
The bounds computation was originally added in aee02eb in an attempt to improve diagnostics when graph fail to be resolve. #3985 changes how the underlying data for these diagnostics is captured which yield even better diagnostics. as such this PR is purely about performance gain by not running that expensive calculation any more since it no longer has any functional value.
Scope of Issue:
This impacts the performance of package dependency resolution, triggered by
swift package updateor when loading the package in Xcode.Reason for Nominating to 5.6 :
This yield significant improvement in performance.
Risk:
Low risk; The risky change was #3985 which is already part of 5.6. This PR is a followup realization that because of #3985 we do not need to run that expensive bounds computation at all.
Reviewed By: @neonichu @abertelrud
Automated Testing: Unit test, package compatibility test suite
Dependencies: None
Impact on CI: None
How to Verify: Resolve [non-trivial] package dependencies and make sure the resolved versions (eg content of Package.resolved) are identical but that the performance is better