[Mono] Fix all the issues related to enabling WasmStripILAfterAOT #90436

fanyang-mono · 2023-08-11T22:18:50Z

This PR enables IL trim by default for WASM apps, by solving the following major problems:

There are various opcode which was translated into a function call. I modified the call goto logic to check if there is an AOT'ed version of the callee method. If there is, invoke that. If there isn't, interp compile the callee method.

Move the logic of checking if a method is supported a JIT/AOT call from interpreter callback to mini-runtime.c. Because the callback was initialized with the desired logic only when mono_use_interpreter is set to true. See the following code

runtime/src/mono/mono/mini/mini-runtime.c

Lines 4629 to 4632 in 8d4f8b8

    
           #ifndef DISABLE_INTERPRETER 
        
           	if (mono_use_interpreter) 
        
           		mono_ee_interp_init (mono_interp_opts_string); 
        
           #endif

However, in driver.c, mono_use_interpreter was not set to true during AOT compile time, even though interp was passed in as an AOT option. It would be set to true during AOT runtime. Alter this behavior is probably not desirable. Thus, I choose to move the checking logic to mini-runtime.c

Set the trimmed assemblies with the same timestamp as the original assemblies. So that it won't trigger unwanted rebuild or re-AOT
Added implementation of g_fopen to properly handle unicode string on windows.
Interpreter can't do JIT call for methods which require synchronization. Remove those methods from trimmable method list.
Disabled tailcall optimization when calling trimmed methods.
Do jitcall when encountering virtual tailcalls.
Change of ILStrip task:
- The trimmed assemblies were saved in a new folder with the same name as the original assembly.
- Created an output variable UpdatedAssemblies with the updated path of assemblies. The trimmed assemblies are updated with their new path.
- Updated the WASM build toolchain to replace the content of _WasmAssembliesInternal with UpdatedAssemblies
Added the input parameter WasmRuntimeConfigFilePath for WasmAppBuilderBaseTask. Got rid of the logic to calculate the path of runtimeconfig.json file. Use the value of WasmRuntimeConfigFilePath instead. (Fixes [wasm] The way of getting the path of runtimeconfig.json file is not always reliable #93692)
Found two issues related to wasm build process. @radical fixed them.
- [WASM] driver.c got recompiled due to the order of methods changed #93680
- Fixes [wasm] dotnet.native.wasm was re-generated because the list of .o files in emcc-link.rsp is non-deterministic #93689
Blazor app is able to packages the trimmed assemblies, when WasmStripILAfterAOT is enabled.
Added tests for wasm template app and blazor template app

Library size change for top 3 largest libraries:

System.Private.CoreLib.dll.br: 452k -> 316K
System.Text.Json.dll.br: 109k -> 81k
Microsoft.AspNetCore.Components.dll.br: 69k -> 54k

SamMonoRT · 2023-08-13T22:00:17Z

cc @lewing @kg

src/mono/wasm/build/WasmApp.targets

SamMonoRT · 2023-08-14T21:17:19Z

As discussed, merge this only after RC1 has forked and discussions with @lewing

…ld call an AOT'ed version of it before trying to interp compile it

src/mono/mono/mini/aot-compiler.c

… on the value of mono_use_interpreter

src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs

radical · 2023-11-09T01:19:52Z

I would suggest splitting this PR into:

the mono/mono, and src/libraries changes
RuntimeConfigJsonPath change, and the bit in Browser.targets
Rest of the changes

fanyang-mono · 2023-11-09T14:12:19Z

/azp run runtime-wasm

azure-pipelines · 2023-11-09T14:12:39Z

Azure Pipelines successfully started running 1 pipeline(s).

…lways

…nges

- to support webcil case - run blazor app - add cases for the default setting, and the opposite - cleanup

…user

radical · 2023-11-10T05:39:53Z

As discussed offline, I have pushed the following changes:

reverted the rebuild-if-the-property-changes

Also:

the tests were explicitly disabling webcil, fixed that to work with webcil, though the tests compare the file size which is not useful for webcil case. In a future PR we should check for stripping in some other way
Added another case for testing the non-default value also

new changes pushed

radical · 2023-11-10T05:43:09Z

/azp run runtime-wasm

azure-pipelines · 2023-11-10T05:43:24Z

Azure Pipelines successfully started running 1 pipeline(s).

radical

Thank you for your patience! LGTM 👍

fanyang-mono · 2023-11-11T14:06:21Z

/azp run runtime-wasm

azure-pipelines · 2023-11-11T14:06:38Z

Azure Pipelines successfully started running 1 pipeline(s).

…lways

fanyang-mono · 2023-11-13T15:13:10Z

/azp run runtime-wasm

azure-pipelines · 2023-11-13T15:13:20Z

Azure Pipelines successfully started running 1 pipeline(s).

fanyang-mono · 2023-11-13T18:47:22Z

According too Build Analysis, all test failures are known.

@rolfbjarne

Changes: dotnet/installer@e1fd7d9...42ace91 % git diff --shortstat e1fd7d9649...42ace91ba7 182 files changed, 8428 insertions(+), 2413 deletions(-) Changes: dotnet/runtime@287c10d...a26802a % git diff --shortstat 287c10d253...a26802aa57 10852 files changed, 266698 insertions(+), 198289 deletions(-) Changes: dotnet/emsdk@1999c8c...74e4868 % git diff --shortstat 1999c8c8ab...74e4868be8 39 files changed, 616 insertions(+), 260 deletions(-) Changes: dotnet/cecil@a112f15...45dd3a7 * dotnet/cecil@45dd3a7: [main] Update dependencies from dotnet/source-build-reference-packages (dotnet/cecil#116) * dotnet/cecil@0b78015: Update dependencies from https://github.com/dotnet/arcade build 20231008.1 (dotnet/cecil#115) * dotnet/cecil@64a8874: [main] Update dependencies from dotnet/source-build-reference-packages (dotnet/cecil#114) * dotnet/cecil@13d6536: [main] Update dependencies from dotnet/source-build-reference-packages (dotnet/cecil#113) * dotnet/cecil@89be445: [main] Update dependencies from dotnet/source-build-reference-packages (dotnet/cecil#112) * dotnet/cecil@84f4527: Update dependencies from https://github.com/dotnet/arcade build 20230913.1 (dotnet/cecil#111) Changes: dotnet/android-tools@08a6990...21de3d7 * dotnet/android-tools@21de3d7: [build] update $(MSBuildPackageReferenceVersion) to 17.6.3 (dotnet/android-tools#221) Updates: * Microsoft.Dotnet.Sdk.Internal: from 8.0.100-rc.2.23468.1 to 9.0.100-alpha.1.23603.1 * Microsoft.NETCore.App.Ref: from 8.0.0-rc.2.23466.4 to 9.0.0-alpha.1.23577.7 * Microsoft.NET.Workload.Emscripten.Current.Manifest-*.Transport: from 8.0.0-rc.2.23463.1 to 9.0.0-alpha.1.23572.3 * Microsoft.DotNet.Cecil: from 0.11.4-alpha.23461.1 to 0.11.4-alpha.23509.2 ~~ Changes to the build ~~ `$(DotNetTargetFrameworkVersion)` is `9.0`, making assemblies like `Mono.Android.dll` target .NET 9.0, as well as `$(TargetFramework)` in project templates, etc. `$(DotNetStableTargetFramework)` is `8.0`, making various bootstrap tooling, command-line tools, and unit tests target .NET 8.0. .NET 9 has a new `.binlog` format, requiring `MSBuild.StructuredLogger` 2.2.100. `System.CodeDom` now targets 8.0.0, as we moved to 17.8 MSBuild assemblies from `xamarin-android-tools`. Add the `dotnet9` and `dotnet9-transport` feeds. The `dotnet9-transport` feed is used for non-customer-facing packages like `Microsoft.NET.ILLink`. `build-linux.yaml` was initially failing to restore `net8.0` projects because of the order it installed a .NET SDK. It was only working for `net7.0` projects, because a .NET 7 SDK was preinstalled on the build machines. Update `TestSlicerToolVersion` and `dotnet-test-slicer` to `0.1.0-alpha7`, previously it was failing to discover tests in a `net8.0` project. Provision Mono workload manifests for: `net6`, `net7`, `net8`, and `current` (`net9`). Call `Utilities.DeleteDirectory()` on these directories to be sure we are in a clean state. dotnet/android-tools@21de3d7 is needed to fix various NU1605 warnings: error NU1605: Warning As Error: Detected package downgrade: Microsoft.Build.Framework from 17.5.0 to 17.3.2. Reference the package directly from the project to select a different version. error NU1605: MSBuildDeviceIntegration -> MSBuild.StructuredLogger 2.2.100 -> doh (>= 17.5.0) error NU1605: MSBuildDeviceIntegration -> Microsoft.Build.Framework (>= 17.3.2) Update workload aliases to include `Microsoft.Android.Sdk.net9` and `Microsoft.Android.Sdk.net8`. Remove the .NET 7 Android workload from .NET 9. Import `Eol.targets` for .NET 6/7 projects. Update various parameterized tests to target `net8.0` and `net9.0` appropriately while dropping `net7.0` support. An initial update to the AOT profile for .NET 9. Updated `*.apkdesc` files: there were some minor `.apk` size changes. ~~ `$(AllowSelfContainedWithoutRuntimeIdentifier)` ~~ Context: dotnet/sdk@d21e6bf Context: d12da3a @rolfbjarne introduced an "escape hatch" in the .NET 9 SDK for emitting a build warning for a case that doesn't make sense on mobile. All mobile apps are self-contained, and we define RIDs by default, so the error is not needed. We can also stop setting `$(PublishSelfContained)` to `false`, as `$(AllowSelfContainedWithoutRuntimeIdentifier)` is sufficient in .NET 9. ~~ Changes to `<ILStrip/>` ~~ Context: dotnet/runtime#90436 * `$(IntermediateOutputPath)` is now required. * `[Output] TrimmedAssemblies` renamed to `UpdatedAssemblies`. * Output items shape changed, so now use `%(UntrimmedAssemblyFilePath)` and we can `<Copy/>` based on `%(ILStripped)`. The new code is better, so generally these changes seem *good*. ~~ Refactoring ~~ Rename `$(AndroidNet7Version)` to `$(AndroidNetPreviousVersion)` to make this property more generic for future releases. Use `@NET_PREVIOUS_VERSION@` as a replacement instead of `@NET7_VERSION@`. ~~ TODO ~~ * #8548 The `MicrosoftIntune` test is failing because they have a check for == `8.0` that needs to be updated to be >= `8.0`. This test is running on our `release/8.0.1xx` branch, but we can hopefully re-enable in main after their next release. * A specific case of `dotnet publish -f net8.0-android -r android-arm` is failing with: error NETSDK1185: The Runtime Pack for FrameworkReference 'Microsoft.Android.Runtime.34.android-arm' was not available. This may be because DisableTransitiveFrameworkReferenceDownloads was set to true. This is related to `$(AllowSelfContainedWithoutRuntimeIdentifier)`, but I will address in a future PR. This will likely require .NET 8 Android workload changes.

Enable IL trim for WASM by default

68dd32a

dotnet-issue-labeler bot added the area-Build-mono label Aug 11, 2023

ghost assigned fanyang-mono Aug 11, 2023

fanyang-mono changed the title ~~[Mono] Enable IL trim for WASM apps by default~~ [Mono][WIP] Enable IL trim for WASM apps by default Aug 11, 2023

This was referenced Aug 12, 2023

Tracking issue for CI build timeouts #76454

Closed

Insufficient memory of docker containers on CI dotnet/dnceng#450

Open

kg reviewed Aug 13, 2023

View reviewed changes

src/mono/wasm/build/WasmApp.targets Outdated Show resolved Hide resolved

Make ILStrip available for local build

7507121

build-analysis bot mentioned this pull request Aug 16, 2023

[wasm] AOT: System.Text.Json.Tests crashing inexplicably #89984

Closed

Make all calling another method go through the logic to see if it cou…

b02a8e5

…ld call an AOT'ed version of it before trying to interp compile it

fanyang-mono commented Aug 25, 2023

View reviewed changes

src/mono/mono/mini/aot-compiler.c Outdated Show resolved Hide resolved

fanyang-mono added 2 commits August 25, 2023 16:21

Add back accidentally removed line of code

1f7b587

Update test to accommodate IL trim with WASM AOT

83cd21e

build-analysis bot mentioned this pull request Sep 5, 2023

Microsoft.NET.HostModel.Tests failing with "No space left on device" #91039

Closed

fanyang-mono added 2 commits September 6, 2023 17:28

Move jit_call_can_be_supported to mini-runtime, so it doesn't depends…

1f2f152

… on the value of mono_use_interpreter

Update var name

d9027dd

build-analysis bot mentioned this pull request Sep 7, 2023

Networking certificate test failures #91705

Closed

fanyang-mono marked this pull request as ready for review September 7, 2023 13:21

fanyang-mono requested review from radical, BrzVlad, vargaz, kotlarmilos, lambdageek and SamMonoRT as code owners September 7, 2023 13:21

fanyang-mono changed the title ~~[Mono][WIP] Enable IL trim for WASM apps by default~~ [Mono] Enable IL trim for WASM apps by default Sep 7, 2023

Attempt to fix rebuild test failures

481db67

vargaz reviewed Sep 11, 2023

View reviewed changes

src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs Outdated Show resolved Hide resolved

build-analysis bot mentioned this pull request Sep 11, 2023

Test_EventSource_EtwManifestGeneration* tests failing in CI #48798

Closed

Revert EmccCompile change

7b76c7e

radical added 5 commits November 9, 2023 17:29

Merge remote-tracking branch 'origin/main' into enable_il_trim_wasm_a…

977f49d

…lways

MonoAOTCompiler: revert rebuilding when WasmStripILAfterAOT value cha…

ada4f8b

…nges

Compress -> GZipCompress, and cleanup

4557eb4

rework the tests

9a7c37d

- to support webcil case - run blazor app - add cases for the default setting, and the opposite - cleanup

ILStrip.cs: Emit a message about stripping to make it obvious to the …

562d845

…user

WasmApp.targets: update comment

740bb45

radical approved these changes Nov 10, 2023

View reviewed changes

Change default value to false and update test

a93b5d1

Merge remote-tracking branch 'origin/main' into enable_il_trim_wasm_a…

5fc92ca

…lways

fanyang-mono changed the title ~~[Mono] Enable IL strip for WASM apps by default~~ [Mono] Fix all the issues related to enabling WasmStripILAfterAOT Nov 13, 2023

fanyang-mono merged commit 085ddb7 into dotnet:main Nov 13, 2023
193 of 199 checks passed

fanyang-mono mentioned this pull request Nov 13, 2023

[wasm] Enable WasmStripILAfterAOT by default #94682

Merged

radekdoulik mentioned this pull request Nov 15, 2023

[Perf] Linux/x64: 10 Regressions on 11/14/2023 8:48:07 AM dotnet/perf-autofiling-issues#24535

Open

This was referenced Nov 16, 2023

[Mono] Enable more WASM AOT tests to run with WASMStripIL=true #90233

Closed

[Mono] Enhance ILStrip to be capable of trimming away a single method #80953

Open

github-actions bot locked and limited conversation to collaborators Dec 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Mono] Fix all the issues related to enabling WasmStripILAfterAOT #90436

[Mono] Fix all the issues related to enabling WasmStripILAfterAOT #90436

fanyang-mono commented Aug 11, 2023 •

edited

Loading

SamMonoRT commented Aug 13, 2023

SamMonoRT commented Aug 14, 2023

radical commented Nov 9, 2023

fanyang-mono commented Nov 9, 2023

azure-pipelines bot commented Nov 9, 2023

radical commented Nov 10, 2023 •

edited

Loading

radical commented Nov 10, 2023

azure-pipelines bot commented Nov 10, 2023

radical left a comment

fanyang-mono commented Nov 11, 2023

azure-pipelines bot commented Nov 11, 2023

fanyang-mono commented Nov 13, 2023

azure-pipelines bot commented Nov 13, 2023

fanyang-mono commented Nov 13, 2023

	#ifndef DISABLE_INTERPRETER
	if (mono_use_interpreter)
	mono_ee_interp_init (mono_interp_opts_string);
	#endif

[Mono] Fix all the issues related to enabling WasmStripILAfterAOT #90436

[Mono] Fix all the issues related to enabling WasmStripILAfterAOT #90436

Conversation

fanyang-mono commented Aug 11, 2023 • edited Loading

SamMonoRT commented Aug 13, 2023

SamMonoRT commented Aug 14, 2023

radical commented Nov 9, 2023

fanyang-mono commented Nov 9, 2023

azure-pipelines bot commented Nov 9, 2023

radical commented Nov 10, 2023 • edited Loading

radical commented Nov 10, 2023

azure-pipelines bot commented Nov 10, 2023

radical left a comment

Choose a reason for hiding this comment

fanyang-mono commented Nov 11, 2023

azure-pipelines bot commented Nov 11, 2023

fanyang-mono commented Nov 13, 2023

azure-pipelines bot commented Nov 13, 2023

fanyang-mono commented Nov 13, 2023

fanyang-mono commented Aug 11, 2023 •

edited

Loading

radical commented Nov 10, 2023 •

edited

Loading