[AUDIO_WORKLET] Wait for Wasm instance to start using audio worklet.#24778
[AUDIO_WORKLET] Wait for Wasm instance to start using audio worklet.#24778brendandahl merged 3 commits intoemscripten-core:mainfrom
Conversation
sbc100
left a comment
There was a problem hiding this comment.
How is this solved in the regular runtime?
|
Regular runtime instantiates on the worker synchronously. We could potentially do the same for minimal, but it will remove some of the shared instantiation code. |
src/postamble_minimal.js
Outdated
| var wasmModule; | ||
| #endif | ||
|
|
||
| var instantiatePromise; |
There was a problem hiding this comment.
How about declaring this in src/audio_worklet.js
Also, the instantiatePromise = usage below should be wrapped in if AUDIO_WORKET I guess? Although no code size tests were effected maybe because closure can delete this? Still best not to generate it in the first place though.. especially with MINIMAL_RUNTIME.
There was a problem hiding this comment.
I'd prefer to leave it as is since adding more ifdefs is going to make this even more unreadable. Closure/rollup/webpack are all able to get rid of unused variables. Are people running minimal runtime without some minification step?
There was a problem hiding this comment.
IIRC the goal of minimal runtime is to have the default output be minimal, not depend on DCE steps
@juj, is that correct?
There was a problem hiding this comment.
Yes absolutely, it is definitely something that MINIMAL_RUNTIME should achieve: smallest output possible.
I can understand that the #ifdef jungle can look a bit messy, but it is worth it in MINIMAL_RUNTIME philosophy. Also the intent is that we should not rely on CLOSURE_COMPILER to optimize code in the "easy cases" that we can deal with ourselves. This is because not every user is able to utilize closure: it requires adhering to defining a heavier set of internal/external public code boundary with the externs model. So not all users can utilize it.
Note that Closure also has challenges of optimizing away variables at global scope.
Fixes intermittent test failures where the AudioWorklet was used before the Wasm module was loaded in MINIMAL_RUNTIME.
ffd3bc5 to
3f0ea0e
Compare
| // the node constructor's "processorOptions" field, we can share the necessary | ||
| // bootstrap information from the main thread to the AudioWorkletGlobalScope. | ||
|
|
||
| var instantiatePromise; |
There was a problem hiding this comment.
#if MINIMAL_RUNTIME here too for consistency.
I'm hoping we can find a nice way to do this one day..
This is an automatic change generated by tools/maint/rebaseline_tests.py. The following (2) test expectation files were updated by running the tests with `--rebaseline`: ``` test/code_size/audio_worklet_wasm.js updated code_size/test_minimal_runtime_code_size_audio_worklet.json: 5689 => 5703 [+14 bytes / +0.25%] Average change: +0.25% (+0.25% - +0.25%) ```
…mscripten-core#24778) Fixes intermittent test failures where the AudioWorklet was used before the Wasm module was loaded in MINIMAL_RUNTIME.
Fixes intermittent test failures where the AudioWorklet was used before the Wasm module was loaded in MINIMAL_RUNTIME.