More tree-merge improvements - #2843
Draft
Sebastian Thiel (Byron) wants to merge 4 commits into
Draft
Conversation
Sebastian Thiel (Byron)
force-pushed
the
deleted-file-added-dir
branch
4 times, most recently
from
July 31, 2026 18:10
c2ae76a to
330d01d
Compare
Add complementary benchmark, Cartesian, and fuzz harnesses around gix_merge::tree() so the scheduling state machine can be measured and checked without commit traversal or result-tree serialization obscuring failures. The Criterion benchmark builds a shallow in-memory repository containing 34 independent additions, deletions, modifications, mode and type changes, file/directory conflicts, and rename interactions. It warms and reuses merge platforms, uses exact-only rename detection and path-unique blobs, reports structural cases as throughput, and asserts that at least twelve conflicts remain so the fixture cannot silently become trivial. Exercise every Cartesian operation pair in both directions with default and ancestor conflict handling, fail-fast behavior, and rename detection disabled. Check side-order symmetry, payload retention, forced-resolution provenance, the public early-exit flag, and independently unambiguous merge results, and record the resulting status quo. Add a bounded structural libFuzzer target covering valid base and side trees, all supported entry modes, file/directory replacements, and file or subtree renames in both side orderings. Keep blob work bounded with NUL-containing resources, a one-byte large-file threshold, identity-only rename tracking, and the Histogram text algorithm, then write each editor result back to the in-memory object database. Treat only the documented missing binary ancestor during ancestor resolution as an expected generated outcome; all other errors remain fuzz failures. Tests: - cargo bench -p gix-merge --bench tree --all-features -- --test - cargo test -p gix-merge --test merge tree::cartesian - cargo check --manifest-path gix-merge/fuzz/Cargo.toml Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Sebastian Thiel (Byron)
force-pushed
the
deleted-file-added-dir
branch
2 times, most recently
from
August 6, 2026 10:29
4eb97df to
a090e25
Compare
<!-- agent --> The tree-merge implementation previously combined side-diff collection, path matching, scheduling, and the complete conflict-resolution matrix in a single function. It also duplicated change collection for both sides and represented pair outcomes with independent boolean flags. Keep the public tree() entry point as a small facade and move the merge engine into focused private modules. Collect each ancestor-to-side diff through one helper that constructs a SideState containing the flat change list and its matching tree together. Isolate path and rename candidate matching, including identical-change suppression, from classification into pairs that the resolution matrix understands. Represent the result of handling each paired change explicitly as a ChangeDisposition. This preserves the important distinction between a change that was merely processed and one whose effect is present in the editor, without duplicating the final state transition in the scheduler. Replace the type-specific side-picking helpers with generic pick() and pick_mut() helpers as well. These boundaries make the state machine easier to review and reduce the chance that a future rename or forced-resolution fix accidentally changes collection, matching, and application at once. They also remove duplicated setup while keeping the exhaustive resolution match in one place, where its symmetry remains visible. The public API and all recorded merge results remain unchanged. Fixes and Improvements ---------------------- Tree merging combines a flat change schedule with per-side path indexes. Valid Git operations can therefore arrive in different orders or expose structural relationships before the leaf changes that ultimately apply them. Several resolver branches treated those relationships as physical occupancy or as content changes for the same identity, leading to hangs, assertions, duplicate entries, lost siblings, or merge results that depended on diff and side order. Separate unique-path occupancy from PassedRewrittenDirectory scheduling so a side-qualified name can terminate below directory rewrites. Prune empty path nodes back to the root, and allow a deferred rewrite to insert only its new destination because its source is already indexed. Resolve the structural cases at their actual identity boundaries: - handle an added file blocking an added directory before mode-specific add/add resolution and defer early descendants until their parent deletion runs; - keep explicit file renames ahead of inferred directory renames, and keep directory replacements at their explicit sources; - treat file replacements of incompatible non-blob ancestors as additions with an empty compatible merge base; - pair shared deletions before descendants and allow file renames into paths vacated by directory renames; - preserve unrelated nested or overlapping rename destinations by keeping the directory in place and moving only the blocking file; - reject incompatible same-destination rewrites before blob merging, while collapsing identical rewrites to one clean shared destination; and - defer file-to-directory children until the parent rename/delete decision is made exactly once. Forced Ancestor and Ours resolution continues to apply only the selected side. Git-backed baselines cover both directions, forced policies, modes, symlinks, gitlinks, nested directories, and documented index-only deviations. The resulting suite contains 155 directional baseline cases, and the Cartesian model reaches 210/210 Git/gix agreement for trees and path/mode results. More Hardening -------------- Deferred tree changes may be reconsidered after another conflict has already consumed or pruned the same path-tree node. This is valid when rename detection has ambiguous identical sources, when structural conflicts overlap, or when a change follows a detected directory rename. The editor and conflict records still contain the required state, but strict bookkeeping removals and older same-path assertions turned these schedules into debug panics, hangs, or side-order-dependent duplicate content. Make cleanup idempotent wherever absence is already the required end state: add/add type conflicts, same-source rewrites, blocking conflict destinations, delete/rewrite sources, and changes deferred through directory renames. Accept cross-path structural matches from ambiguous rewrite candidates and let the existing conservative unknown-conflict fallback handle them. Preserve each rewrite input mode when blob content is identical so executable mode changes remain visible to the merge. Make unique-path selection respect childless tracked directories and qualify the first blocking file component, which guarantees termination instead of varying an ineffective descendant suffix forever. Finally, when a deferred addition is relocated to a unique conflict path, remove its temporary original path from the side index before marking it processed so a later descendant cannot relocate the same content a second time. The minimized and accumulated fuzz inputs now complete without failure. Git-backed regressions cover ambiguous sources, consumed nodes, repeated rename/delete candidates, mode-only rewrite collisions, unique paths below files, and nested rename destinations in both side orderings. The final tree baseline contains 165 directional cases with 130 intentionally skipped forced resolution checks, and reversing the nested relocation case retains exactly `a/a/a` and `a~A` without inventing `a~A_0`. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 <codex@openai.com>
The earlier focused tree-merge benchmark reports about 500k synthetic cases per second, but it does not show how the merge scales with a production-sized tree or with thousands of changes spread throughout it. Add a deterministic in-memory fixture shaped after Linux commit 8ba098e6b6ff0db8edf28528d1552be261af30d4: 94,852 files in 6,202 trees, with 24 top-level directories and a maximum depth of 11. Fixture creation stays outside the measured loop so Criterion isolates gix_merge::tree() without requiring a large checked-in repository fixture. Measure both a single conflicting edit and a merge with 10,000 logical side changes. The large case spreads 4,500 modifications and 500 exact renames across each side and retains one conflict. Run both workloads with and without rename tracking, and validate their shape and conflict count before sampling them. Criterion throughput counts logical side changes here, whereas the earlier benchmark counted conceptual structural cases. A representative local run measured about 854k changes/s without rename tracking and 808k changes/s with it, so those figures are useful for this workload but are not identical units to the earlier 500k cases/s result.
Sebastian Thiel (Byron)
force-pushed
the
deleted-file-added-dir
branch
from
August 6, 2026 12:10
a090e25 to
9b91c76
Compare
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.
Add more explicit test coverage, and more coverage of auto-resolution modes, and benchmarks.
Tasks