Skip to content
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] Add legs to build wasm with internal threads only and full threading enabled #68787

Merged
merged 13 commits into from
Jun 8, 2022

Conversation

steveisok
Copy link
Member

@steveisok steveisok commented May 2, 2022

This change adds two build-only lanes to runtime and two full build+test lanes to runtime-wasm that builds wasm with internal threads only (WasmEnablePerfTracing) and full threading (WasmEnableThreads).

The runtime-wasm additions can only be manually triggered.

@ghost
Copy link

ghost commented May 2, 2022

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

Temporarily bring up a leg that builds wasm with threading enabled. This will allow us to make sure we aren't easily broken.

Author: steveisok
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@ghost
Copy link

ghost commented May 2, 2022

Tagging subscribers to this area: @directhex
See info in area-owners.md if you want to be subscribed.

Issue Details

Temporarily bring up a leg that builds wasm with threading enabled. This will allow us to make sure we aren't easily broken.

Author: steveisok
Assignees: steveisok
Labels:

area-Infrastructure-mono

Milestone: -

@steveisok steveisok force-pushed the wasm-threading-build branch from c08d03b to ec6532f Compare May 2, 2022 21:28
@steveisok steveisok requested a review from vargaz as a code owner May 2, 2022 22:20
@steveisok steveisok force-pushed the wasm-threading-build branch from 526db1e to c1e0f8f Compare May 2, 2022 22:23
@steveisok
Copy link
Member Author

@lewing I don't see the icu issues locally. is there a version mismatch on ci?

@steveisok steveisok requested a review from marek-safar as a code owner June 1, 2022 19:51
@lambdageek lambdageek force-pushed the wasm-threading-build branch from c2088a0 to e81fbd4 Compare June 1, 2022 19:55
src/mono/wasm/wasm.proj Outdated Show resolved Hide resolved
eng/pipelines/runtime.yml Outdated Show resolved Hide resolved
src/mono/wasm/wasm.proj Outdated Show resolved Hide resolved
src/mono/wasm/wasm.proj Outdated Show resolved Hide resolved
Steve Pfister and others added 7 commits June 6, 2022 17:27
@steveisok
Copy link
Member Author

@radical @lambdageek I updated the description to indicate what we are doing re: CI and threads.

@lambdageek
Copy link
Member

Nice!

From https://helix.dot.net/api/2019-06-17/jobs/d47d55a8-b32a-47e1-9992-47238a47fe96/workitems/Wasm.Browser.ES6.Sample/console

[1.0.0-prerelease.22270.1+9cacd2f874c946a6497110124a2369fb5131c68f] XHarness command issued: wasm test-browser --app=. --browser=Chrome --html-file=index.html --output-directory=/datadisks/disk1/work/A8C608FF/w/B57709C8/uploads/xharness-output -- Wasm.Browser.ES6.Sample.dll
[23:22:38] info: Starting chromedriver with args: --headless --incognito --disable-background-timer-throttling --disable-backgrounding-occluded-windows --disable-renderer-backgrounding --enable-features=NetworkService,NetworkServiceInProcess --allow-insecure-localhost --disable-breakpad --disable-component-extensions-with-background-pages --disable-dev-shm-usage --disable-extensions --disable-features=TranslateUI --disable-ipc-flooding-protection --force-color-profile=srgb --metrics-recording-only
Starting ChromeDriver 100.0.4896.0 (ecbece54737a3df72a99f07de4850663ea5a48d3-refs/heads/main@{#972765}) on port 38931
Only local connections are allowed.
Please see https://chromedriver.chromium.org/security-considerations for suggestions on keeping ChromeDriver safe.
ChromeDriver was started successfully.
[23:22:45] info: WASM ERROR ReferenceError: SharedArrayBuffer is not defined
[23:22:45] info: WASM EXIT 2
[23:22:45] fail: Application has finished with exit code HELP_SHOWN but 0 was expected
XHarness exit code: 71 (GENERAL_FAILURE)

I guess we need to teach xharness to set the right headers for SAB

@radical
Copy link
Member

radical commented Jun 7, 2022

I guess we need to teach xharness to set the right headers for SAB

<WasmXHarnessArgs>$(WasmXHarnessArgs) --web-server-use-cop</WasmXHarnessArgs>

@jeromelaban
Copy link
Contributor

The runtime builds with threading enabled on my side using this PR's patch. Thanks!

@steveisok steveisok force-pushed the wasm-threading-build branch from bf74b08 to 56d0023 Compare June 7, 2022 15:15
@steveisok
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@steveisok
Copy link
Member Author

Leaving this here so I remember:

Need to enable threading in v8:

[16:13:11] fail: requested a shared WebAssembly.Memory but the returned buffer is not a SharedArrayBuffer, indicating that while the browser has SharedArrayBuffer it does not have WebAssembly threads support - you may need to set a flag
[16:13:11] fail: failed to load the dotnet.js file.
[16:13:11] fail: Error: bad memory
[16:13:11] fail:     at ./dotnet.js:12:15206
[16:13:11] fail:     at test-main.js:405:12
[16:13:11] info: Incoming arguments: --run WasmTestRunner.dll System.Buffers.Tests.dll -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing
[16:13:11] info: Application arguments: --run WasmTestRunner.dll System.Buffers.Tests.dll -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing
[16:13:11] info: Process v8 exited with 0

@steveisok
Copy link
Member Author

Looks like System.Buffers passes with a threading runtime:

https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-68787-merge-af53e064493a4e69ac/WasmTestOnBrowser-System.Buffers.Tests/1/console.7881bbbc.log?helixlogtype=result

[16:13:55] info: Discovering: System.Buffers.Tests.dll (method display = ClassAndMethod, method display options = None)
[16:13:55] info: Discovered:  System.Buffers.Tests.dll (found 42 of 46 test cases)
[16:13:55] info: Using random seed for test cases: 330518693
[16:13:55] info: Using random seed for collections: 330518693
[16:13:55] info: Starting:    System.Buffers.Tests.dll
[16:14:00] info: Finished:    System.Buffers.Tests.dll
[16:14:00] info: 
[16:14:00] info: === TEST EXECUTION SUMMARY ===
[16:14:00] info: Total: 79, Errors: 0, Failed: 0, Skipped: 8, Time: 4.23806s

Further evidence in System.Threading.Tests

https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-68787-merge-af53e064493a4e69ac/WasmTestOnBrowser-System.Threading.Tests/1/console.1ed48cce.log?helixlogtype=result

16:14:48] info: Discovering: System.Threading.Tests.dll (method display = ClassAndMethod, method display options = None)
[16:14:49] info: Discovered:  System.Threading.Tests.dll (found 241 of 274 test cases)
[16:14:49] info: Using random seed for test cases: 1646623512
[16:14:49] info: Using random seed for collections: 1646623512
[16:14:49] info: Starting:    System.Threading.Tests.dll
**[16:14:59] warn: Blocking on the main thread is very dangerous, see https://emscripten.org/docs/porting/pthreads.html#blocking-on-the-main-browser-thread**
[16:15:13] info: Finished:    System.Threading.Tests.dll
[16:15:13] info: 
[16:15:13] info: === TEST EXECUTION SUMMARY ===
[16:15:13] info: Total: 457, Errors: 0, Failed: 0, Skipped: 70, Time: 23.6206299s

@radical
Copy link
Member

radical commented Jun 7, 2022

Should we run only a few selected tests for the new jobs?

@radical radical added the arch-wasm WebAssembly architecture label Jun 7, 2022
@ghost
Copy link

ghost commented Jun 7, 2022

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

Issue Details

This change adds two lanes to runtime-wasm that builds wasm with internal threads only (WasmEnablePerfTracing) and full threading (WasmEnableThreads).

The official build will be modified to combine both single threaded and multi-threaded artifacts so that we only distribute one runtime pack.

Author: steveisok
Assignees: steveisok
Labels:

arch-wasm, area-Infrastructure-mono

Milestone: -

@steveisok
Copy link
Member Author

Should we run only a few selected tests for the new jobs?

I'll defer to @lewing on this one.

@radical
Copy link
Member

radical commented Jun 7, 2022

If you add https://gist.github.com/radical/b1bbb96502b7d185439c63c525536d7a then you can pass shouldContinueOnError: true for the new jobs, since the tests are not expected to be stable there.

@steveisok steveisok force-pushed the wasm-threading-build branch from f9b73c4 to 6b1df44 Compare June 7, 2022 18:46
@steveisok
Copy link
Member Author

@radical @lambdageek I think this is ready to go.

Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. @radical can you take one more look at the yml?

@radical
Copy link
Member

radical commented Jun 7, 2022

/azp run runtime-wasm

@radical
Copy link
Member

radical commented Jun 7, 2022

Running runtime-wasm to sanity check the yml.

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical
Copy link
Member

radical commented Jun 7, 2022

A follow up PR should update https://github.com/dotnet/runtime/blob/main/src/mono/wasm/README.md#pr too.

@radical
Copy link
Member

radical commented Jun 7, 2022

All the builds in runtime-wasm started up without any issues.

@steveisok steveisok merged commit fcdb3fb into dotnet:main Jun 8, 2022
@steveisok steveisok deleted the wasm-threading-build branch June 8, 2022 19:50
@ghost ghost locked as resolved and limited conversation to collaborators Jul 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants