Repository navigation
Conversation
Previously, we treated all modules items in the same way: if a module
item was used in the primary module or multiple secondary modules, we
put the item in the primary module and exported it from there, and
secondary modules using the item imported it. So
1. When an item was used in both the primary and secondary modules, it
was placed in the primary module and exported from there, and
imported from the secondary modules
2. When an item was not used in the primary module but used in multiple
secondary modules, it was still placed in the primary module and
exported from there, and imported from the secondary modules
But in case of immutable globals, we can have as many copies of the same
global as possible, and we can just copy them to the secondary modules.
So whether it is used in the primary module or multiple secondary
modules, each module that uses it has its own copy. This reduces the
primary module size because, in case of 1, we don't need to put the
global in the primary module in the first place. Also in case of 2, we
don't need to add an export of a global to the primary module.
This is a trade-off and may increase the secondary module sizes instead.
But given that the main goal of using wasm-split is mostly to reduce the
loading time of the primary module, I think this trade-off is worth it.
In case of merge-opt-split workflow, after interprocedural
optimizations, many globals that used to be only used in a single
secondary module gets used in multiple secondary modules, and thus are
placed in the primary module after splitting, increasing the primary
module size. This undoes this effect.
Note that this also copy immutable global imports to secondary modules
whenever it is possible, because we didn't exclude them in
`shareElement`.
---
Results
- acx_gallery
- Input
- Total: 23648k
- Primary: 1337k
- Current
- Total: 22240k
- Primary: 1579k
- This PR
- Total: 22884k
- vs. Current: +2.8%
- vs. Input: -3.2%
- Primary: 1348k
- vs. Current: -14.6%
- vs. Input: +0.8%
- essentials
- Input
- Total: 83384k
- Primary: 5600k
- Current
- Total: 77060k
- Primary: 6396k
- This PR
- Total: 79624k
- vs. Current: +3.3%
- vs. Input: -4.5%
- Primary: 5715k
- vs. Current: -10.6%
- vs. Input: +2.0%
Compared to the current code (without this patch), we can see this
reduces the primary module size significantly (-14.6% and -10.6% each)
at the expense of slight increase of the total combined module size
(+3-4%).
But compared to the original input, this still does not reduce the
primary module size (+1-2%), while it decreases the combined
module size slightly (-3-4%).
The measurements were done with with 07/2026 version of Dart apps, and
after removing all (internal) exports that were not marked as
`@binaryen.js.called`. (I have a pass that does this in my local
machine, but I haven't uploaded it yet)
There was a problem hiding this comment.
We need to make sure we don't copy immutable globals whose initializers contain Properties::isGenerative expressions (such as struct.new) because there would be an observable difference (e.g. struct identity tested with ref.eq) between the values in the copies.
Done: 9a0b05d |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // there would be an observable difference (e.g. struct identity tested with | ||
| // ref.eq) between the values in the copies. | ||
| return !global->mutable_ && | ||
| (!global->init || |
There was a problem hiding this comment.
[HIGH | Confidence: 5/5 | BUG]
In canCopyGlobal(), checking (!global->init || !Properties::isGenerative(...)) causes imported immutable globals to be treated as copyable because imported globals have global->init == nullptr.
When an imported immutable global (such as (global $base (import "env" "base") i32)) is shared between the primary module and a secondary module, shareImportableItems() enters the isCopyableGlobal branch instead of calling makeImportExport(). As a result, the external import definition is duplicated into the secondary module ((import "env" "base" ...)), as seen in the updated test dispatch-table-base-global-used-elsewhere.wast, rather than being exported by the primary module and imported from config.importNamespace ((import "primary" "global" ...)).
This breaks the wasm-split ABI contract where secondary modules only import dependencies from the primary module (importNamespace, defaulting to "primary"). Host embedders instantiate secondary modules with { [importNamespace]: primaryInstance.exports }, meaning instantiation will fail at runtime with a LinkError or TypeError because the external import module (e.g., "env") is not provided to the secondary module.
Imported globals should not be treated as copyable globals. Update canCopyGlobal to require !global->imported():
| (!global->init || | |
| static bool canCopyGlobal(Global* global, Module& wasm) { | |
| return !global->imported() && !global->mutable_ && | |
| !Properties::isGenerative(global->init, nullptr, wasm); | |
| } |
There was a problem hiding this comment.
It is intended we also move/copy imported globals when possible. If we do this, when an imported global is movable/copyable to secondary module(s) and it is not used in the primary module, the primary module doesn't need an additional import + export for it.
Actually, Emscripten loader currently does not support this at the moment. But AFAIK it doesn't have users now, and the primary users for us now don't use Emscripten loader. And we were already splitting imported globals that were used in a single secondary module before this PR, so this doesn't make it worse on that front. I'll update Emscripten loader separately.
Not having the static is also fine because the code is within an anonymous namespace, but I'll add it given that it's a matter of style.
| // one module would not be visible in others. Immutable globals with | ||
| // generative initializers (e.g. struct.new) also cannot be copied because | ||
| // there would be an observable difference (e.g. struct identity tested with | ||
| // ref.eq) between the values in the copies. |
There was a problem hiding this comment.
It's probably worth calling out that imported globals can be moved.
|
Turns out, preventing generative immutable globals from being copied erases most of the gains this PR achieved 😭 |
Ugh, that's annoying. In principle it would be possible to do an analysis to see which globals could ever flow into a edit: Maybe have Gemini prototype it and measure the impact? |
|
Can we land this anyway? I'll update the PR description. |
tlively
left a comment
tlively
left a comment
There was a problem hiding this comment.
Yeah, sorry, I had meant to approve this along with that last comment.
Previously, we treated all modules items in the same way: if a module item was used in the primary module or multiple secondary modules, we put the item in the primary module and exported it from there, and secondary modules using the item imported it. So
But in case of immutable globals, we can have as many copies of the same global as possible, and we can just copy them to the secondary modules. So whether it is used in the primary module or multiple secondary modules, each module that uses it has its own copy. This reduces the primary module size because, in case of 1, we don't need to put the global in the primary module in the first place. Also in case of 2, we don't need to add an export of a global to the primary module.
This is a trade-off and may increase the secondary module sizes instead. But given that the main goal of using wasm-split is mostly to reduce the loading time of the primary module, I think this trade-off is worth it.
In case of merge-opt-split workflow, after interprocedural optimizations, many globals that used to be only used in a single secondary module gets used in multiple secondary modules, and thus are placed in the primary module after splitting, increasing the primary module size. This undoes this effect.
Note that this also copy immutable global imports to secondary modules whenever it is possible, because we didn't exclude them in
shareElement.Results
acx_gallery
essentials
Compared to the current code (without this patch), we can see this reduces the primary module size significantly (-14.6% and -10.6% each) at the expense of slight increase of the total combined module size (+3-4%).
But compared to the original input, this still does not reduce the primary module size (+1-2%), while it decreases the combined module size slightly (-3-4%).
The measurements were done with with 07/2026 version of Dart apps, and after removing all (internal) exports that were not marked as
@binaryen.js.called. (I have a pass that does this in my local machine, but I haven't uploaded it yet)