[lib] Always load the dictionary in one go#2607
Merged
terrelln merged 2 commits intofacebook:devfrom May 5, 2021
Merged
Conversation
terrelln
force-pushed
the
deterministic-dict
branch
3 times, most recently
from
359da5e to
94db439
Compare
Dictionaries larger than `ZSTD_CHUNKSIZE_MAX` used to have to be loaded in multiple segments. Instead, when we detect large dictionaries, ensure that we reset the context's indicies. Then, for dictionaries larger than `ZSTD_CURRENT_MAX - 1`, only load the suffix of the dictionary. Finally, enable DDS for large dictionaries, since we no longer load in multiple segments. This simplifes the dictionary loading code, and reduces opportunities for non-determinism to slip in.
terrelln
force-pushed
the
deterministic-dict
branch
from
d71ca48 to
0b88c25
Compare
Dictionary size must be > `ZSTD_CHUNKSIZE_MAX`.
Cyan4973
approved these changes
May 5, 2021
Merged
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
3 participants
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.
Dictionaries larger than
ZSTD_CHUNKSIZE_MAXused to have to be loadedin multiple segments. Instead, when we detect large dictionaries, ensure
that we reset the context's indicies. Then, for dictionaries larger than
ZSTD_CURRENT_MAX - 1, only load the suffix of the dictionary. Finally,enable DDS for large dictionaries, since we no longer load in multiple
segments.
This simplifes the dictionary loading code, and reduces opportunities
for non-determinism to slip in.
I've manually tested with large dictionaries, and added one large
--patch-fromtest. This gives us some coverage of largedictionaries, and we should have should have good coverage of
normal sized dictionaries from our fuzzers/tests.