[wasm] Allow assemblies to be skipped for AOTMode=AotInterp#48500

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

eng/testing/tests.mobile.targets

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

src/mono/wasm/build/WasmApp.targets

    </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.

*/

typedef enum {

/* 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

src/mono/wasm/tools/create-pack.proj

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

src/mono/wasm/tools/create-pack.proj Show resolved Hide resolved

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

radical and 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 b12c9d2 Compare 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

runfoapp bot mentioned this pull request Feb 26, 2021

Tests failures: System.Net.NameResolution.Tests / DnsObsolete* & Dns_GetHostEntry* #1488

Open

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

radical and 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.
	*/
	typedef enum {
	/* 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;

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