Remove lz4_c patch #851
Merged
Remove lz4_c patch #851
Conversation
This file contains 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
We use lz4 to compress files in our virtual filesystem. The lz4_c patch patches emscripten to use a wasm version of lz4 instead of the default javascript version. I don't think this was ever supposed to work, and it definitely does not work with emscripten 1.38.35. The reason is that we use lz4 to manage the filesystem, which is loaded before the emscripten runtime is initialized, and we are not supposed to call wasm functions before the runtime is initialized. The patch was originally included for performance reasons, but it appears that there is in fact not much measurable performance difference - the decompression algorithm is fairly straightforward and presumably not difficult to JIT well.
|
It appears that removing |
|
Thanks, that a good argument to include it in the release. |
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.
We use lz4 to compress files in our virtual filesystem. The lz4_c patch
patches emscripten to use a wasm version of lz4 instead of the default
javascript version.
I don't think this was ever supposed to work, and it definitely does not
work with emscripten 1.38.35. The reason is that we use lz4 to manage
the filesystem, which is loaded before the emscripten runtime is
initialized, and we are not supposed to call wasm functions before the
runtime is initialized.
The patch was originally included for performance reasons, but it
appears that there is in fact not much measurable performance
difference - the decompression algorithm is fairly straightforward and
presumably not difficult to JIT well.