This repository was archived by the owner on Feb 25, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6k
Compile dart2wasm modules using the JS runtime exported compileStreaming #51488
Merged
Merged
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
a0a8805
Update dart2wasm module compilation to use Wasm string builtins
osa1 5ecec4a
Fix detectWasmStringBuiltin call
osa1 cc677af
Merge remote-tracking branch 'origin/main' into dart2wasm_string_buil…
osa1 8232d85
Remove polyfill, dart2wasm mjs file provides it
osa1 0789214
Revert whitespace
osa1 0ebfbf8
Merge remote-tracking branch 'origin/main' into dart2wasm_string_buil…
osa1 4d70159
Use compileStreaming from JS shim
osa1 4064581
Revert whitespace change
osa1 adac7ec
Merge branch 'main' into dart2wasm_string_builtins
osa1 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 may consider in the future to make the
.mjsfile export 3 things:WebAssembly.compile*WebAssembly.instantiate*(we already provide this)mainAPI (we already provide this)That would encapsulate this logic here in the dart2wasm compiler instead of making a user of the compiler aware of what builtins the compiled code requires.
There's an aspect to think about: The current way allows flutter JS code to be loaded in parallel to the mjs file, then we'd have a dependency from former to the latter - which may harm startup latency. This may especially be noticeable if we use imported strings (which may make the
mjsfile much bigger). So we'd have to think whether it would make sense to do that or not.Anyways, that's not needed right 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.
Why do you want separate compilation and instantiation steps? Currently we don't do anything with the compiled module other than instantiating it. We could combine both steps in a single export.
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 can also offer this in the mjs api:
WebAssembly.compile*andWebAssembly.instantiate*The setup should be optimized for startup time / initial page load. So we should try to do as much in parallel we can. So one advantage of separating the two, is that it allows the browser to download the wasm file & do some work in the streaming compile step in parallel to downloading/parsing/running the mjs file (which may become bigger once we have imported strings).
I don't know if Chrome/V8 does a lot of work in the compile step or most in instantiation step. May also differ between browsers.
This would need to be measured.
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.
If the compile function is provided by the mjs file so you can't compile the Wasm and download mjs file with lots of strings in parallel, right?
Maybe strings can be in a separate file that we download in parallel.