remove avoidable per-chunk memory copies on the hot data path - #10059
Merged
Conversation
The chunker hands chunk data to the compressors as a memoryview into its scan buffer, but LZ4._decide (the default compression) coerced it to bytes first, copying the full plaintext chunk once per chunk just to get a char* for the C call. Use a typed memoryview instead, so lz4 reads straight from the chunker's buffer. CompressorBase.compress (used by CNONE, i.e. also the fallback for incompressible chunks) did the same coercion, which is only needed in legacy mode for the ID/level prefix concatenation - keep it there and pass the buffer through otherwise. This removed a second full copy for incompressible chunks. ObfuscateSize.compress now may receive a memoryview from the inner compressor, so join instead of concatenating (and skip the copy completely when the added padding size is 0). Measured on a 5 GiB borg create (lz4, aes256-ocb, half incompressible / half compressible input): ~3% faster overall, compression-path time per stored chunk -23%.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10059 +/- ##
==========================================
+ Coverage 86.64% 86.74% +0.09%
==========================================
Files 98 98
Lines 17037 17085 +48
Branches 2581 2586 +5
==========================================
+ Hits 14762 14820 +58
+ Misses 1583 1571 -12
- Partials 692 694 +2 ☔ View full report in Codecov by Harness. |
ThomasWaldmann
commented
Aug 9, 2026
ThomasWaldmann
commented
Aug 9, 2026
_AEAD_BASE.encrypt and .decrypt used a PyMem_Malloc'd scratch buffer (with a one-block safety margin) and created the returned bytes object by slicing it, copying the whole ciphertext / plaintext once more per chunk before freeing the scratch buffer. Our AEAD ciphers (AES-OCB, chacha20-poly1305) are padding-free, so the output size is known exactly up front: allocate the result via PyBytes_FromStringAndSize(NULL, n) and let OpenSSL write into it directly - no scratch buffer, no copy, no malloc/free per chunk. Caveat found by the tests: EVP_EncryptFinal_ex is not output-free for OCB - OpenSSL buffers the partial final block in EncryptUpdate and emits it at Final (the total is still exactly the input length). As our AEAD modes are padding-free, Final can never write more than the space left in the exact-size result buffer, so it writes directly into it; a length check afterwards verifies the total. This helps both directions: borg create saves one ciphertext-sized copy per chunk, and the read paths (extract, mount, check --verify-data) save a plaintext-sized copy per chunk. Measured on 5 GiB (lz4, aes256-ocb): create ~1.5% faster, extract ~6% faster.
LZ4.decompress guessed the output size, decompressed into a thread-local scratch buffer (growing and retrying on overflow) and then copied the whole plaintext into the returned bytes object - one full plaintext-sized copy per chunk on every read (extract, mount, check --verify-data). borg2 stores the exact plaintext size in the (authenticated) object metadata, so when meta["size"] is known, allocate the result exactly and let LZ4_decompress_safe write straight into it: no scratch buffer, no guessing, no copy. A size mismatch raises DecompressionError (this also covers what check_fix_size asserted). The guess-and-retry loop remains as the fallback for borg 1.x data read in legacy mode (borg transfer), where the size is not known up front. Measured on a 5 GiB extract (lz4, aes256-ocb): ~4% faster.
ThomasWaldmann
force-pushed
the
memory-perf
branch
from
August 9, 2026 11:34
261ad1b to
10fbb99
Compare
ThomasWaldmann
added a commit
to ThomasWaldmann/borg
that referenced
this pull request
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. 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
added a commit
to ThomasWaldmann/borg
that referenced
this pull request
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.
This was referenced Aug 9, 2026
ThomasWaldmann
added a commit
to ThomasWaldmann/borg
that referenced
this pull request
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.
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
1 participant
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.
Analysis of where
borg create/borg extractcopy or allocate big amounts of memory (chunk-sized buffers) found several avoidable full-chunk copies per chunk. This PR removes the easy ones; three commits, each independently tested:compress: do not copy chunk data to bytes, use the buffer protocol
The chunker hands chunk data to the compressors as a memoryview into its scan buffer, but
LZ4._decide(default compression) coerced it to bytes first — one full plaintext copy + allocation per chunk. Now uses a typed memoryview.CompressorBase.compressonly materializes bytes in legacy mode (the prefix concat needs it), which also removes a second full copy for incompressible chunks (LZ4→CNONE fallback).ObfuscateSize.compressjoins instead of concatenating (and skips the copy for zero padding), as CNONE may now pass a memoryview through.crypto: AEAD encrypt/decrypt directly into the result bytes object
_AEAD_BASE.encrypt/decryptused a PyMem_Malloc'd scratch buffer and created the returned bytes by slicing it — one ciphertext-sized (encrypt) / plaintext-sized (decrypt) copy plus a malloc/free round-trip per chunk. Our AEAD ciphers are padding-free, so the output size is known exactly: allocate the result viaPyBytes_FromStringAndSize(NULL, n)and let OpenSSL write into it directly. Caveat (caught by the tests): OpenSSL's OCB emits the buffered partial final block fromEVP_EncryptFinal_ex, not from Update. As our AEAD modes are padding-free, Final can never write more than the space left in the exact-size result buffer, so it writes directly into it (review feedback); a length check afterwards verifies the total.compress: lz4 decompresses directly into the result bytes object
LZ4.decompressguessed the output size, decompressed into a scratch buffer (growing/retrying) and copied the plaintext into the returned bytes. borg2 stores the exact plaintext size in the authenticated object metadata, so decompress straight into an exactly-sized result. The guess-and-retry loop remains as fallback for borg 1.x data read in legacy mode (borg transfer).Measurements (20 GiB, half incompressible / half compressible non-deduplicating data, lz4, aes256-ocb, page-cache-warm, 4 alternating A/B runs each on Apple Silicon; every PR run was faster than every master run):
borg createborg extract --stdoutThe extract gain also benefits
borg mountandcheck --verify-data.cProfile confirms the savings come entirely out of the compression/crypto path (chunker, id_hash unchanged).
Noticed while working on this (pre-existing on master, not addressed here):
--compression auto,...raises ZeroDivisionError inAuto.compressfor empty input; not reachable via the chunker (it never yields empty chunks).🤖 Generated with Claude Code