Complete rewrite of XZ handler. #363
Merged
Conversation
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
martonilles
requested changes
May 9, 2022
Contributor
martonilles
left a comment
martonilles
left a comment
There was a problem hiding this comment.
Maybe also add a test file for a multi-stream file?
qkaiser
force-pushed
the
362-fix-xz-chunk
branch
4 times, most recently
from
3906728 to
8352ee0
Compare
martonilles
requested changes
May 13, 2022
Contributor
martonilles
left a comment
martonilles
left a comment
There was a problem hiding this comment.
I would also add a few unit tests at least to cover the stream continuation and padding logic
Also an integration test example for a multi-stream file would be nice
Contributor
Author
|
@martonilles fixed the code following your remarks. I had forgotten to add the test file to my commit :) Let me know what you think. I'll squash the commits when approved. |
martonilles
requested changes
May 16, 2022
Contributor
Author
|
@martonilles no obvious timing differences in pytest runtime (compared with previously merged branches). |
The presence of '#' characters in the ASCII part of legit hexdumps was leading to flaky tests. We fixed this by removing comment parsing in unhex().
In order to emit a single ValidChunk for concatenated XZ compression streams, we converted the handler to use Hyperscan like we did with bzip2 handler (see c970077). The idea is that for a given match, we use Hyperscan to look for end of stream markers after our current start offset. If we have a match, we read the whole chunk in memory and attempt to decompress it. If decompression is successful, we adjust start and end offsets of our context and instruct Hyperscan to continue looking for other end of stream marker matches. Otherwise, we stop the search and return.
martonilles
approved these changes
May 17, 2022
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.
In order to emit a single ValidChunk for concatenated XZ compression streams, we converted the handler to use Hyperscan like we did with bzip2 handler (see c970077).
The idea is that for a given match, we use Hyperscan to look for end of stream markers after our current start offset. If we have a match, we attempt to decompress our candidate chunk using unblob's file iterator (iterate_file) to limit memory consumption. If decompression is successful, we adjust start and end offsets of our context and instruct Hyperscan to continue looking for other end of stream marker matches. Otherwise, we stop the search and return.
Closes #362