[wasm] Allow assemblies to be skipped for AOTMode=AotInterp by radical · Pull Request #48500 · dotnet/runtime

radical · 2021-02-19T05:03:30Z

Allow skipping AOT, per assembly with %(_InternalForceInterpret) metadata
build: Rename @(WasmAssembliesToBundle) -> @(WasmAssembly)
new $(WasmNativeDebugSymbols) to control symbols for dotnet.wasm (useful only for AOT case), which defaults to true
Path handling fixes for AOT builds on windows
add create-packs.proj for setting up packs from a local build, for quick local testing

- The reason for this change is that generating the app bundle is optional now. Eg. in case of blazor, which handles the layout itself. - In that case, the assemblies will be used for other things during `WasmBuildApp` target (eg. AOT). - Change the name to reflect that

pranavkm · 2021-02-19T05:42:42Z

 </ItemGroup>
 <ItemGroup>
- <_AotInputAssemblies Include="@(_WasmAssemblies->Distinct())">
+ <_AotInputAssemblies Include="@(_WasmAssembliesInternal->Distinct())" Condition="'%(_WasmAssembliesInternal.InterpOnly)' != 'true'">


InterpOnly
Could we use a non-abbreviated name InterprettedOnly? Users might need to configure it.
Might also want to consider something like AOTMode=Intepretted.
This would look like:
_AotInputAssemblies Include="@(_WasmAssembliesInternal->Distinct())" Condition="'%(_WasmAssembliesInternal.AOTMode)' != 'Interpretted'">

Might also want to consider something like AOTMode=Intepretted.
Does it make sense to have the possible values for AOTMode to be as what Android, and iOS have? @lewing @marek-safar @jonathanpeppers

What we have now:
https://github.com/xamarin/xamarin-android/blob/accc846e35ccc2b21e2a472d51ab58ed0682643a/src/Xamarin.Android.Build.Tasks/Tasks/Aot.cs#L20-L27
It kind of maps to the native side:
runtime/src/mono/mono/mini/jit.h
Lines 46 to 72 in 1d9e50c
/**
* Allows control over our AOT (Ahead-of-time) compilation mode.
*/
typedefenum {
/* Disables AOT mode */
MONO_AOT_MODE_NONE,
/* Enables normal AOT mode, equivalent to mono_jit_set_aot_only (false) */
MONO_AOT_MODE_NORMAL,
/* Enables hybrid AOT mode, JIT can still be used for wrappers */
MONO_AOT_MODE_HYBRID,
/* Enables full AOT mode, JIT is disabled and not allowed,
* equivalent to mono_jit_set_aot_only (true) */
MONO_AOT_MODE_FULL,
/* Same as full, but use only llvm compiled code */
MONO_AOT_MODE_LLVMONLY,
/* Uses Interpreter, JIT is disabled and not allowed,
* equivalent to "--full-aot --interpreter" */
MONO_AOT_MODE_INTERP,
/* Same as INTERP, but use only llvm compiled code */
MONO_AOT_MODE_INTERP_LLVMONLY,
/* Use only llvm compiled code, fall back to the interpeter */
MONO_AOT_MODE_LLVMONLY_INTERP,
/* Same as --interp */
MONO_AOT_MODE_INTERP_ONLY,
/* Sentinel value used internally by the runtime. We use a large number to avoid clashing with some internal values. */
MONO_AOT_MODE_LAST=1000,
} MonoAotMode;

I don't think our integer values matter at build time for what is passed at runtime.

And those c# enum names are the strings used with $(AOTMode) (or equivalent property name)?

Our property is $(AndroidAotMode) right now, but we can use a new one in .NET 6 to match other platforms. I'm fine with $(AOTMode) if that is what we pick.

Sorry, I'm not being clear. What values do you use for $(AndroidAotMode)? Are they $(AndroidAotMode)=normal, $(AndroidAotMode)=hybrid etc, to match the C# enum?

It looks like they all map exactly to this C# enum, except for Intepreter:
https://github.com/xamarin/xamarin-android/blob/d8df924702a753bbba8d317b3d5e88dde10f7a52/src/Xamarin.Android.Build.Tasks/Tasks/Aot.cs#L96-L122

This was added earlier to support blazor workload which only uses AOT. But that has since been fixed to correctly use `WasmBuildApp`+`@(WasmNativeAsset)`, so this can be removed.

ghost · 2021-02-24T14:24:10Z

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

Issue Details

Allow skipping AOT, per assembly with %(InterpreterOnly) metadata
build: Rename @(WasmAssembliesToBundle) -> @(WasmAssembly)
add create-packs.proj for setting up packs from a local build, for quick local testing

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

Co-authored-by: Pranav K <prkrishn@hotmail.com>

.. and enable it by default.

….wasm with symbols .. defaults to True.

…not set

1. tests in `src/tests` failed with: `Bug: The wasm build started with 2 assemblies, but completed with 11.` .. this was because they build with `$(WasmResolveAssembliesBeforeBuild)=true` and that adds to the original list of assemblie. So, skip checking in that case. 2. AOT functional tests failed with: `Bug: The wasm build started with 356 assemblies, but completed with 178. ` .. this was because the original list had dupes! So, ensure to count only distinct items.

radical · 2021-02-25T19:08:26Z

Unrelated test failures in Libraries Test Run release {coreclr,mono} Linux x64 Debug

radical · 2021-02-25T21:55:21Z

Test failures being tracked in #48751

lewing

The changes look good to me. I'd like to come to some broader consensus on naming, especially Aot vs AOT and the (Aot|AOT)Mode bits.

lewing · 2021-02-26T20:43:37Z

But I'm ok to approve to unblock things if @pranavkm is waiting on this?

This uses `mono-cil-strip` from a mono installation. And in it's current form it can cause issues, so disabling it for now.

radical · 2021-03-15T20:30:26Z

The parts of this that didn't do naming changes have been merged as part of #49446 . I will open a new PR, when we have a consensus on the naming.

radical added 4 commits February 18, 2021 22:09

[wasm] reduce visibility of [AOT] /foo/bar.dll messages
68b16f5

[wasm] Allow skipping AOT, per assembly with %(InterpOnly) metadata
258698c

[wasm] Add tools/create-pack.proj for creating workload packs
4ad57fb

radical requested a review from marek-safar as a code owner February 19, 2021 05:03

ghost added the area-Build-mono label Feb 19, 2021

radical requested review from lewing, pranavkm, steveisok and vargaz February 19, 2021 05:03

pranavkm reviewed Feb 19, 2021
View reviewed changes

radical added 2 commits February 19, 2021 12:21

Merge remote-tracking branch 'origin/master' into aot-interp
8eb46e8

[wasm] Rename @(WasmAssemblies) -> @(WasmAssembly)
416aee4

steveisok reviewed Feb 19, 2021
View reviewed changes

Comment threadsrc/mono/wasm/tools/create-pack.proj

radical added 3 commits February 19, 2021 13:35

[wasm] don't copy dotnet.* to appbundle unnecessarily
e875bfd
This was added earlier to support blazor workload which only uses AOT. But that has since been fixed to correctly use `WasmBuildApp`+`@(WasmNativeAsset)`, so this can be removed.

fix md linter error
0e3cf66

[wasm] %(WasmAssembly.InterpOnly) -> %(WasmAssembly.InterpreterOnly)
8fa3f07

radical added the arch-wasm WebAssembly architecture label Feb 24, 2021

radical requested a review from pranavkm February 24, 2021 17:07

radical mentioned this pull request Feb 24, 2021
[wasm] Improve AOT builds #48718
Closed

radicaland others added 8 commits February 24, 2021 22:29

[wasm] quote file paths in shell invocations
cbcfa78

[wasm] Quote assembly path when invoking mono-aot-cross
cbe956e

[wasm] avoid trailing windows slash escaping the ending double quote
727b79b

Update src/mono/wasm/build/WasmApp.targets
d2e8c08
Co-authored-by: Pranav K <prkrishn@hotmail.com>

[wasm] Use the msbuild equivalent of Path.Combine in the targets/props
ad80e4f
.. and enable it by default.

[wasm] WasmApp.targets: Add $(WasmNativeDebugSymbols) to build dotnet…
9677b40
….wasm with symbols .. defaults to True.

[wasm] Build dotnet.wasm with symbols, even in Release if NOSTRIP is …
2c6734c
…not set

Merge remote-tracking branch 'origin/master' into quote-paths
b12c9d2

radical force-pushed the aot-build branch from c89520e to b12c9d2Compare February 25, 2021 03:43

radical added 5 commits February 24, 2021 22:58

[wasm] Run the build native step for %(InterpreterOnly) assemblies too
a11c3be

[wasm] rename %(InterpreterOnly) -> %(_Unsupported_InterpreterOnly_)
332bff1

[wasm] %(_Unsupported_InterpreterOnly_) -> %(_InternalForceInterpret)
0ea4b40

[wasm] dedupe @(WasmAssembly)
6175119

radical requested a review from steveisok February 25, 2021 18:54

lewing reviewed Feb 26, 2021
View reviewed changes

runfoappBot mentioned this pull request Feb 26, 2021
Tests failures: System.Net.NameResolution.Tests / DnsObsolete* & Dns_GetHostEntry* #1488
Open

Base automatically changed from master to mainMarch 1, 2021 09:08

radicaland others added 5 commits March 1, 2021 13:09

Merge remote-tracking branch 'origin/master' into quote-paths
0c101b0

Merge remote-tracking branch 'origin/main' into quote-paths
4b922d7

Merge branch 'main' into aot-build
92da568

[wasm] Check that EMSDK_PATH is set for WasmBuildNative
687b163

wasm] Disable il stripping completely
422048c
This uses `mono-cil-strip` from a mono installation. And in it's current form it can cause issues, so disabling it for now.

This was referenced Mar 8, 2021
[wasm] Add Wasm.Build tests, for testing wasm app builds #47683
Merged
[wasm] Allow assemblies to be skipped for AOTMode=AotInterp #49446
Merged

radical closed this Mar 15, 2021

ghost locked as resolved and limited conversation to collaborators Apr 14, 2021

karelz added this to the 6.0.0 milestone May 20, 2021

	/**
	* Allows control over our AOT (Ahead-of-time) compilation mode.
	*/
	typedefenum {
	/* Disables AOT mode */
	MONO_AOT_MODE_NONE,
	/* Enables normal AOT mode, equivalent to mono_jit_set_aot_only (false) */
	MONO_AOT_MODE_NORMAL,
	/* Enables hybrid AOT mode, JIT can still be used for wrappers */
	MONO_AOT_MODE_HYBRID,
	/* Enables full AOT mode, JIT is disabled and not allowed,
	* equivalent to mono_jit_set_aot_only (true) */
	MONO_AOT_MODE_FULL,
	/* Same as full, but use only llvm compiled code */
	MONO_AOT_MODE_LLVMONLY,
	/* Uses Interpreter, JIT is disabled and not allowed,
	* equivalent to "--full-aot --interpreter" */
	MONO_AOT_MODE_INTERP,
	/* Same as INTERP, but use only llvm compiled code */
	MONO_AOT_MODE_INTERP_LLVMONLY,
	/* Use only llvm compiled code, fall back to the interpeter */
	MONO_AOT_MODE_LLVMONLY_INTERP,
	/* Same as --interp */
	MONO_AOT_MODE_INTERP_ONLY,
	/* Sentinel value used internally by the runtime. We use a large number to avoid clashing with some internal values. */
	MONO_AOT_MODE_LAST=1000,
	} MonoAotMode;

Uh oh!

Conversation

radical commented Feb 19, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ghost commented Feb 24, 2021

Uh oh!

radical commented Feb 25, 2021

Uh oh!

radical commented Feb 25, 2021

Uh oh!

lewing left a comment

Choose a reason for hiding this comment

Uh oh!

lewing commented Feb 26, 2021

Uh oh!

radical commented Mar 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

radical commented Feb 19, 2021•
edited
Loading