Fix PackageGraph traversal performance regressions
#7248
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
MaxDesiatov
added
bug
performance
neonichu
approved these changes
Jan 12, 2024
|
@swift-ci test |
|
The actual fix is also verified by |
|
Currently blocked due to #7249 |
This restores the performance of traversal of deep package graphs previously available when `ResolvedTarget`, `ResolvedProduct`, and `ResolvedPackage` were value types. Now instead of recursively applying `Hashable` conformance to all properties in graph's node, we only hash and compare for equality their identities. The main benefit of this approach is that build triple of `ResolvedProduct` and `ResolvedTarget` is now a part of their identity. Thus we can still use value types, while the same `Target` from `PackageModel` instantiated as two different `ResolvedTarget`s with different `BuildTriples` will have different identities. Resolves rdar://120861833 Resolves rdar://120711211
MaxDesiatov
force-pushed
the
maxd/nonshable-packagegraph
branch
from
e9ede20 to
98fd47f
Compare
|
@swift-ci test |
|
@swift-ci test |
MaxDesiatov
added a commit
to swiftlang/sourcekit-lsp
that referenced
this pull request
Jan 17, 2024
`ResolvedTarget` is no longer `Hashable` in SwiftPM, SourceKit-LSP should use `ResolvedTarget.ID` instead. Depends on swiftlang/swift-package-manager#7248.
|
@swift-ci test windows |
1 similar comment
|
@swift-ci test windows |
MaxDesiatov
added a commit
to swiftlang/sourcekit-lsp
that referenced
this pull request
Jan 17, 2024
`ResolvedTarget` is no longer `Hashable` in SwiftPM, SourceKit-LSP should use `ResolvedTarget.ID` instead. Depends on swiftlang/swift-package-manager#7248.
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.
Depends on #7258
This restores performance of traversal of deep package graphs previously available when
ResolvedTarget,ResolvedProduct, andResolvedPackagewere reference types. Now instead of recursively applyingHashableandEquatableconformances to all properties in graph's node, we only hash and compare for equality their identities defined byIdentifiable.Uses of
Seton these types are now replaced with the newIdentifiableSettype, which uses a dictionary of[Element.ID: Element]type as its storage.The main benefit of this approach is that build triple of
ResolvedProductandResolvedTargetis now a part of their identity. Thus we can still use value types, while the sameTargetfromPackageModelinstantiated as two differentResolvedTargets with differentBuildTripleswill have different identities.Resolves rdar://120861833
Resolves rdar://120711211