Repository navigation
fix(gix-diff): only compare rewrite destinations to possible sources - #3055
Merged
Sebastian Thiel (Byron) merged 2 commits intoOct 12, 2026
Merged
Conversation
Contributor
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1b2dafe00
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".
`Tracker::emit()` kept all changes in a single list, and for each destination, `find_match()` scanned that whole list for possible sources. That takes time proportional to the number of destinations times the number of all changes, even when there are few or no possible sources. For example, diffing the empty tree against a commit in the `rust-lang/rust` repository, which adds about 63,000 files, took about 17 seconds in [`jj`](https://github.com/jj-vcs/jj) with copy tracking enabled. There are no deletions or modifications, so nothing can be a source. The `limit` doesn't help since it applies to `num_sources * num_destinations`, which is 0. `match_pairs()` now collects the indices of possible sources once, and `find_match()` only looks at those. This applies to the exact-id matching too, which was also quadratic when many destinations had the same id. If there are no possible sources, matching is skipped entirely. The same diff now takes about 0.15 seconds, and the rewrites found in diffs of the `rust-lang/rust` repository are unchanged. Note that `num_checks` for exact-id matching now only counts possible sources, so the cancellation for very large trees triggers later. Assisted-by: Claude Opus 5.5
Martin von Zweigbergk (martinvonz)
force-pushed
the
mz/txulrqqtxoln
branch
from
October 11, 2026 23:18
a1b2daf to
cca6c27
Compare
Member
|
Thanks a lot! While some benchmark scenarios are now up to ~20% slower, most others are vastly improved and up to 4x faster. As we know, in more realistic scenarios that quickly becomes unbearably slow. So let's have it, and I will see to create a patch release. Benchmarks
❯ cargo bench -p gix-diff rewrite-tracker --features blob
Compiling gix-diff v0.69.0 (/Users/byron/dev/github.com/GitoxideLabs/gitoxide.mz-txulrqqtxoln/gix-diff)
Finished `bench` profile [optimized + debuginfo] target(s) in 4.80s
Running unittests src/lib.rs (target/release/deps/gix_diff-902dda2f3d2d9d7e)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 5 filtered out; finished in 0.00s
Running benches/line_count.rs (target/release/deps/line_count-e14c01ef12afa590)
Gnuplot not found, using plotters backend
Running benches/rewrite_tracker.rs (target/release/deps/rewrite_tracker-002afbb3a2c18fe3)
Gnuplot not found, using plotters backend
rewrite-tracker/exact-renames/128
time: [30.793 µs 31.026 µs 31.256 µs]
thrpt: [4.0952 Melem/s 4.1256 Melem/s 4.1568 Melem/s]
change:
time: [+14.361% +15.286% +16.220%] (p = 0.00 < 0.05)
thrpt: [−13.956% −13.259% −12.558%]
Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
4 (4.00%) high mild
rewrite-tracker/exact-renames/1024
time: [325.89 µs 326.88 µs 327.90 µs]
thrpt: [3.1229 Melem/s 3.1326 Melem/s 3.1421 Melem/s]
change:
time: [+18.158% +18.791% +19.493%] (p = 0.00 < 0.05)
thrpt: [−16.313% −15.818% −15.367%]
Performance has regressed.
Found 9 outliers among 100 measurements (9.00%)
8 (8.00%) high mild
1 (1.00%) high severe
Benchmarking rewrite-tracker/exact-renames/4096: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 8.3s, enable flat sampling, or reduce sample count to 50.
rewrite-tracker/exact-renames/4096
time: [1.6244 ms 1.6306 ms 1.6375 ms]
thrpt: [2.5013 Melem/s 2.5120 Melem/s 2.5216 Melem/s]
change:
time: [+24.219% +24.848% +25.519%] (p = 0.00 < 0.05)
thrpt: [−20.331% −19.902% −19.497%]
Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
4 (4.00%) high mild
rewrite-tracker/ambiguous-identical-content/16
time: [2.4156 µs 2.4253 µs 2.4343 µs]
thrpt: [6.5727 Melem/s 6.5973 Melem/s 6.6237 Melem/s]
change:
time: [−24.708% −24.366% −24.008%] (p = 0.00 < 0.05)
thrpt: [+31.593% +32.216% +32.817%]
Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
4 (4.00%) low mild
3 (3.00%) high mild
1 (1.00%) high severe
rewrite-tracker/ambiguous-identical-content/128
time: [28.878 µs 29.021 µs 29.173 µs]
thrpt: [4.3875 Melem/s 4.4107 Melem/s 4.4324 Melem/s]
change:
time: [−65.495% −65.239% −65.000%] (p = 0.00 < 0.05)
thrpt: [+185.72% +187.68% +189.81%]
Performance has improved.
rewrite-tracker/ambiguous-identical-content/512
time: [229.84 µs 230.87 µs 232.02 µs]
thrpt: [2.2067 Melem/s 2.2177 Melem/s 2.2276 Melem/s]
change:
time: [−79.738% −79.637% −79.542%] (p = 0.00 < 0.05)
thrpt: [+388.80% +391.09% +393.54%]
Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mild
rewrite-tracker/similarity-renames/16
time: [227.49 µs 228.19 µs 228.84 µs]
thrpt: [69.918 Kelem/s 70.116 Kelem/s 70.332 Kelem/s]
change:
time: [+0.2023% +0.6431% +1.0553%] (p = 0.00 < 0.05)
thrpt: [−1.0443% −0.6390% −0.2019%]
Change within noise threshold.
rewrite-tracker/similarity-renames/64
time: [3.3021 ms 3.3125 ms 3.3230 ms]
thrpt: [19.260 Kelem/s 19.321 Kelem/s 19.382 Kelem/s]
change:
time: [+0.6085% +0.9952% +1.3785%] (p = 0.00 < 0.05)
thrpt: [−1.3598% −0.9854% −0.6048%]
Change within noise threshold.
rewrite-tracker/similarity-renames/128
time: [12.727 ms 12.762 ms 12.799 ms]
thrpt: [10.001 Kelem/s 10.030 Kelem/s 10.057 Kelem/s]
change:
time: [−0.5571% −0.2028% +0.1458%] (p = 0.28 > 0.05)
thrpt: [−0.1456% +0.2032% +0.5603%]
No change in performance detected.
Found 3 outliers among 100 measurements (3.00%)
3 (3.00%) high mild
rewrite-tracker/exhaustive-copy-sources/16
time: [2.6333 µs 2.6430 µs 2.6526 µs]
thrpt: [6.0317 Melem/s 6.0536 Melem/s 6.0761 Melem/s]
change:
time: [−47.972% −47.687% −47.424%] (p = 0.00 < 0.05)
thrpt: [+90.200% +91.157% +92.205%]
Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
rewrite-tracker/exhaustive-copy-sources/128
time: [46.843 µs 46.998 µs 47.172 µs]
thrpt: [2.7134 Melem/s 2.7235 Melem/s 2.7325 Melem/s]
change:
time: [−76.046% −75.942% −75.840%] (p = 0.00 < 0.05)
thrpt: [+313.91% +315.66% +317.46%]
Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
3 (3.00%) high mild
rewrite-tracker/exhaustive-copy-sources/512
time: [561.39 µs 563.01 µs 564.85 µs]
thrpt: [906.43 Kelem/s 909.41 Kelem/s 912.02 Kelem/s]
change:
time: [−80.082% −80.002% −79.920%] (p = 0.00 < 0.05)
thrpt: [+398.02% +400.05% +402.06%]
Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
4 (4.00%) high mild
1 (1.00%) high severe |
Martin von Zweigbergk (martinvonz)
added a commit
to jj-vcs/jj
that referenced
this pull request
Oct 12, 2026
Path comparisons show up prominently in profiles of the concurrent tree walk, which keeps pending items in a BTreeMap keyed by path. Splitting both paths into components on every comparison is slow. Comparing the strings byte by byte, with '/' sorting before all other bytes, gives the same order. When forcing the concurrent diff walk, this makes `jj diff --name-only --from 'root()' --to @` in the rust-lang/rust repo (63k files) about 40% faster (257 ms -> 159 ms, and 3.5G -> 2.1G instructions). I disabled copy/rename detection for this because it was slow (improved with GitoxideLabs/gitoxide#3055) and would have drowned out the tree walk cost. Assisted-by: Claude Opus 5.5
Martin von Zweigbergk (martinvonz)
added a commit
to jj-vcs/jj
that referenced
this pull request
Oct 13, 2026
Path comparisons show up prominently in profiles of the concurrent tree walk, which keeps pending items in a BTreeMap keyed by path. Splitting both paths into components on every comparison is slow. Comparing the strings byte by byte, with '/' sorting before all other bytes, gives the same order. When forcing the concurrent diff walk, this makes `jj diff --name-only --from 'root()' --to @` in the rust-lang/rust repo (63k files) about 40% faster (257 ms -> 159 ms, and 3.5G -> 2.1G instructions). I disabled copy/rename detection for this because it was slow (improved with GitoxideLabs/gitoxide#3055) and would have drowned out the tree walk cost. Assisted-by: Claude Opus 5.5
Martin von Zweigbergk (martinvonz)
added a commit
to jj-vcs/jj
that referenced
this pull request
Oct 14, 2026
Path comparisons show up prominently in profiles of the concurrent tree walk, which keeps pending items in a BTreeMap keyed by path. Splitting both paths into components on every comparison is slow. Comparing the strings byte by byte, with '/' sorting before all other bytes, gives the same order. When forcing the concurrent diff walk, this makes `jj diff --name-only --from 'root()' --to @` in the rust-lang/rust repo (63k files) about 40% faster (257 ms -> 159 ms, and 3.5G -> 2.1G instructions). I disabled copy/rename detection for this because it was slow (improved with GitoxideLabs/gitoxide#3055) and would have drowned out the tree walk cost. Assisted-by: Claude Opus 5.5
Kevin Rajan (kvnloo)
pushed a commit
to kvnloo/jj
that referenced
this pull request
Oct 14, 2026
Path comparisons show up prominently in profiles of the concurrent tree walk, which keeps pending items in a BTreeMap keyed by path. Splitting both paths into components on every comparison is slow. Comparing the strings byte by byte, with '/' sorting before all other bytes, gives the same order. When forcing the concurrent diff walk, this makes `jj diff --name-only --from 'root()' --to @` in the rust-lang/rust repo (63k files) about 40% faster (257 ms -> 159 ms, and 3.5G -> 2.1G instructions). I disabled copy/rename detection for this because it was slow (improved with GitoxideLabs/gitoxide#3055) and would have drowned out the tree walk cost. Assisted-by: Claude Opus 5.5
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.
Tracker::emit()kept all changes in a single list, and for each destination,find_match()scanned that whole list for possible sources. That takes time proportional to the number of destinations times the number of all changes, even when there are few or no possible sources.For example, diffing the empty tree against a commit in the
rust-lang/rustrepository, which adds about 63,000 files, took about 17 seconds injjwith copy tracking enabled. There are no deletions or modifications, so nothing can be a source. Thelimitdoesn't help since it applies tonum_sources * num_destinations, which is 0.match_pairs()now collects the indices of possible sources once, andfind_match()only looks at those. This applies to the exact-id matching too, which was also quadratic when many destinations had the same id. If there are no possible sources, matching is skipped entirely. The same diff now takes about 0.15 seconds, and the rewrites found in diffs of therust-lang/rustrepository are unchanged.Note that
num_checksfor exact-id matching now only counts possible sources, so the cancellation for very large trees triggers later.Assisted-by: Claude Opus 5.5