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] Bump emscripten to 2.0.23 #53603

Merged
merged 28 commits into from
Jun 24, 2021

Conversation

radekdoulik
Copy link
Member

No description provided.

@ghost
Copy link

ghost commented Jun 2, 2021

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

Issue Details
Author: radekdoulik
Assignees: -
Labels:

area-System.IO

Milestone: -

@radekdoulik radekdoulik added area-Build-mono arch-wasm WebAssembly architecture and removed area-System.IO labels Jun 2, 2021
@ghost
Copy link

ghost commented Jun 2, 2021

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

Issue Details
Author: radekdoulik
Assignees: -
Labels:

arch-wasm, area-Build-mono

Milestone: -

@radekdoulik radekdoulik marked this pull request as ready for review June 9, 2021 14:43
@radekdoulik radekdoulik requested a review from marek-safar as a code owner June 9, 2021 14:43
@radekdoulik
Copy link
Member Author

The failing System.Net.Primitives.Unit.Tests.CookieContainerTest.SetCookies_Success is #53931

@radekdoulik
Copy link
Member Author

The fix for FS bug, where it ignores permissions didn't make it into 2.0.23, even if it looks like it from the commit history.

I still see this in upstream\emscripten\src\library_fs.js:

  $FS__postset: function() {
    // TODO: do we need noFSInit?
    addAtInit('if (!Module["noFSInit"] && !FS.init.initialized) FS.init();');
    addAtMain('FS.ignorePermissions = false;');
...

addAtMain shouldn't be used here.

Context: emscripten-core/emscripten@a56817a

@radekdoulik radekdoulik requested a review from kg June 9, 2021 14:52
@radical
Copy link
Member

radical commented Jun 9, 2021

Do we know why the AOT tests are failing?
for example - System.Xml.XPath.XmlDocument.Tests failed (received SIGKILL (-9))
and System.Data.Common.Tests

@radical
Copy link
Member

radical commented Jun 9, 2021

Some of them are timing out, due to the recent increase in build time. We should bump the timeout, to we can at least get the tests running, and not miss any regressions. I would add

diff --git a/src/libraries/sendtohelixhelp.proj b/src/libraries/sendtohelixhelp.proj
index 84c140a2182..1dd6716f2e8 100644
--- a/src/libraries/sendtohelixhelp.proj
+++ b/src/libraries/sendtohelixhelp.proj
@@ -29,6 +29,7 @@
     <_workItemTimeout Condition="'$(Scenario)' == '' and '$(_workItemTimeout)' == '' and ('$(TargetArchitecture)' == 'arm64' or '$(TargetArchitecture)' == 'arm')">00:45:00</_workItemTimeout>
     <_workItemTimeout Condition="'$(Scenario)' != '' and '$(_workItemTimeout)' == '' and ('$(TargetArchitecture)' == 'arm64' or '$(TargetArchitecture)' == 'arm')">01:00:00</_workItemTimeout>
     <_workItemTimeout Condition="'$(Scenario)' == 'BuildWasmApps' and '$(_workItemTimeout)' == ''">01:00:00</_workItemTimeout>
+    <_workItemTimeout Condition="'$(TargetOS)' == 'Browser' and '$(NeedsToBuildWasmAppsOnHelix)' == 'true'">01:00:00</_workItemTimeout>
     <_workItemTimeout Condition="'$(Scenario)' == '' and '$(_workItemTimeout)' == ''">00:15:00</_workItemTimeout>
     <_workItemTimeout Condition="'$(Scenario)' != '' and '$(_workItemTimeout)' == ''">00:30:00</_workItemTimeout>

radekdoulik added a commit to radekdoulik/dotnet-buildtools-prereqs-docker that referenced this pull request Jun 10, 2021
To be able to use this image before dotnet/runtime#53603
is finished
@radical
Copy link
Member

radical commented Jun 10, 2021

8 libraries are failing with AOT. Some are timing out, some with emcc getting killed with:

emcc : error : '/datadisks/disk1/work/B1410964/p/build/emsdk/upstream/bin/wasm-ld @/datadisks/disk1/work/B1410964/t/emscripten_icjo4j9w.rsp' failed (received SIGKILL (-9))

@radekdoulik
Copy link
Member Author

radekdoulik commented Jun 17, 2021

In the last run there are 7 test jobs failing, only 1 of them receiving signal SIGKILL, System.Data.Common.Tests.

On linux helix machine, the limits are set like this:

+ ulimit -a
time(seconds)        unlimited
file(blocks)         unlimited
data(kbytes)         unlimited
stack(kbytes)        8192
coredump(blocks)     unlimited
memory(kbytes)       unlimited
locked memory(kbytes) 16384
process              31703
nofiles              16384
vmemory(kbytes)      unlimited
locks                unlimited
rtprio               0

and the system has cca 8GB of memory and 128GB of storage space for /datadisks/disk1 mount.

@radekdoulik
Copy link
Member Author

Looks like the wasm-ld is indeed causing issues here. It is always this tool, which gets killed or is timeouting.

I tried to build the first library test, which timeouts - Microsoft.Extensions.Configuration.Json. It builds on my machine, but the memory usage during wasm-ld run is peaking around 12.3GB. The helix system has only 8GB, so that might be the issue here. It took cca 16 minutes to build. I will try to build it with 2.0.21 as well to see how the memory usage compares.

Note: the local build might be behaving differently compared to helix, I see 81 assemblies AOT'ed after linking vs 61 on helix.

@radical
Copy link
Member

radical commented Jun 17, 2021

Do you think #54053 might help? It builds dll -> .bc -> .o with Oz, and then links with O0.

I tried to change EmccLinkOptimizationFlag property to -O0 and the memory peaked around 12.7GB as well. Not saure whether it is good enough test though.

@radekdoulik
Copy link
Member Author

The 2.0.21 build of the same lib test peaked around 8.8GB of memory. So it looks like regression of wasm-ld. I will try to get more details and open an issue in emscripten repo.

@lewing
Copy link
Member

lewing commented Jun 17, 2021

cc @marek-safar

@radekdoulik
Copy link
Member Author

It might be the new string tail merging in wasm-ld:

- wasm-ld will now perform string tail merging in debug string sections as well
  as regular data sections.   This behaviour can be be disabled with `-Wl,-O0`.
  This should significantly reduce the size of dwarf debug information in the
  wasm binary.

Let see if the -Wl,-O0 changes the memory load.

@radekdoulik
Copy link
Member Author

radekdoulik commented Jun 18, 2021

-Wl,O0 doesn't help with the memory load, neither does -Oz -Wl,-O0.

Merging current main improves it a bit, we are still at 10.9GB peek.

I have opened emscripten-core/emscripten#14485 and will try to dig deeper to see if I can spot what might be causing the higher memory usage.

To be sure it behaves the same as in local build
@radekdoulik
Copy link
Member Author

Lukas Lansky opened a ticket for us to get more memory (and cores as side effect) https://github.com/dotnet/core-eng/issues/13425

It looks like it reduces the memory load a lot
@lewing
Copy link
Member

lewing commented Jun 23, 2021

Nice to see this green, please get in mergeable condition so we can land it.

@lewing
Copy link
Member

lewing commented Jun 24, 2021

browser failure is #53957

@radekdoulik
Copy link
Member Author

CoreCLR Pri0 Runtime Tests Run windows arm64 checked - failure is the same seen in #54053 and doesn't contain logs

@radekdoulik radekdoulik merged commit 055a38a into dotnet:main Jun 24, 2021
thaystg added a commit to thaystg/runtime that referenced this pull request Jun 24, 2021
…bugger2

* origin/main: (107 commits)
  Disable MacCatalyst arm64 PR test runs on staging pipeline (dotnet#54678)
  [WASM] Fix async/await in config loading (dotnet#54652)
  Fix for heap_use_after_free flagged by sanitizer (dotnet#54679)
  [wasm] Bump emscripten to 2.0.23 (dotnet#53603)
  Fix compiler references when building inside VS (dotnet#54614)
  process more TLS frames at one when available (dotnet#50815)
  Add PeriodicTimer (dotnet#53899)
  UdpClient with span support (dotnet#53429)
  exclude fragile tests (dotnet#54671)
  get last error before calling a method that might fail as well (dotnet#54667)
  [FileStream] add tests for device and UNC paths (dotnet#54545)
  Fix sporadic double fd close (dotnet#54660)
  Remove Version.Clone from AssemblyName.Clone (dotnet#54621)
  [wasm] Enable fixed libraries tests (dotnet#54641)
  [wasm] Fix blazor/aot builds (dotnet#54651)
  [mono][wasm] Fix compilation error on wasm (dotnet#54659)
  Fix telemetry for Socket connects to Dns endpoints (dotnet#54071)
  [wasm] Build static components; include hot_reload in runtime (dotnet#54568)
  [wasm][debugger] Reuse debugger-agent on wasm debugger (dotnet#52300)
  Put Crossgen2 in sync with dotnet#54235 (dotnet#54438)
  ...
@ghost ghost locked as resolved and limited conversation to collaborators Jul 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Build-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants