Repository navigation
Small snapshot uploads no longer reserve 1 MiB compression chunks per worker - #3375
Open
Abhishek Sharma (abhisheksharma2411) wants to merge 1 commit into
Conversation
… worker A chunk is a whole single-segment frame, so the floor only has to cover the encoder's own state. Lower it to 64 KiB. Measured on a 36-byte file: 16.6 MiB -> 1.6 MiB at 8 workers. The two uploader expectations that moved were written against parZstdMinChunk where they meant the chunk actually chosen; they now pass at either floor. Refs agent-substrate#2553 Signed-off-by: Abhishek Sharma <[email protected]>
2 tasks done
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.
Why is this needed? (Or link the issue.)
Follow-up to #2945, under #2553. #2945 sizes the compression chunk to the data,
but floors it at 1 MiB, and the pool is still filled eagerly with
workers * parZstdQueuebuffers. So a tiny file still costsworkers * 2 * 1 MiB,and
planStreamtakesworkersfromGOMAXPROCS, so the cost grows with thenode rather than with the file.
Measured on a 36-byte file, allocation during
writeSparseZstd:The floor can come down because a chunk is a whole single-segment frame. An
input at or below the floor is one frame either way and nothing can match past
its end, so a smaller floor costs no ratio -- it only narrows the encoder's
window to something the frame could never have used.
This also shows up in the planner: for an all-holes file the reservation drops
from 8.5 MiB to 1 MiB on GCS, and 168 MiB to 160.5 MiB on S3.
Two existing uploader expectations moved. Both were written in terms of
parZstdMinChunkwhere they meant the chunk actually chosen for that input --the two were equal only while the floor was 1 MiB. They now say
parZstdChunkSize(...)and pass at either floor, which I checked by revertingthe constant.
parZstdChunkSizehad no test, so this adds one, plus a round-trip at sizeseither side of the new floor (the one place a too-small window would corrupt
rather than just enlarge output).
Not included: making the pool allocate lazily instead of eagerly, which would
flatten the remaining worker-count scaling. That is a real change to the pool
rather than a constant, so it belongs in its own PR.
Adjacent, not conflicting as far as I can tell: #1766 also touches parzstd.go
but predates #2945.
AI use: AI-assisted. I reviewed and tested it.