-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Avoid creating JS symbols for symbols only used in dynamic linking #21785
Avoid creating JS symbols for symbols only used in dynamic linking #21785
Conversation
After this change the section of the generated file that deal with exmaple export looks like this:
Prior to this change there are 7993 lines like this:
Note that not only does it include all these extra symbols, including the very long C++ ones, but that these lines cannot be DCE's because they are exported on the Module object. |
My alternative reason for wanting to do this is to avoid |
0bfea47
to
216552f
Compare
Depends on #21784 |
What is the cost of running wasm-ld twice? It seems like in LTO mode that could be very expensive. If so perhaps we can add a flag to wasm-ld to emit the information we need in a single run? |
I added a TODO regarding that. Brenden and I have been looking into ways to allow the linker to preserve this information, but its non-trivial. The cost of wasm-ld is generally low (compared to say wasm-opt), but you are right that LTO is the exception. However, I think this change is still worth to since it reduces code size my so much, and folks doing LTO are already opting into slowing link times in the hope of getting smaller binaries. Also, remember that this only effects |
216552f
to
5dd1967
Compare
Ooops, I noticed that I forgot to upload the patch with the TODO in it. |
The code size saving here is pretty huge for the JS file: 3.5mb -> 0.5mb:
|
Actually the impact of link time, even with LTO, is not so great, since the first link is done without
Here you can see the actual link take 12 seconds and the "pre-link" that I do only takes 1.5 (i'm assuming since its only compiling what is needed by the core set of exports and the EMSCRIPTEN_KEEPALIVE functions). |
5dd1967
to
fcb1066
Compare
This change should be much easier to review now that #21784 has landed. I think it make |
fcb1066
to
f2db899
Compare
Updated. I even remembered to use a separate commit for the feedback ! |
In your example I'm guessing most of the code could be optimized out? But imagine a large game engine where most of it ends up in the final binary either way (tons of indirect calls, little can be DCE'd). Or, imagine one of the various corner cases that cause very slow LTO, like optimizing SQLite for example (unless LLVM fixed that) - it was almost a minute when I last measured it iirc, for just one function. This change would double such potentially long LTO times, which worries me. I guess an argument could be that LTO is meant to be slow anyhow, and we can tolerate additional slowdowns for better code, which this provides. But a potentially 2x slowdown to the llvm LTO part still seems quite steep compared to other slowdowns we usually consider. I wonder if we can mitigate or document it somehow. |
But I don't think there are many users of |
I'll write the ChangeLog entry regarding this.. we can revisit if we get reports of folks being negatively effected. (And hopefully we can fix the TODO when we get time regardless) |
ceacfdf
to
6f1a769
Compare
Yeah, makes sense that such a big code size win might be worth this risk. But I agree that documenting this is safer, as then it's easier for people (and us) to triage future reports of "LTO is slow in this release". |
edeb8a4
to
a66d170
Compare
Symbols that are exported using EMSCRIPTEN_KEEPALIVE are supposed to be exported to the outside world (i.e. on the Module object) and also be available to call JS within the module. Symbols exports for the purposed of dynamic linking so not need to be exported on the Module and are added (at runtime) to `wasmImports` which acts as a kind of global symbol table for the program. In in the case of `-sMAIN_MODULE=1` we export *all* symbols from all libraries, and prior to this change it was not possible to distingish between all the exported generated because of `--export-dynamic`, and the exports generated due to `EMSCRIPTEN_KEEPALIVE`. This change allows us to differentiate by running `wasm-ld` twice: once without `--export-dynamic` (to get the smaller list of `EMSCRIPTEN_KEEPALIVE`) and then once with `--export-dynamic` to produce the actual wasm that we output. This takes the list of exports that we turn in to JS globals from 7993 to 28, massively reducing the overhead of `-sMAIN_MODULE=1`.
a66d170
to
61f9a9e
Compare
Just curious, what is the difference between this and MAIN_MODULE=2? |
|
Symbols that are exported using EMSCRIPTEN_KEEPALIVE are supposed
to be exported to the outside world (i.e. on the Module object) and
also be available to call JS within the module.
Symbols exports for the purposed of dynamic linking so not need to be
exported on the Module and are added (at runtime) to
wasmImports
whichacts as a kind of global symbol table for the program.
In in the case of
-sMAIN_MODULE=1
we export all symbols fromall libraries, and prior to this change it was not possible to
distingish between all the exported generated because of
--export-dynamic
, and the exports generated due toEMSCRIPTEN_KEEPALIVE
.This change allows us to differentiate by running
wasm-ld
twice: oncewithout
--export-dynamic
(to get the smaller list ofEMSCRIPTEN_KEEPALIVE
) and then once with--export-dynamic
toproduce the actual wasm that we output.
This takes the list of exports that we turn in to JS globals from 7993
to 28, massively reducing the overhead of
-sMAIN_MODULE=1
.