[wasm-mt] Use asset loading for dotnet.worker.js; update WasmAppBuilder #73697

lambdageek · 2022-08-10T15:23:28Z

Enables using asset loading to get the dotnet.worker.js file that provides the emscripten pthread worker code.
Also allows specifying the number of pre-allocated workers that will be created at startup using MSBuild properties.

Fixes #68509 and fixes #68397 and fixes #72606

Override Emscripten PThread.allocateUnusedWorker
We want to use our own allocateUnusedWorker because we want to load
dotnet.worker.js using our asset loading machinery.

Unfortunately, Emscripten first calls allocateUnusedWorker very early (from
PThread.init) to pre-allocate the pthread worker pool.

So we set Emscripten's own pthread worker pool to size 0 and make our own. This requires calling loadWasmModuleToWorker during our startup because Emscripten deletes their code that normally does it (in "receiveInstance" in "createWasm" in "emscripten/src/preamble.js") when the pthread pool size is 0.

Also added a pthreadPoolSize field to MonoConfig to allow specifying the initial pthread pool size in mono-config.json
Add IncludeThreadsWorker and PThreadPoolSize props to WasmAppBuilder

IncludeThreadsWorker adds the "js-module-threads" asset to the mono-config.json

PThreadPoolSize can be -1 or >=0 to specify the number of workers that will be pre-allocated at startup for the pthread worker pool. -1 means use the default compiled into dotnet.js
Reorganize the pthreads TS code by moving Internals (access API that digs through Emscripten's pthreads implementation) to its own module. And add types.
Replace emscripten's allocateUnusedWorker function with our own that goes through the asset loading API.
Update samples
Set up console proxying for the workers.
This is done by sending a message from the main thread to the pthread workers with the current MonoConfig on our
dedicated channel. (This means the proxying is setup asynchronously, so if the worker is busy before it receives the message, it may not start redirecting messages right away).

We want to use our own allocateUnusedWorker because we want to load `dotnet.worker.js` using our asset loading machinery. Unfortunately, Emscripten first calls allocateUnusedWorker very early (from `PThread.init`) to pre-allocate the pthread worker pool. So we set Emscripten's own pthread worker pool to size 0 and make our own. This requires calling `loadWasmModuleToWorker` during our startup because Emscripten deletes their code that normally does it (in "receiveInstance" in "createWasm" in "emscripten/src/preamble.js") when the pthread pool size is 0. Also added a pthreadPoolSize field to MonoConfig to allow specifying the initial pthread pool size in mono-config.json

IncludeThreadsWorker adds the js-module-threads asset to the mono-config PThreadPoolSize can be -1 or >=0 to specify the number of workers that will be pre-allocated at startup for the pthread worker pool. -1 means use the default compiled into dotnet.js

and add types

Makes the WasmAppBuilder include the threads worker module

ghost · 2022-08-10T15:24:14Z

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Enables using asset loading to get the dotnet.worker.js file that provides the emscripten pthread worker code.
Also allows specifying the number of pre-allocated workers that will be created at startup using MSBuild properties.

Fixes #68509 and #68397

Override Emscripten PThread.allocateUnusedWorker
We want to use our own allocateUnusedWorker because we want to load
dotnet.worker.js using our asset loading machinery.

Unfortunately, Emscripten first calls allocateUnusedWorker very early (from
PThread.init) to pre-allocate the pthread worker pool.

So we set Emscripten's own pthread worker pool to size 0 and make our own. This requires calling loadWasmModuleToWorker during our startup because Emscripten deletes their code that normally does it (in "receiveInstance" in "createWasm" in "emscripten/src/preamble.js") when the pthread pool size is 0.

Also added a pthreadPoolSize field to MonoConfig to allow specifying the initial pthread pool size in mono-config.json
Add IncludeThreadsWorker and PThreadPoolSize props to WasmAppBuilder

IncludeThreadsWorker adds the "js-module-threads" asset to the mono-config.json

PThreadPoolSize can be -1 or >=0 to specify the number of workers that will be pre-allocated at startup for the pthread worker pool. -1 means use the default compiled into dotnet.js
Reorganize the pthreads TS code by moving Internals (access API that digs through Emscripten's pthreads implementation) to its own module. And add types.
Replace emscripten's allocateUnusedWorker function with our own that goes through the asset loading API.
Update samples

Author:	lambdageek
Assignees:	lambdageek
Labels:	`arch-wasm`, `area-Build-mono`
Milestone:	-

lambdageek · 2022-08-10T15:34:02Z

/azp run runtime-wasm

azure-pipelines · 2022-08-10T15:34:13Z

Azure Pipelines successfully started running 1 pipeline(s).

src/mono/wasm/runtime/startup.ts

lambdageek · 2022-08-11T15:19:57Z

/azp run runtime-wasm

azure-pipelines · 2022-08-11T15:20:07Z

Azure Pipelines successfully started running 1 pipeline(s).

lambdageek · 2022-08-11T15:20:38Z

@pavelsavara @radical any more feedback? Anything for WasmApp.targets or the Task?

src/mono/wasm/runtime/pthreads/shared/emscripten-replacements.ts

Co-authored-by: Marek Fišera <mara@neptuo.com>

radical

I'm not very familiar with src/mono/wasm/runtime, so I can't much review those changes. Rest of it looks fine. I would suggest running runtime-wasm though, and if the tests pass then it should be good to merge 👍

src/tasks/WasmAppBuilder/WasmAppBuilder.cs

radical · 2022-08-13T14:10:12Z

/azp run runtime-wasm

azure-pipelines · 2022-08-13T14:10:25Z

Azure Pipelines successfully started running 1 pipeline(s).

pavelsavara · 2022-08-13T15:20:28Z

src/mono/wasm/runtime/pthreads/shared/emscripten-replacements.ts

+    const asset = resolve_asset_path("js-module-threads");
+    const uri = asset.resolvedUrl;
+    mono_assert(uri !== undefined, "could not resolve the uri for the js-module-threads asset");
+    const worker = new Worker(uri);


we should subscribe onError here

and maybe proxy the console in the worker, similar way how it was done in crypto worker. See how we passed the json.stringify(runtimeHelpers.config) and then only forward console when enabled.

we should subscribe onError here

Emscripten installs an onerror handler of their own that will overwrite ours. we can set one up later (in afterLoadWasmModuleToWorker) but we should probably not use the worker onerror handler for anything that we want to handle - we should catch exceptions around our code and post it using our custom message channel. We have a task to do that in the tracking issue.

and maybe proxy the console in the worker, similar way how it was done in crypto worker. See how we passed the json.stringify(runtimeHelpers.config) and then only forward console when enabled.

ok, i'll see if I can add that over the weekend; if not, there's a task to do it in the tracking issue

pavelsavara · 2022-08-13T15:24:44Z

src/mono/wasm/wasm.proj

@@ -125,7 +125,7 @@
      <_EmccCommonFlags Condition="'$(WasmEnableSIMD)' == 'true'" Include="-msimd128" />
      <_EmccCommonFlags Condition="'$(MonoWasmThreads)' == 'true'" Include="-s USE_PTHREADS=1" />
      <_EmccLinkFlags Condition="'$(MonoWasmThreads)' == 'true'" Include="-Wno-pthreads-mem-growth" />
-      <_EmccLinkFlags Condition="'$(MonoWasmThreads)' == 'true'" Include="-s PTHREAD_POOL_SIZE=4" />
+      <_EmccLinkFlags Condition="'$(MonoWasmThreads)' == 'true'" Include="-s PTHREAD_POOL_SIZE=0" />


i don't know if this is the case when the flag would be written into .rsp file in workload and could not be switched back on customer machine. just asking. And maybe it's OK

Yea if they use wasm-tools, they will inherit this flag. I'm not sure if it's possible to override it.

src/mono/wasm/runtime/startup.ts

pavelsavara

lgtm

Co-authored-by: Ankit Jain <radical@gmail.com>

use a new MonoThreadMessageApplyMonoConfig message to send the MonoConfig from the main thread to each worker when the workers set up the communication channel to the main thread. then if the diagnosticTracing property is true, redirect the worker console logging to a websocket. Fixes dotnet#72606

lambdageek · 2022-08-13T20:11:05Z

Added console proxying. verified that it gets set up in the browser-threading sample and the worker messages are going through the proxy. Did not try running via xharness, though, but i assume that part works

lambdageek · 2022-08-13T20:13:51Z

/azp run runtime-wasm

azure-pipelines · 2022-08-13T20:14:00Z

Azure Pipelines successfully started running 1 pipeline(s).

lambdageek · 2022-08-14T00:39:01Z

Failures seem to be #73901.

lambdageek and others added 5 commits August 10, 2022 00:23

Move emscripten PThread internals access to a separate module

e12de7b

and add types

Load js-module-threads asset in replacement allocateUnusedWorker

a0e71e4

Update samples to explicitly enable threading / perftracing

5db8e8c

Makes the WasmAppBuilder include the threads worker module

lambdageek requested review from radical, lewing and pavelsavara as code owners August 10, 2022 15:23

dotnet-issue-labeler bot added the area-Build-mono label Aug 10, 2022

ghost assigned lambdageek Aug 10, 2022

lambdageek added the arch-wasm WebAssembly architecture label Aug 10, 2022

lambdageek mentioned this pull request Aug 10, 2022

[wasm-mt] Don't hardcode PTHREAD_POOL_SIZE #68397

Closed

tighten up Internals types

af2b21a

pavelsavara reviewed Aug 10, 2022

View reviewed changes

src/mono/wasm/runtime/startup.ts Show resolved Hide resolved

Merge remote-tracking branch 'origin/main' into wasm-mt-assets

f1c7d59

maraf reviewed Aug 12, 2022

View reviewed changes

src/mono/wasm/runtime/pthreads/shared/emscripten-replacements.ts Outdated Show resolved Hide resolved

lambdageek and others added 2 commits August 12, 2022 13:47

apply review feedback

0a8357b

Co-authored-by: Marek Fišera <mara@neptuo.com>

fix import

e655a46

radical approved these changes Aug 13, 2022

View reviewed changes

src/tasks/WasmAppBuilder/WasmAppBuilder.cs Outdated Show resolved Hide resolved

src/tasks/WasmAppBuilder/WasmAppBuilder.cs Outdated Show resolved Hide resolved

pavelsavara reviewed Aug 13, 2022

View reviewed changes

src/mono/wasm/runtime/startup.ts Show resolved Hide resolved

pavelsavara approved these changes Aug 13, 2022

View reviewed changes

lambdageek and others added 2 commits August 13, 2022 14:39

Apply suggestions from code review

8c686a2

Co-authored-by: Ankit Jain <radical@gmail.com>

lambdageek mentioned this pull request Aug 13, 2022

[wasm-mt] Hook up webworker console to xharness websockets #72606

Closed

lambdageek merged commit 88ba045 into dotnet:main Aug 14, 2022

CarnaViire mentioned this pull request Aug 24, 2022

[QUIC] Abort on cancellation throws QUIC_STATUS_INVALID_PARAMETER #73688

Closed

ghost locked as resolved and limited conversation to collaborators Sep 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[wasm-mt] Use asset loading for dotnet.worker.js; update WasmAppBuilder #73697

[wasm-mt] Use asset loading for dotnet.worker.js; update WasmAppBuilder #73697

lambdageek commented Aug 10, 2022 •

edited

Loading

ghost commented Aug 10, 2022

lambdageek commented Aug 10, 2022

azure-pipelines bot commented Aug 10, 2022

lambdageek commented Aug 11, 2022

azure-pipelines bot commented Aug 11, 2022

lambdageek commented Aug 11, 2022

radical left a comment

radical commented Aug 13, 2022

azure-pipelines bot commented Aug 13, 2022

pavelsavara Aug 13, 2022

pavelsavara Aug 13, 2022

lambdageek Aug 13, 2022

lambdageek Aug 13, 2022

pavelsavara Aug 13, 2022

lambdageek Aug 13, 2022

pavelsavara left a comment

lambdageek commented Aug 13, 2022

lambdageek commented Aug 13, 2022

azure-pipelines bot commented Aug 13, 2022

lambdageek commented Aug 14, 2022

[wasm-mt] Use asset loading for dotnet.worker.js; update WasmAppBuilder #73697

[wasm-mt] Use asset loading for dotnet.worker.js; update WasmAppBuilder #73697

Conversation

lambdageek commented Aug 10, 2022 • edited Loading

ghost commented Aug 10, 2022

lambdageek commented Aug 10, 2022

azure-pipelines bot commented Aug 10, 2022

lambdageek commented Aug 11, 2022

azure-pipelines bot commented Aug 11, 2022

lambdageek commented Aug 11, 2022

radical left a comment

Choose a reason for hiding this comment

radical commented Aug 13, 2022

azure-pipelines bot commented Aug 13, 2022

pavelsavara Aug 13, 2022

Choose a reason for hiding this comment

pavelsavara Aug 13, 2022

Choose a reason for hiding this comment

lambdageek Aug 13, 2022

Choose a reason for hiding this comment

lambdageek Aug 13, 2022

Choose a reason for hiding this comment

pavelsavara Aug 13, 2022

Choose a reason for hiding this comment

lambdageek Aug 13, 2022

Choose a reason for hiding this comment

pavelsavara left a comment

Choose a reason for hiding this comment

lambdageek commented Aug 13, 2022

lambdageek commented Aug 13, 2022

azure-pipelines bot commented Aug 13, 2022

lambdageek commented Aug 14, 2022

lambdageek commented Aug 10, 2022 •

edited

Loading