[wasm-split] Precompute ownership info (NFC) - #8986
Conversation
Given a module element name, many parts of the code queries for its owning modules (where the module element has to be placed) or secondary modules using that module element. This adds `OwnershipTracker`, which precomputes and manages that information. All calls to `getOwner` or `getUsingSecondaries` that required computations iterating on all secondary modules which can be as many as thousands, has been replaced with a call that simply returns prcomputed information. For the Jul 2026 version of the applications received from the Dart team, this reduces the running time of wasm-split by 17% for acx_gallery (30s -> 25s) and by 33% for essentials (230s -> 153s). Suggested in #8832 (comment).
Previously we removed module elements one by one within a loop. But because `Module` stores a module element in both a map and a vector, removing a single module element using `removeModuleElement` is O(N), because it needs to shift all vector elements after it: https://github.com/WebAssembly/binaryen/blob/302396a676433152a32375a81d71e74687c97a1b/src/wasm/wasm.cpp#L1970-L1979 This removes module elements in bulk using `removeModuleElements`, which does the shifting only once. https://github.com/WebAssembly/binaryen/blob/302396a676433152a32375a81d71e74687c97a1b/src/wasm/wasm.cpp#L2004-L2018 Combining with #8986, acx_gallery's running time improved by 50.3% (30s -> 15s), and essentials by 60.8% (230s -> 90s). (for Jul 2026 version) I guess the main reason for the running time increase in #8441 was this O(N) `removeModuleElement` called within a loop after all.
| // In the initial building phase, we just directly add to a UsedName struct. | ||
| // After OwnershipTracker is constructed, we all its insert() method to update | ||
| // owner modules and using secondary modules correctly. |
There was a problem hiding this comment.
How do these two phases relate to the general process of splitting described in the comment at the top of the file?
There was a problem hiding this comment.
All of shareImportableItems (and computeUsedNames called from shareImportableItems and construction of OwnershipTracker inside computeUsedNames) correspond to this paragraph:
// 4. Export globals, tags, tables, and memories from the primary module and
// import them in the secondary modules. If possible, move those module
// items instead to the secondary modules.The reason ADD_ITEM does two different things is, before we create OwnershipTracker, we build UsedName for each module, and used.field.insert(val); is done in this phase. After OwnershipTracker is created, we add more constraints. But unlike the straightforward first scanning phase this constraints can add one module item to multiple modules, and we need to keep track of which one should be the owner and such. So in this second phase we use tracker->insert.
There was a problem hiding this comment.
So if I understand correctly, the first pass collects the used names and the second pass computes the owners.
What I'm not understanding yet is why we need two passes for this. Can't we just insert used names into the OwnershipTracker in a single pass and have it automatically update the owner to be the primary module as soon as it sees that two different modules have used the same item?
There was a problem hiding this comment.
The reason I used the two-phase thing was the first phase was using ParallelFunctionAnalysis:
binaryen/src/ir/module-splitting.cpp
Lines 704 to 728 in dae272c
and if we use the
OwnershipTracker from the beginning we can't use ParallelFunctionAnalysis anymore.
But is using ParallelFunctionAnalysis really faster? Several months ago I tried to use ParallelFunctionAnalysis for other functions that were not using it and it actually slowed the program down. I just tried to remove it from scanModule, and it apparently makes at least Dart applications on my local machine faster.
I uploaded the removing of ParallelFunctionAnalysis as a separate PR to make diffs clearer: #9007
After this we can remove this two phase and use the tracker from the beginning.
Co-authored-by: Thomas Lively <tlively@google.com>
| #define ADD_ITEM_TO_TRACKER(FIELD, VAL) \ | ||
| tracker.insert(VAL, owner, &OwnershipTracker::FIELD, &UsedNames::FIELD) |
There was a problem hiding this comment.
This macro implicitly using whatever owner happens to be in scope at the use site makes it harder to understand what's going on. Can we pass the owner in explicitly?
Also it's probably possible to use some template overloading magic to do something like tracker.insert<Memory>(segment->memory, owner) without using macros.
| // In the initial building phase, we just directly add to a UsedName struct. | ||
| // After OwnershipTracker is constructed, we all its insert() method to update | ||
| // owner modules and using secondary modules correctly. |
There was a problem hiding this comment.
So if I understand correctly, the first pass collects the used names and the second pass computes the owners.
What I'm not understanding yet is why we need two passes for this. Can't we just insert used names into the OwnershipTracker in a single pass and have it automatically update the owner to be the primary module as soon as it sees that two different modules have used the same item?
Co-authored-by: Thomas Lively <tlively123@gmail.com>
Given a module element name, many parts of the code queries for its owning modules (where the module element has to be placed) or secondary modules using that module element. This adds
OwnershipTracker, which precomputes and manages that information. All calls togetOwnerorgetUsingSecondariesthat required computations iterating on all secondary modules which can be as many as thousands, has been replaced with a call that simply returns prcomputed information.For the Jul 2026 version of the applications received from the Dart team, this reduces the running time of wasm-split by 17% for acx_gallery (30s -> 25s) and by 33% for essentials (230s -> 153s).
Suggested in #8832 (comment).