Repository navigation
put repo objects as parts, join them into the pack once, refs #10058 - #10547
Open
ThomasWaldmann wants to merge 1 commit into
Open
ThomasWaldmann wants to merge 1 commit into
ThomasWaldmann wants to merge 1 commit into
Conversation
…kup#10058 RepoObj.format_parts returns a repo object as a list of parts (header + encrypted metadata, then the data envelope parts) instead of concatenating them, and Repository.put / PackWriter.add accept such a list. The PackWriter joins the parts of all objects of a pack once, when it builds the pack. For "authenticated-*" keys, encrypt_parts returns header + tag and the payload as separate parts, so the payload is not copied just to prepend 34 bytes. A part may reference the caller's buffer (e.g. the chunker buffer for uncompressed data with an authenticated key), so PackWriter.add copies every part that is not bytes; bytes parts are buffered as they are. cache.add_chunk (the create / transfer / recreate hot path) uses format_parts; format() still returns bytes for the other callers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10547 +/- ##
==========================================
- Coverage 89.36% 89.34% -0.02%
==========================================
Files 104 104
Lines 20082 20092 +10
Branches 3152 3153 +1
==========================================
+ Hits 17947 17952 +5
- Misses 1478 1482 +4
- Partials 657 658 +1 ☔ View full report in Codecov by Harness. |
This branch has not been deployed
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.
Description
Implements the remaining item 1 of #10058 (the "pieces" refactor): the last avoidable full-size copies on the create side.
Before, each new chunk was copied up to three times before reaching the pack:
key.encryptforauthenticated-*keys:b"".join([header, tag, data])copied the whole payload to prepend 34 bytes,RepoObj.format:hdr + meta_encrypted + data_encryptedcopied the encrypted data again,PackWriter:b"".join(...)of all objects of a pack (needed anyway).Now:
KeyBase.encrypt_parts()returns the envelope as a list of parts; forauthenticated-*keys that is[header + tag, data](no payload copy), AEAD keys return[encrypt(...)].encrypt()is unchanged in what it returns.RepoObj.format_parts()returns[header + meta_encrypted, *data_parts];format()joins it, so its other callers are unchanged.Repository.put()/PackWriter.add()accept bytes or a list of parts (MAX_DATA_SIZEapplies to the total); the PackWriter joins all parts of all objects once when it builds the pack.cache.add_chunk()(create / transfer / recreate hot path) usesformat_parts().A part may reference the caller's buffer: on create, the chunk data is a memoryview into the reused chunker buffer, and with an
authenticated-*key and data that stays uncompressed, it ends up as a part as is. As the pack is only built later,PackWriter.add()copies every part that is notbytes(bytesparts are buffered by reference).Full-size payload copies per new chunk, from encryption to the finished pack:
authenticated-*none): borrowed chunker bufferPackWriter.addcopy, pack join)authenticated-*bytes)The copies saved are of the payload after compression, so the case that saves 2 copies is also the one where the copied payload is smaller than the chunk.
Tests:
test_format_parts: authenticated / chacha20-poly1305 / aes-ocb × none / lz4: joined parts parse, equalformat()output (deterministic key), uncompressed authenticated data is not copied.test_put_parts: the caller overwrites its buffer afterput()and before the flush; stored objects, offsets and sizes are still correct.test_max_data_size_parts: the size limit covers all parts together.Benchmarks
Unmodified master vs. this branch (same compiled extensions), each tree's own path:
format()/format_parts()→PackWriter.add→ pack join + store hash, null store. 2 MiB chunks as a memoryview into one reused buffer (like the chunker), 200 chunks (~8 packs) per run, each run in a fresh process; old/new interleaved with alternating order, 2 batches × 7 rounds, medians, macOS / Apple Silicon.Incompressible data saves 1 copy of the full 2 MiB with every key (see the table above): ~25–50 µs per chunk. For comparison, an isolated 2 MiB copy costs ~50–55 µs here into an existing buffer and ~85 µs into a freshly allocated one (macOS returns large blocks to the OS, so a fresh one page-faults), so the saving is somewhat less than one isolated copy.
To see the 2-copy case, a third batch used data that lz4 compresses to ~50% (2 MiB of alternating 4 KiB random / 4 KiB zero blocks), so ~1 MiB payloads:
The authenticated keys save clearly more there than the AEAD keys (2 vs. 1 copies; an isolated 1 MiB copy costs ~11 µs here). The AEAD rows are close to the noise level, so the ratio between the two is not precise.
Compressible text (lz4: 7.8%, zstd,3: 1.3% of the input) leaves only small payloads to copy, so the difference is within noise there. On a whole
borg create(reading, chunking, id hashing on top) the relative gain is smaller, in line with the 2–3% estimate in #10058.Checklist
master🤖 Generated with Claude Code