core: use more efficient methods for finding ancestors #35873
Merged
Conversation
This file contains 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
Add ancestor equivalents of FirstDescendantsWith and MatchDescendant
In configurations with long chains of dependencies, we spend a lot of time recording redundant dependency edges, and writing all those dependencies in the state. Rather than record every possible dependency edge, only record the first managed resource in any lineage, such that the resulting graph is topologically equivalent having removed many transitive edges. For some configurations this can save considerable time and space when handling the state.
Use the more efficient FirstAncestorsWith when connecting dependencies cause by `depends_on`.
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Add ancestor equivalents of FirstDescendantsWith and MatchDescendant.
In configurations with long chains of dependencies, we spend a lot of time recording redundant dependency edges, and writing all those dependencies in the state.
Rather than record every possible dependency edge, we can use the new methods above to only record the first managed resource in any lineage, such that the resulting graph is topologically equivalent having removed many transitive edges. For some configurations this can save considerable time and space when handling the state.
While this can remove the bulk of transitive dependencies in a lot of cases, this does not reduce the subgraph of direct dependencies. Take for example a resource
X, and a graph with edgesX->AX->BA->B. We will still currently record bothAandBas dependencies even though the final graph will reduce to justX->A->B. Doing that extra processing is a harder cost-tradeoff to make however, and doesn't seem necessary at this point, whereas this PR is never less efficient than tracking all possible ancestors.