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] Compile .bc->.o in parallel, before passing to the linker #54053

Merged
merged 22 commits into from
Jun 23, 2021

Conversation

radical
Copy link
Member

@radical radical commented Jun 11, 2021

Currently, we use mono-aot-cross to compile the managed assemblies
into bitcode files. Then we pass the .bc files to emcc for linking.
We can save some amount of work, if precompile .bc files to .o
files, and pass them for linking.

This would also allow us to rebuild the .o files only when needed,
thus improving incremental builds.
Also, emscripten has recently removed some amount of parallelization, so
this allows us to do some pre-linking work in parallel, and speed up the
builds a little bit.

This is implemented via a new EmccCompile task, that is meant only
for compiling, and not as a general purpose emcc invocation task.

Also, this bumps msbuild used for tasks compilation to 16.9.0.

- this cleans up `Utils.RunProcess` a bit, but doesn't change the
  behavior for the existing non-wasm users (Apple, and Android build
  tasks)

- New overloads are added to support getting the output, and exit code,
  and throwing only when really needed
- And running shell commands, appropriately on windows, and unix.
Currently, we use `mono-aot-cross` to compile the managed assemblies
into bitcode files. Then we pass the `.bc` files to `emcc` for linking.
We can save some amount of work, if precompile `.bc` files to `.o`
files, and pass them for linking.

This would also allow us to rebuild the `.o` files only when needed,
thus improving incremental builds.
Also, emscripten has recently removed some amount of parallelization, so
this allows us to do some pre-linking work in parallel, and speed up the
builds a little bit.

This is implemented via a new `EmccCompile` task, that is meant *only*
for compiling, and not as a general purpose `emcc` invocation task.
`System.IO.MemoryMappedFiles.Tests` - Issue: dotnet#54194
@radical radical marked this pull request as ready for review June 15, 2021 00:20
@radical radical added the arch-wasm WebAssembly architecture label Jun 15, 2021
@ghost
Copy link

ghost commented Jun 15, 2021

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

Issue Details
Author: radical
Assignees: -
Labels:

arch-wasm, area-Build-mono

Milestone: -

@radical
Copy link
Member Author

radical commented Jun 16, 2021

@akoeplinger Does the msbuild bump to 16.10.0 look fine?

@radical
Copy link
Member Author

radical commented Jun 16, 2021

/azp run runtime,runtime-staging

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@lewing
Copy link
Member

lewing commented Jun 17, 2021

/azp run runtime

- This is meant to be disabled for CI (it has `IgnoreForCI=true` in the
  csproj)
.. because we keep running into the following on browser, and other
platforms:

```
/__w/1/s/src/libraries/Microsoft.NETCore.Platforms/tests/Microsoft.NETCore.Platforms.Tests.csproj : error NU1201: Project System.Security.AccessControl is not compatible with net472 (.NETFramework,Version=v4.7.2). Project System.Security.AccessControl supports: net6.0 (.NETCoreApp,Version=v6.0) [/__w/1/s/Build.proj]
\#\#[error]src/libraries/Microsoft.NETCore.Platforms/tests/Microsoft.NETCore.Platforms.Tests.csproj(0,0): error NU1201: (NETCORE_ENGINEERING_TELEMETRY=Restore) Project System.Security.AccessControl is not compatible with net472 (.NETFramework,Version=v4.7.2). Project System.Security.AccessControl supports: net6.0 (.NETCoreApp,Version=v6.0)
```
@radical radical removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jun 22, 2021
This is already not being sent to CI with `IgnoreForCI=true`.
It fails on CI with:

```
/_/src/Shared/NativeMethodsShared.cs(855,29): error IL1005: Microsoft.Build.Shared.NativeMethodsShared.SystemInformation.get: Error processing method 'Microsoft.Build.Shared.NativeMethodsShared.SystemInformationData.SystemInformationData()' in assembly 'Microsoft.Build.Utilities.Core.dll' [/__w/1/s/src/libraries/Microsoft.NETCore.Platforms/tests/Microsoft.NETCore.Platforms.Tests.csproj]
\##[error]/_/src/Shared/NativeMethodsShared.cs(855,29): error IL1005: (NETCORE_ENGINEERING_TELEMETRY=Build) Microsoft.Build.Shared.NativeMethodsShared.SystemInformation.get: Error processing method 'Microsoft.Build.Shared.NativeMethodsShared.SystemInformationData.SystemInformationData()' in assembly 'Microsoft.Build.Utilities.Core.dll'
  Mono.Linker.LinkerFatalErrorException: /_/src/Shared/NativeMethodsShared.cs(855,29): error IL1005: Microsoft.Build.Shared.NativeMethodsShared.SystemInformation.get: Error processing method 'Microsoft.Build.Shared.NativeMethodsShared.SystemInformationData.SystemInformationData()' in assembly 'Microsoft.Build.Utilities.Core.dll'
   ---> System.NotImplementedException: switch
     at Mono.Linker.Steps.UnreachableBlocksOptimizer.BodyReducer.IsConstantBranch(OpCode opCode, Int32 operand) in illink.dll:token 0x6000770+0x41
     at Mono.Linker.Steps.UnreachableBlocksOptimizer.BodyReducer.RemoveConditions() in illink.dll:token 0x6000768+0x22a
     at Mono.Linker.Steps.UnreachableBlocksOptimizer.BodyReducer.RewriteBody() in illink.dll:token 0x6000766+0xa
     at Mono.Linker.Steps.UnreachableBlocksOptimizer.ProcessStack() in illink.dll:token 0x60004e9+0x102
     at Mono.Linker.Steps.UnreachableBlocksOptimizer.ProcessMethod(MethodDefinition method) in illink.dll:token 0x60004e5+0x44
     at Mono.Linker.Steps.MarkStep.ProcessMethod(MethodDefinition method, DependencyInfo& reason, Scope& scope) in illink.dll:token 0x600040e+0x9b
     at Mono.Linker.Steps.MarkStep.ProcessQueue() in illink.dll:token 0x6000399+0x24
     --- End of inner exception stack trace ---
     at Mono.Linker.Steps.MarkStep.ProcessQueue() in illink.dll:token 0x6000399+0xaa
     at Mono.Linker.Steps.MarkStep.ProcessPrimaryQueue() in illink.dll:token 0x6000396+0xa
     at Mono.Linker.Steps.MarkStep.Process() in illink.dll:token 0x6000393+0x0
     at Mono.Linker.Steps.MarkStep.Process(LinkContext context) in illink.dll:token 0x600038a+0x41
     at Mono.Linker.Pipeline.ProcessStep(LinkContext context, IStep step) in illink.dll:token 0x6000264+0x0
     at Mono.Linker.Pipeline.Process(LinkContext context) in illink.dll:token 0x6000263+0xf
     at Mono.Linker.Driver.Run(ILogger customLogger) in illink.dll:token 0x60000fd+0x20
  Optimizing assemblies for size, which may change the behavior of the app. Be sure to test after publishing. See: https://aka.ms/dotnet-illink
/__w/1/s/.dotnet/sdk/6.0.100-preview.4.21255.9/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ILLink.targets(80,5): error NETSDK1144: Optimizing assemblies for size failed. Optimization can be disabled by setting the PublishTrimmed property to false. [/__w/1/s/src/libraries/Microsoft.NETCore.Platforms/tests/Microsoft.NETCore.Platforms.Tests.csproj]
\##[error].dotnet/sdk/6.0.100-preview.4.21255.9/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ILLink.targets(80,5): error NETSDK1144: (NETCORE_ENGINEERING_TELEMETRY=Build) Optimizing assemblies for size failed. Optimization can be disabled by setting the PublishTrimmed property to false.
```
@radical
Copy link
Member Author

radical commented Jun 22, 2021

cc @ericstj @ViktorHofer for the change disabling Microsoft.NETCore.Platforms.Tests for wasm.

@radical
Copy link
Member Author

radical commented Jun 23, 2021

1 test failure for wasm:

[04:55:37] info: browser: Console websocket connected.
[04:55:37] info: Arguments: --run,WasmTestRunner.dll,System.IO.Compression.Tests.dll,-notrait,category=IgnoreForCI,-notrait,category=OuterLoop,-notrait,category=failing
[04:55:37] fail: [out of order message from the browser]: http://127.0.0.1:34229/runtime.js 234:26 Uncaught TypeError: Cannot set property 'loaded_cb' of null
[05:10:37] fail: Application has finished with exit code TIMED_OUT but 0 was expected

@Daniel-Genkin is already looking at fixing this.
I think we can merge this.

@lewing
Copy link
Member

lewing commented Jun 23, 2021

@Daniel-Genkin is already looking at fixing this.
I think we can merge this.

fix should be in #54587

@ViktorHofer
Copy link
Member

@ViktorHofer Any idea why a msbuild bump to 16.10.0 in this PR might be failing wasm/android/iOS/etc builds? That's the only non-wasm specific change here.

That very much sounds like NuGet/Home#10368. Usually you can fix this by adding the transitive dependencies, in this case in the Platforms.Tests.csproj file.

@radical
Copy link
Member Author

radical commented Jun 23, 2021

The failures are all unrelated to this PR.

@radical radical merged commit 0416c34 into dotnet:main Jun 23, 2021
@radical radical deleted the wasm-bc-to-o-parallel branch June 23, 2021 16:54
@ViktorHofer
Copy link
Member

The failures are all unrelated to this PR.

Usually we link to issues that track the failures according to https://github.com/dotnet/runtime/blob/main/docs/issues-pr-management.md#common-policies. Can you please do so for the issues that you were seeing in the last build?

@radical
Copy link
Member Author

radical commented Jun 23, 2021

Failures:

Libraries Test Run checked coreclr windows x86 Release - #54628
Libraries Test Run release mono OSX x64 Debug - #54624
Libraries Test Run release coreclr windows x64 Debug - #54627
Libraries Test Run release coreclr Linux x64 Debug - #54623

And for CoreCLR Pri0 Runtime Tests Run windows arm64 checked - profiler.gc, and JIT.Directed failed but I'm not able to access the logs for them.

thaystg added a commit to thaystg/runtime that referenced this pull request Jun 23, 2021
…nitial_config

* origin/main: (362 commits)
  [wasm][debugger] Reuse debugger-agent on wasm debugger (dotnet#52300)
  Put Crossgen2 in sync with dotnet#54235 (dotnet#54438)
  Move System.Object serialization to ObjectConverter (dotnet#54436)
  Move setting fHasVirtualStaticMethods out of sanity check section (dotnet#54574)
  [wasm] Compile .bc->.o in parallel, before passing to the linker (dotnet#54053)
  Change PathInternal.IsCaseSensitive to a constant (dotnet#54340)
  Make mono_polling_required a public symbol (dotnet#54592)
  Respect EventSource::IsSupported setting in more codepaths (dotnet#51977)
  Root ComActivator for hosting (dotnet#54524)
  Add ILLink annotations to S.D.Common related to DbConnectionStringBuilder (dotnet#54280)
  Fix finalizer issue with regions (dotnet#54550)
  Add support for multi-arch install locations (dotnet#53763)
  Update library testing docs page to reduce confusion (dotnet#54324)
  [FileStream] handle UNC and device paths (dotnet#54483)
  Update NetAnalyzers version (dotnet#54511)
  Added runtime dependency to fix the intermittent test failures (dotnet#54587)
  Disable failing System.Reflection.Tests.ModuleTests.GetMethods (dotnet#54564)
  [wasm] Move AOT builds from `runtime-staging` to `runtime` (dotnet#54577)
  Keep obj node for ArrayIndex. (dotnet#54584)
  Disable another failing MemoryCache test (dotnet#54578)
  ...
@ghost ghost locked as resolved and limited conversation to collaborators Jul 23, 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.

None yet

4 participants