diff: align the chunk lists to count added/removed bytes - #10358
Open
ThomasWaldmann wants to merge 1 commit into
Open
diff: align the chunk lists to count added/removed bytes#10358ThomasWaldmann wants to merge 1 commit into
ThomasWaldmann wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10358 +/- ##
==========================================
- Coverage 88.02% 85.68% -2.35%
==========================================
Files 103 103
Lines 18913 18913
Branches 2919 2919
==========================================
- Hits 16649 16205 -444
- Misses 1572 2026 +454
+ Partials 692 682 -10 ☔ View full report in Codecov by Harness. |
ThomasWaldmann
force-pushed
the
diff-chunk-alignment
branch
from
September 12, 2026 13:36
470d0b2 to
c5c3a39
Compare
The byte counts of a "modified" file were derived from the difference of the two chunk id SETS, which loses everything about order and multiplicity: a file whose chunks were only reordered was reported as modified with 0 B added and 0 B removed, and duplicating a chunk added no bytes at all. Align the two chunk lists as sequences instead (difflib.SequenceMatcher, the way a text diff aligns lines) and count the chunks that are not part of the alignment: the ones of archive1 as removed, the ones of archive2 as added. Insertions, removals, moves and duplicated chunks are now all reflected by the byte counts. The common prefix and suffix of the two lists are stripped first. That is what makes the usual cases cheap (e.g. a file that was appended to) and it also keeps the matcher away from the long runs of identical chunks it is slow on. SequenceMatcher still degrades to quadratic runtime on chunk lists that repeat the same chunk id very often (sparse files, VM images with big all-zero ranges), so the alignment is skipped above MAX_ALIGN_CHUNKS / MAX_ALIGN_WORK and the chunk ids are only counted then, which is what borg did before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ThomasWaldmann
force-pushed
the
diff-chunk-alignment
branch
from
September 12, 2026 13:47
c5c3a39 to
da0c417
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
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.
What
borg diffderived the added/removed byte counts of a modified file from the difference of the two chunk id sets. A set knows nothing about order or multiplicity, so it misses real content changes:[A]->[A,A,A])[A,A,A]->[A])This PR aligns the two chunk lists as sequences instead, the way a text diff aligns lines (
difflib.SequenceMatcher). The chunks that are part of the alignment are the unchanged content; everything else is counted, the chunks of ARCHIVE1 as removed and the ones of ARCHIVE2 as added. Insertions, removals, moves and duplicates are all accounted for now.Real run, a 9 MB file whose two 3 MB blocks were swapped (default chunker):
Runtime
SequenceMatcheris quadratic in the worst case, so the newchunks_diff_size()has two guards:MAX_ALIGN_CHUNKS(65536) orMAX_ALIGN_WORK(2^20) the alignment is skipped and the chunk ids are only counted per id, i.e. approximately what borg did before (but multiplicity-aware).MAX_ALIGN_WORKis the estimated matcher work,sum over list1 of the chunk's number of occurrences in list2; it is what catches the pathological case, chunk lists that repeat one id very often (sparse files, VM images with big all-zero ranges), where 8k chunks already took 5.6 s.Measured on an M3 Pro, worst cases per file after both guards:
Without the guards the last two rows would be several seconds each.
Notes
--chunker-paramsare unaffected (they are still compared by content and still report no byte counts).test_reordered_chunks(from diff: report a file whose chunks were only reordered or duplicated as modified #10350) is updated: the swap of two 1 KB chunks now reports +1.0 kB / -1.0 kB rather than 0 B / 0 B, which is the point of this PR. New tests:test_duplicated_chunks,test_inserted_chunk, and unit tests forchunks_diff_size()including both fallback paths.Item.get_size(consider_ids=...)has no caller any more, it existed only for the old set-based counting. Left in place to keep the diff small - say the word and I will drop it.borg diffepilog anddocs/internals/frontends.rst.Full archiver test suite passes locally (1177 passed, 623 skipped).
🤖 Generated with Claude Code