PackReader.read() copies every chunk out of a pack that borg already holds in memory:
https://github.com/borgbackup/borg/blob/master/src/borg/repository.py#L227-L231
def read(self, offset, size):
# read from the in-memory pack if we have it, else range-read from the store
if self.pack_contents is not None:
return self.pack_contents[offset : offset + size]
return self.store.load(self.key, offset=offset, size=size)
pack_contents is a bytes object, so the slice allocates a fresh buffer and memcpy's the chunk into it, once per chunk read. On the borg extract path that is every chunk of the archive, and _cached_pack_reader() means the pack really is already resident, so the copy buys nothing.
A memoryview slice would hand out a view instead:
if self.pack_contents is not None:
return memoryview(self.pack_contents)[offset : offset + size]
How much is it worth
Profiled borg extract on master (f8dee0c), 6 GiB archive, zstd,3 / aes256-ocb / sha256 ids, macOS/arm64, native sampling:
|
% of extract cpu |
| zstd decompression |
48.1% |
| file write |
23.9% |
| memory management |
17.5% |
| aes-ocb decryption |
6.6% |
| file read |
3.3% |
Breaking the memory-management part down by caller, bytes_subscript - this slice - is 1.33% of total extract cpu. Small, but it is pure waste and the fix is one line.
What to check before/while doing it
- The consumers need to cope with a memoryview. The AEAD layer already does:
low_level.pyx takes buffers via ro_buffer()/PyObject_GetBuffer, so decryption should be fine. RepoObj.parse() already does memoryview(cdata) internally.
msgpack.unpackb() and anything doing data[a:b] == b"..." style comparisons on the result should be checked.
- A memoryview keeps the whole pack (up to
DEFAULT_PACK_MAX_SIZE, 50 MB) alive for as long as any view of it exists. With _cached_pack_reader()'s LRU that is probably fine, since the cache holds the packs anyway, but it is worth convincing ourselves that no view outlives the cache entry in a way that pins packs which would otherwise be evicted.
- Please measure before and after rather than trusting the 1.33% above - it was measured on one machine with one workload, and the win will differ with chunk size and pack cache hit rate.
@mr-raj12 you have been in this code recently with the pack header AAD work (#9946), so you may already have the context for this one - would you like to take it?
Related: I also found the AEAD encrypt/decrypt path allocating a fresh output buffer per chunk, which is a bigger slice of the same 17.5% - filed separately.
🤖 Generated with Claude Code
PackReader.read()copies every chunk out of a pack that borg already holds in memory:https://github.com/borgbackup/borg/blob/master/src/borg/repository.py#L227-L231
pack_contentsis abytesobject, so the slice allocates a fresh buffer and memcpy's the chunk into it, once per chunk read. On theborg extractpath that is every chunk of the archive, and_cached_pack_reader()means the pack really is already resident, so the copy buys nothing.A
memoryviewslice would hand out a view instead:How much is it worth
Profiled
borg extracton master (f8dee0c), 6 GiB archive,zstd,3/aes256-ocb/sha256ids, macOS/arm64, native sampling:Breaking the memory-management part down by caller,
bytes_subscript- this slice - is 1.33% of total extract cpu. Small, but it is pure waste and the fix is one line.What to check before/while doing it
low_level.pyxtakes buffers viaro_buffer()/PyObject_GetBuffer, so decryption should be fine.RepoObj.parse()already doesmemoryview(cdata)internally.msgpack.unpackb()and anything doingdata[a:b] == b"..."style comparisons on the result should be checked.DEFAULT_PACK_MAX_SIZE, 50 MB) alive for as long as any view of it exists. With_cached_pack_reader()'s LRU that is probably fine, since the cache holds the packs anyway, but it is worth convincing ourselves that no view outlives the cache entry in a way that pins packs which would otherwise be evicted.@mr-raj12 you have been in this code recently with the pack header AAD work (#9946), so you may already have the context for this one - would you like to take it?
Related: I also found the AEAD encrypt/decrypt path allocating a fresh output buffer per chunk, which is a bigger slice of the same 17.5% - filed separately.
🤖 Generated with Claude Code