-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add Controls to Constrain Window Size to Protocol Limits #4261
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
Open
felixhandte
wants to merge
4
commits into
facebook:dev
Choose a base branch
from
felixhandte:constrain-window-for-protocols
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -515,6 +515,7 @@ typedef enum { | |
| * ZSTD_c_prefetchCDictTables | ||
| * ZSTD_c_enableSeqProducerFallback | ||
| * ZSTD_c_maxBlockSize | ||
| * ZSTD_c_constrainWindowForProtocol | ||
| * Because they are not stable, it's necessary to define ZSTD_STATIC_LINKING_ONLY to access them. | ||
| * note : never ever use experimentalParam? names directly; | ||
| * also, the enums values themselves are unstable and can still change. | ||
|
|
@@ -538,7 +539,8 @@ typedef enum { | |
| ZSTD_c_experimentalParam17=1014, | ||
| ZSTD_c_experimentalParam18=1015, | ||
| ZSTD_c_experimentalParam19=1016, | ||
| ZSTD_c_experimentalParam20=1017 | ||
| ZSTD_c_experimentalParam20=1017, | ||
| ZSTD_c_experimentalParam21=1018 | ||
| } ZSTD_cParameter; | ||
|
|
||
| typedef struct { | ||
|
|
@@ -2355,6 +2357,70 @@ ZSTDLIB_STATIC_API size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx, const vo | |
| #define ZSTD_c_repcodeResolution ZSTD_c_experimentalParam19 | ||
| #define ZSTD_c_searchForExternalRepcodes ZSTD_c_experimentalParam19 /* older name */ | ||
|
|
||
| /** | ||
| * Used in conjunction with the `ZSTD_c_constrainWindowForProtocol` option | ||
| * described below. | ||
| */ | ||
| typedef enum { | ||
| /* Equivalent to disabling. */ | ||
| ZSTD_ConstrainWindow_auto = ZSTD_ps_auto, | ||
|
|
||
| /* No equivalent for ZSTD_ps_enable: you need to specify which protocol. */ | ||
|
|
||
| /* No constraints imposed. */ | ||
| ZSTD_ConstrainWindow_disable = ZSTD_ps_disable, | ||
|
|
||
| /* Constrains the window size to comply with the limits imposed on the | ||
| * `zstd` Content-Encoding, specified by RFCs 8878 and 9659. I.e., liimits | ||
| * the window to <= 8MB. */ | ||
| ZSTD_ConstrainWindow_HTTP_Zstd = 3, | ||
|
|
||
| /* Constrains the window size to comply with the limits imposed on the | ||
| * `dcz` Content-Encoding, as specified by the Compression Dictionary | ||
| * Transport protocol. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth mentioning what the actual limit is? |
||
| * | ||
| * Note: as of the writing of this version of zstd, this protocol has not | ||
| * been finalized and published as an RFC. This implementation is written to | ||
| * comply with draft-ietf-httpbis-compression-dictionary-19. | ||
| * | ||
| * The likeliness that the window sizing constraints between this draft and | ||
| * the final document is judged to be low enough that we aren't bothering with | ||
| * an interim enum value representing this draft version, which would impose | ||
| * almost certainly needless migration burden once the RFC is published. | ||
| * However, the macro `ZSTD_CONSTRAINWINDOW_HTTP_DCZ_DRAFT_VERSION` is defined | ||
| * during the transition to help libraries validate that they have the | ||
| * behavior they expect. It will be updated and then removed once the draft | ||
| * is published as an RFC. */ | ||
| ZSTD_ConstrainWindow_HTTP_DCZ = 4 | ||
| } ZSTD_ConstrainWindow_e; | ||
|
|
||
| #define ZSTD_CONSTRAINWINDOW_HTTP_DCZ_DRAFT_VERSION 19 | ||
|
|
||
| /** | ||
| * ZSTD_c_constrainWindowForProtocol | ||
| * | ||
| * This option constrains the window size of compressions to comply with limits | ||
| * imposed by protocols that use Zstandard. This option clamps the selected | ||
| * window size of a compression into the range allowed by the selected protocol. | ||
| * If the window size a compression would use is already inside that range, | ||
| * this option will not change the resolved window size. (E.g., if a compression | ||
| * would use a 1 MB window, selecting `ZSTD_ConstrainWindow_HTTP_Zstd` won't | ||
| * *raise* the window size to the 8 MB limit imposed by that protocol). | ||
| * | ||
| * This is intended to be a convenience option, and save users the burden of | ||
| * implementing these constraints themselves, because doing that properly would | ||
| * not be simple. In normal operation with only a compression level selected by | ||
| * the user, zstd has non-trivial internal logic to select a window size. The | ||
| * user would have to reimplement that logic and then apply the protocol's | ||
| * limits. This option lets zstd continue to use its internal logic to resolve | ||
| * an appropriate window size and then applies the protocol constraint. | ||
| * | ||
| * The currently understood protocols, and the values that should be used to | ||
| * indicate them, are defined and described in the `ZSTD_ConstrainWindow_e` | ||
| * enum. | ||
| */ | ||
| #define ZSTD_c_constrainWindowForProtocol ZSTD_c_experimentalParam21 | ||
|
|
||
|
|
||
| /*! ZSTD_CCtx_getParameter() : | ||
| * Get the requested compression parameter value, selected by enum ZSTD_cParameter, | ||
|
|
||
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| #!/bin/sh | ||
| datagen -g256M > file | ||
|
|
||
| zstd --long=30 --single-thread --constrain-window=http-zstd -f < file > file.zst | ||
| zstd -l -v file.zst | ||
|
|
||
| zstd --long=30 --single-thread --constrain-window=http-dcz -f < file > file.zst | ||
| zstd -l -v file.zst | ||
|
|
||
| cp file dict | ||
| zstd --long=30 --single-thread --constrain-window=http-dcz --patch-from dict -f file | ||
| zstd -l -v file.zst | ||
|
|
||
| rm dict file file.zst |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| ... | ||
| Window Size: 8.00 MiB (8388608 B) | ||
| ... | ||
| Window Size: 8.00 MiB (8388608 B) | ||
| ... | ||
| Window Size: 128 MiB (134217728 B) | ||
| ... |
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, what is the difference between "auto" and "disable", they seem basically the same? Do you see a situation where they aren't the same in the future?
The command line option also only has "none".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're just here by analogy to the
ZSTD_paramSwitch_eenum, which is used for most cctx param options. They behave identically.