fixed chunker: read directly into per-chunk buffers - #10060
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10060 +/- ##
==========================================
- Coverage 86.74% 86.64% -0.10%
==========================================
Files 98 98
Lines 17085 17086 +1
Branches 2586 2585 -1
==========================================
- Hits 14820 14804 -16
- Misses 1571 1589 +18
+ Partials 694 693 -1 ☔ View full report in Codecov by Harness. |
ThomasWaldmann
force-pushed
the
fixed-chunker
branch
2 times, most recently
from
August 9, 2026 11:43
cdbed5b to
76500ab
Compare
This was referenced Aug 9, 2026
ChunkerFixed used FileReader.read(), which assembles each chunk in an intermediate bytearray (copying the data out of the reader's block buffers) and then converts it to bytes (copying everything again). Read each chunk via FileReader.readinto() into a fresh per-chunk buffer instead: each byte is copied exactly once, from the reader's block buffer into the chunk, and the chunk is yielded as a memoryview over that buffer. All-zero detection now happens at chunk granularity, like in the content-defined chunkers. Also update the reader type stub: FileReader.readinto was missing there, and Chunk data may be a memoryview (as the CDC chunkers already yield). Behavior notes: - chunk data stays valid after the iterator advances (the buffer is per chunk, not reused), matching the previous semantics. - ranges stemming from holes in sparse files are now reported as CH_ALLOC instead of CH_HOLE - downstream treats both identically. Measured on a 20 GiB create (fixed,4194304, lz4, unencrypted repo, on top of the copy-removal changes of borgbackup#10059 which let the yielded memoryview flow through compression without being copied to bytes): ~10% faster. Standalone (without borgbackup#10059) only ~2%, as the compressor's bytes() coercion then re-adds one copy.
ThomasWaldmann
force-pushed
the
fixed-chunker
branch
from
August 9, 2026 12:52
76500ab to
bfc12c0
Compare
FileReader.readinto copied each byte once: from the 1 MiB blocks that FileFMAPReader reads (a freshly allocated bytes object per block) into the caller's buffer. When there is no sparse processing and no fmap was given - the normal case - there are no ranges to consider: the file is read start to end. readinto() then skips the block reader entirely and lets the OS read directly into the caller's buffer (os.readv, or the file object's own readinto): zero user-space copies, no per-block allocations, and one syscall per request (typically a whole chunk or scan-buffer fill) instead of one per block. fadvise DONTNEED behavior is kept. This benefits the fixed chunker (which now goes disk -> chunk buffer with no intermediate steps) as well as the content-defined chunkers, whose fill() reads into the scan buffer through the same method. The sparse/fmap path is unchanged. Measured (20 GiB, unencrypted repo, lz4, page-cache-warm, Apple M3 Pro): fixed,4194304 create ~2-3% faster; for the CDC chunkers the saved copy is below run-to-run noise. The structural wins: ~20000 fewer 1 MiB allocations and 4x fewer read syscalls per 20 GiB.
This was referenced Aug 9, 2026
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.
Based on the copy-removal work of #10059 (now merged). Two commits:
ChunkerFixedreads viaFileReader.readinto()into per-chunk buffers (one copy) instead ofFileReader.read()'s bytearray-assembly +bytes()conversion (two copies).FileReader.readinto()gets a fast path: without sparse processing and without an fmap (the normal case), the OS reads the file data directly into the caller's buffer (os.readv/ the file object'sreadinto) - zero user-space copies, no per-block 1 MiB allocations, one syscall per chunk instead of one per block. This also covers the content-defined chunkers, whosefill()reads the scan buffer through the same method. Sparse/fmap readers keep the buffered path.ChunkerFixedusedFileReader.read(), which assembles each chunk in an intermediate bytearray (copying the data out of the reader's block buffers) and then converts it to bytes (copying everything again). Profiling a 20 GiBcreatewithfixed,4194304on an unencrypted repo showedchunkifyas the single largest stage (29%), most of it these copies — the content-defined chunkers avoid both via theirreadintopath.Now each chunk is read via
FileReader.readinto()into a fresh per-chunk buffer: each byte is copied exactly once, from the reader's block buffer into the chunk, which is yielded as a memoryview over that buffer. All-zero detection happens at chunk granularity, like in the CDC chunkers.Behavior notes:
Measured (20 GiB,
fixed,4194304, lz4, unencrypted repo, page-cache-warm, alternating runs, Apple Silicon): commit 1 on top of #10059 takes create from 29.35 s to 26.27 s (−10.5%), withchunkifyself-time dropping 8.55 s → 6.73 s in the profile. Commit 2 adds another ~2-3% for the fixed chunker (the remaining user-space copy; what remains inchunkifynow is the kernel's page-cache→buffer transfer); for the CDC chunkers its saved copy is below run-to-run noise, but the ~20000 avoided 1 MiB allocations and 4x fewer read syscalls per 20 GiB apply there too.🤖 Generated with Claude Code