Stop merge-base walks when one paint side is exhausted - #3003
Draft
Sebastian Thiel (Byron) wants to merge 1 commit into
Draft
Sebastian Thiel (Byron) wants to merge 1 commit into
Sebastian Thiel (Byron) wants to merge 1 commit into
Conversation
…sted The merge-base walk kept following unrelated ancestry while any queued commit remained non-stale, even after one paint color could no longer contribute a merge base. Its termination check also scanned the queue on every iteration. Adapt Git's side-exhaustion optimization from `02d7ba092d`: count non-stale queued commits carrying each color and stop when either count reaches zero in the topologically ordered region. Candidates contribute to both counts so they are processed before termination. Track queue membership to handle flag changes and duplicate inputs without counting a commit twice. Keep the existing stale-only termination for missing, zero, or saturated generations, where date ordering can revive an apparently exhausted color. Continue removing redundant candidates after the paint walk. Use existing fixtures to prove that unrelated ancestry is skipped and cover pending candidates, duplicate inputs, graph reuse, and clock skew. The skipped-ancestry regression fails before this change. Validation: - `GIX_TEST_IGNORE_ARCHIVES=1 cargo test -p gix-revision --all-features` - The same 114 tests with `GIX_TEST_FIXTURE_HASH=sha256` - `cargo fmt --all -- --check` - `cargo clippy -p gix-revision --all-features --all-targets -- -D warnings -A unknown-lints --no-deps` Clippy without `--no-deps` encounters a pre-existing `collapsible_if` warning in `gix-tempfile/src/registry.rs`.
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
2 participants
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.
Tasks
Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Summary
Stop the merge-base walk once either non-stale paint frontier is exhausted and no pending merge-base candidates remain. This avoids following unrelated ancestry after it can no longer contribute a result. Per-color counters also replace the queue scan used to check whether all entries are stale.
Context
This ports the side-exhaustion optimization from Git's commit
02d7ba092d, identified while comparing Git's current merge-base computation withgix.The early exit requires positive, unsaturated generation numbers. Missing, zero, and saturated generations retain the existing stale-or-empty termination because commit-date ordering can revive an apparently exhausted color. Pending candidates count toward both colors, and queue membership tracking keeps counts correct when commits gain flags or inputs are duplicated. The API still returns all merge bases and removes redundant candidates afterward.
Regression tests reuse existing fixtures to verify skipped ancestry, pending candidates, both input orders, duplicate inputs, graph reuse, and clock skew with unreliable generations.
Validation
GIX_TEST_IGNORE_ARCHIVES=1 cargo test -p gix-revision --all-features— 114 tests passed.GIX_TEST_IGNORE_ARCHIVES=1 GIX_TEST_FIXTURE_HASH=sha256 cargo test -p gix-revision --all-features -- --quiet— 114 tests passed.cargo fmt --all -- --check— passed.cargo clippy -p gix-revision --all-features --all-targets -- -D warnings -A unknown-lints --no-deps— passed.Clippy without
--no-depsencounters a pre-existingcollapsible_ifwarning ingix-tempfile/src/registry.rs. The configured removed Clippy lint also emits a warning. No timing benchmark was run; the regression checks that unnecessary ancestry is not loaded.User Prompts