Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate buffer management and split point calculation #41

Merged
merged 5 commits into from
Oct 15, 2022

Conversation

MichaelEischer
Copy link
Member

Chunker.Next currently handles buffer management and the split point calculation at the same time. While experimenting with a way to also chunk Tree blobs, I noticed that this combination is a problem: trees are built incrementally such that the Chunker cannot simply read the next part of the tree into its buffer. Moving the buffer out of the Chunker helps in this case, as it allows to manage a buffer in a way that better matches what is necessary to split tree blobs.

For now the PR splits the chunker into BaseChunker and Chunker. However, if the tree blob chunking works out as planned, then the Chunker will no longer be used by restic and could be removed unless there are other users.

Performance seems to be mostly unaffected, maybe even improve a tiny bit:

name               old time/op   new time/op   delta
ChunkerWithSHA256   93.3ms ± 0%   93.2ms ± 0%    ~     (p=1.000 n=9+9)
Chunker             41.7ms ± 0%   41.5ms ± 1%  -0.41%  (p=0.002 n=9+9)

name               old speed     new speed     delta
ChunkerWithSHA256  360MB/s ± 0%  360MB/s ± 0%    ~     (p=0.950 n=9+9)
Chunker            805MB/s ± 0%  808MB/s ± 1%  +0.41%  (p=0.002 n=9+9)

@MichaelEischer
Copy link
Member Author

LGTM.

@MichaelEischer MichaelEischer merged commit 5536e97 into restic:master Oct 15, 2022
@MichaelEischer MichaelEischer deleted the splitpoint branch October 15, 2022 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant