-
Notifications
You must be signed in to change notification settings - Fork 735
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
[wasm-split] Add a multi-split mode #6943
Conversation
Wasm-split generally assumes that calls to secondary functions made before the secondary module has been loaded and instantiated should go to imported placeholder functions that can be responsible for loading the secondary module and forwarding the call to the loaded function. That scheme makes the loading entirely transparent from the application's point of view, which is not always a good thing. Other schemes would make it impossible for a secondary function to be called before the secondary module has been explicitly loaded, in which case the placeholder functions would never be called. To improve code size and simplify instantiation under these schemes, add a new `--no-placeholders` option that skips adding imported placeholder functions.
Add a mode that splits a module into arbitrarily many parts based on a simple manifest file. This is currently implemented by splitting out one module at a time in a loop, but this could change in the future if splitting out all the modules at once would improve the quality of the output.
cc @dschuff FYI |
Module wasm; | ||
parseInput(wasm, options); | ||
|
||
std::map<std::string, std::unordered_set<std::string>> moduleFuncs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not hard to figure these out, but some comments would be nice I think.
} | ||
auto splitResults = ModuleSplitting::splitFunctions(wasm, config); | ||
// TODO: symbolMap, placeholderMap, emitModuleNames | ||
// TODO: Support --emit-text and use .wast in that case. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe error on these for now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our special option validation that checks that each flag is allowed in the selected mode already takes care of that.
// TODO: Support --emit-text and use .wast in that case. | ||
auto moduleName = options.outPrefix + mod + ".wasm"; | ||
PassRunner runner(&*splitResults.secondary); | ||
runner.add("remove-unused-module-elements"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed here but not in single-split?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably add it to the normal split mode as well. In experiments with hundreds of relatively small modules, this was obviously very necessary. I guess it's harder to tell by visual inspection that it is necessary for larger secondary modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Maybe add a TODO for the normal mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just uploaded #6945 to do this for all secondary modules.
Add a mode that splits a module into arbitrarily many parts based on a
simple manifest file. This is currently implemented by splitting out one
module at a time in a loop, but this could change in the future if
splitting out all the modules at once would improve the quality of the
output.