modules/zstd: add Zstd decoder skeleton#1169
Closed
rdob-ant wants to merge 12 commits intogoogle:mainfrom
Closed
Conversation
This commit adds rules to build zstd library and exposes the following targets: * `@com_github_facebok_zstd//:libzstd` - zstd library itself * `@com_github_facebok_zstd//:zstd` - binary for performing zstd compression/decompression * `@com_github_facebok_zstd//:decodecorpus` - binary for creating test data for a zstd decoder Internal-tag: #[49966] Signed-off-by: Robert Winkler <[email protected]>
This commit enables building zstd library in version 1.5.5 in the XLS repository. The sources of the library come from the official release of zstd on Meta's GitHub. Internal-tag: #[49966] Signed-off-by: Robert Winkler <[email protected]>
Disable building llvm with zstd to avoid conflicts with newly-added zstd library. It seems that llvm_zstd is not necessary for correct llvm operation. Internal-tag: #[49966] Signed-off-by: Robert Winkler <[email protected]>
This commit adds a DSLX Buffer library that provides the Buffer struct, and helper functions that can be used to operate on it. The Buffer is meant to be a storage for data coming from the channel. It acts like a FIFO, allowing data of any length to be put in or popped out of it. Provided DSLX tests verify the correct behaviour of the library. Internal-tag: [#50221] Signed-off-by: Robert Winkler <[email protected]>
Internal-tag: [#50221] Signed-off-by: Robert Winkler <[email protected]>
This commit adds the library with functions for parsing a magic number and tests that verify its correctness. Internal-tag: [#50221] Signed-off-by: Robert Winkler <[email protected]>
Internal-tag: [#50221] Signed-off-by: Robert Winkler <[email protected]>
This commit adds the library with functions for parsing a frame header. The provided tests verify the correcness of the library. Internal-tag: [#49967] Signed-off-by: Robert Winkler <[email protected]>
Internal-tag: [#49967] Signed-off-by: Robert Winkler <[email protected]>
Fix module behavior when single_segment is true and FCS size is set as 0. Internal-tag: [#49967] Signed-off-by: Roman Dobrodii <[email protected]>
This commit adds a skeleton of ZSTD decoder that parses a magic number and a frame header. The provided tests examine the model using C++ API, which is a prerequisite for detailed tests using zstd library. Internal-tag: [#50221] Signed-off-by: Robert Winkler <[email protected]>
Internal-tag: [#50221] Signed-off-by: Robert Winkler <[email protected]>
Member
👍 for writing test in C++ that compare against libzstd. |
proppy
reviewed
Oct 26, 2023
| DECODE_BLOCKS = 2, | ||
| } | ||
|
|
||
| struct ZstdDecoderState { |
Member
There was a problem hiding this comment.
can we add inline doc string for each state members?
proppy
reviewed
Oct 26, 2023
| _ => state | ||
| }; | ||
|
|
||
| let state = match state.status { |
Member
There was a problem hiding this comment.
are you keeping the match separte so that you can process the frame header in the same tick? (if yes, is this state even necessary?)
Contributor
There was a problem hiding this comment.
With the current shape of the skeleton, the state is not necessary. However, soon we will extend it with the decoding capabilities that would make use of the internal state
proppy
reviewed
Oct 26, 2023
| const char *send_channel_name = "zstd_dec__input_r"; | ||
|
|
||
| const char *ir_file = "xls/modules/zstd/zstd_dec_opt_ir.opt.ir"; | ||
| const char *zstd_file = "xls/modules/zstd/data.zst"; |
Member
There was a problem hiding this comment.
maybe we could generate this from the text fixture?
Contributor
There was a problem hiding this comment.
Yes! We will add the ability to generate test data
proppy
changed the title
This was referenced Nov 29, 2023
cdleary
added
the
app
Member
|
should we close this and focus on reviewing #1315 ? |
|
The review will happen in #1315, so this one can be closed. |
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
5 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.
This PR adds a skeleton of Zstd decoder that for now only parses a frame header (using DSLX modules added by other Zstd PRs). The module itself is in DSLX, while tests are in C++ as they need to use file IO and will also call into libzstd in the future. As this is a skeleton only, it'll most probably undergo substantial changes when other Zstd parser modules are added.
NOTE: this is based on #1168 , please ignore commits from that branch when reviewing.