Bump zstd 1.5.0 #26
Bump zstd 1.5.0 #26
Conversation
|
@thewtex thanks for this! I'll need to review the updates here in a bit more detail, and will get to this PR as soon as I can. |
|
|
||
| ```shell | ||
| emcc zstddeclib.c -Oz -s EXPORTED_FUNCTIONS="['_ZSTD_decompress', '_ZSTD_findDecompressedSize', '_ZSTD_isError', '_malloc', '_free']" -Wl,--no-entry -s ALLOW_MEMORY_GROWTH=1 -s MALLOC=emmalloc -o zstddec.wasm | ||
| base64 -w 0 zstddec.wasm > zstddec.txt |
There was a problem hiding this comment.
What is the -w 0 flag? I don't have that option (macOS Ventura), maybe we have different versions of this utility?
There was a problem hiding this comment.
On Linux it prevents newline's
-w, --wrap=COLS wrap encoded lines after COLS character (default 76).
Use 0 to disable line wrapping
There was a problem hiding this comment.
I added a note on this in the README.
README.md
Outdated
| ``` | ||
|
|
||
| ```shell | ||
| emcc zstddeclib.c -Oz -s EXPORTED_FUNCTIONS="['_ZSTD_decompress', '_ZSTD_findDecompressedSize', '_ZSTD_isError', '_malloc', '_free']" -Wl,--no-entry -s ALLOW_MEMORY_GROWTH=1 -s MALLOC=emmalloc -o zstddec.wasm |
There was a problem hiding this comment.
Just for my own learning, what is -Wl,--no-entry? I couldn't find this in the emcc help output.
There was a problem hiding this comment.
It is a standard linking flag for not creating the entry function when the program starts up (what gets called before main). This is usually _start, which is also true for WebAssembly, but we do not need it because we are calling the other functions instead, _ZSTD_decompress, etc.
|
|
||
| ```shell | ||
| emcc zstddeclib.c -Oz -s EXPORTED_FUNCTIONS="['_ZSTD_decompress', '_ZSTD_findDecompressedSize', '_ZSTD_isError', '_malloc', '_free']" -Wl,--no-entry -s ALLOW_MEMORY_GROWTH=1 -s MALLOC=emmalloc -o zstddec.wasm | ||
| base64 -w 0 zstddec.wasm > zstddec.txt |
There was a problem hiding this comment.
Unfortunately, the size of the build is roughly doubled after the update. Any idea what might be the cause of that?
There was a problem hiding this comment.
Hmm, not sure why that is case.
There was a problem hiding this comment.
I'm afraid this may be a blocker -- I don't know if I can justify updating the dependency with that size difference. Many users of the package are sensitive to bundle size.
There was a problem hiding this comment.
I pushed improved flags and also backed down on the version. Here is what I found:
The size of the resulting zstddec.wasm increases in subsequent versions:
1.5.0: 32k
1.5.1: 40k
1.5.5: 52k
package.json
Outdated
| "require": "./dist/zstddec.cjs", | ||
| "default": "./dist/zstddec.modern.js" | ||
| }, | ||
| "main": "dist/zstddec.cjs", |
There was a problem hiding this comment.
I'll pull these package.json changes out into another PR, so you don't need to worry about them here.
There was a problem hiding this comment.
thewtex
force-pushed
the
bump-zstd-1.5.5
branch
from
d8cf9db
to
f4e3b9b
Compare
thewtex
force-pushed
the
bump-zstd-1.5.5
branch
from
f4e3b9b
to
e4d20d4
Compare
|
rebased |
There is now a single script available, ./create_single_file_decoder.sh that does the desired work. Add additional emscripten flags that make explicit our build type and improve the build. `-DNDEBUG=1` is particularly import for our size. These flags also removed the need for the wasi shims.
The size of the resulting zstddec.wasm increases in subsequent versions: 1.5.0: 32k 1.5.1: 40k 1.5.5: 52k
|
Something is wrong with the last commit? The tests are failing: I cannot seem to reproduce the previous working version without the recent changes. @donmccurdy would you mind please trying the build? |
This results in: RuntimeError: memory access out of bounds as discussed in donmccurdy#13.
As discussed in #13 , this is weirdly unreliably unless |
No description provided.