perf(monorepo): reuse the tag index ancestor set instead of walking twice#683
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
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.
run_release_logicwalked HEAD's full ancestry twice, and threw the first walk away.build_head_ancestors(mod.rs:150) andTagIndex::buildrun the byte-identical revwalk, andTagIndexalready exposes the result aspub ancestors. On top of that, all four reads ofinputs.head_ancestors(plan.rs:128/160/183/197) live in theelseofif let (Some(idx), OrphanedTagStrategy::Warn) = (inputs.tag_index, strategy)— andWarnis#[default]whileTagIndex::buildnormally succeeds. So in the default configuration the first walk was dead work: nothing read it.src/query.rs:229already doesSome(&idx.ancestors); this path simply missed the same reuse.Now the index is built first and its ancestor set is passed through; the standalone walk only runs as a fallback when the index is unavailable (which is also the only case where the
elsebranches can fire).Measured — A/B on the real CI fixture
mono-large(200 packages × 10k commits), generated frombenchmarks/fixtures/definitions/mono-large.jsonwith theFixturesgenerator and the sameferrflow.jsonthe benchmark harness writes.check --jobs 1, cache cleared before every run, same binary/machine, n=3:−490 ms, −19%.
This is not a benchmark-only win. The cache key is
head + tags_hash + config_hash(cache.rs:42-56), so every push invalidates it — a one-run-per-push CI job is always cold. Cold numbers are the real numbers.875 tests pass,
clippy -D warningsclean. No observable behaviour change: the ancestor set is identical, only its provenance changes.Closes #682