-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Use -Oz instead of -O2 to reduce compiled asset sizes significantly #479
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
Conversation
This reduces the chance that another dev will forget to do this.
Changes required: * Defined EM_NODE_JS environment variable to get rid of a warning that appears if the NODE environment variable is set, but EM_NODE_JS is not. * EXTRA_EXPORTED_RUNTIME_METHODS is now EXPORTED_RUNTIME_METHODS * No longer pass the `-s LINKABLE=1` option to emcc when compiling. (This is a linktime setting and emcc warns about not using a linktime setting when compiling now).
Sqlite now publishes the hash as a SHA3 instead of SHA1, necessitating the installation of the sha3sum command.
This reduces most compilation output by around 50%
On a slightly related note, I would also like to improve our default SQLite compilation options to adopt most of these official recommendations: https://www.sqlite.org/compile.html
note the recommended-compile-option SQLITE_USE_ALLOCA might be worse for both filesize and performance according to this article - http://troubles.md/the-stack-is-not-the-stack/
but wow, what was reported is awesome
These results are very encouraging ! Let's run a performance tests, and if there is no significant performance degradation, let's merge !
Thanks for that warning @kaizhu256! I didn't realize that, and will definitely keep that in mind!
rhashimoto
commented
Jan 8, 2022
I just experimented with switching from -O3 to -Oz in my own SQLite project where the WASM build behavior should be nearly identical. My .wasm size improved from 987964 to 495761, so comparable to what you have.
I have my own benchmark page, and I have attached results for both builds on my local machine. The "default" column would be closest to the expected SQL.js results. Neither is consistently faster so I think most of the difference is just noise.
So I concur that using -Oz yields pretty substantial space savings with little to no performance cost on essentially the same code base.
Nice! This set of benchmarks, or something like it, was exactly what I wanted to see. Thanks for putting it together. This meets the bar as far as I'm concerned. (50% space savings, near-equivalent perf).
I motion to merge it. @lovasoa - would you be up for clicking the button? (On a related note, I am happy to help maintain as well, and there might be some others here that are interested?)
...ql-js#479) * Run `npm ci` automatically after the devcontainer is created. This reduces the chance that another dev will forget to do this. * Upgrade to Emscripten 2.0.29 Changes required: * Defined EM_NODE_JS environment variable to get rid of a warning that appears if the NODE environment variable is set, but EM_NODE_JS is not. * EXTRA_EXPORTED_RUNTIME_METHODS is now EXPORTED_RUNTIME_METHODS * No longer pass the `-s LINKABLE=1` option to emcc when compiling. (This is a linktime setting and emcc warns about not using a linktime setting when compiling now). * Upgrade to Sqlite 3.36.0 Sqlite now publishes the hash as a SHA3 instead of SHA1, necessitating the installation of the sha3sum command. * Use -Oz optimizations instead of -O2 This reduces most compilation output by around 50%
Uh oh!
There was an error while loading. Please reload this page.
(Based on PR #478, so the diff of this is actually minimal - just a change to using
-Ozinstead of-O2)This reduces the compiled size of most assets by about 50% :
image
To specify the result I'm happiest about: The .wasm file went from
1,108,328 bytesto593,151 bytes.Zipped it went from
442,420 bytesto298,116 bytes.(I included the .zip version of each file to represent what this is more likely to look like across the wire.)
For details on optimization settings: https://clang.llvm.org/docs/CommandGuide/clang.html
We were previously running with
-O2:-O2 Moderate level of optimization which enables most optimizations.This switches to
-Oz:-Oz Like -Os (and thus -O2), but reduces code size further.In rudimentary perf testing I did (long, long ago), it was also fast (as fast as the -O2 option as I recall). However, at the time, I noted that the -Oz file took 4x as long to load the .wasm file as the -O2 file did.
Out of date notes on my experiments here in the Makefile: https://github.com/Taytay/sql.js/pull/1/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R213-R378
Before we merge this, I think we need a better benchmarking script to ensure we aren't about to doom everyone to a massive perf regression. I can dig up my old script as a starting point, or perhaps we can use what jlongster wrote to benchmark: #447 (comment)
If anyone has a suggestion on a benchmarking script to use, I'm all ears.
On a slightly related note, I would also like to improve our default SQLite compilation options to adopt most of these official recommendations: https://www.sqlite.org/compile.html
That slightly improves file sizes and runtimes as well. However, we should do that in a separate PR, as that has the greater chance of breaking existing deployments.