[wasm] Build improvements based on feedback #59391

radical · 2021-09-21T03:08:09Z

Fix for a incremental build issue
Change native build defaults for Debug config, to make it faster (thanks @SteveSandersonMS, @mattleibow)
MonoAOTCompiler:
- Skip unmanaged assemblies, and emit a warning
PInvokeTableGenerator:
- Fix to not fail in presence of native variadic functions, and pinvokes with function pointers (eg. used by sqlite)

ghost · 2021-09-21T03:08:26Z

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

Issue Details

null

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

src/mono/wasm/build/WasmApp.Native.targets

The task needs to run `mono-aot-cross` on all the assemblies, even if a given assembly is older than its output files, so as to correctly handle changes in its dependencies. This commit tries to check if all the input assemblies are up-to-date with their corresponding output files, and avoids doing any work if that's true.

`WasmNativeStrip` -> `false` Compile, and link optimization flags for `emcc` to `-O1`

.. rebuilds. Specifically, when a rebuild causes only the *linking* part to be run again. In that case, we were correctly skipping over compiling native files, but didn't add them to `@(FileWrites)`, which caused msbuild's incremental clean logic to treat them as "orphaned" files, and delete them!

Co-authored-by: Larry Ewing <lewing@microsoft.com>

- Ensure that the cache gets written even when some of the files failed to compile - And when some were skipped, and others failed

dotnet#59538

For non-publish builds, undefined symbols will show up as warnings: ``` EXEC : warning : undefined symbol: sqlite3_column_database_name (referenced by top-level compiled C/C++ code) [/BlazorWebAssemblySqlite.csproj] EXEC : warning : undefined symbol: sqlite3_column_origin_name (referenced by top-level compiled C/C++ code) [/BlazorWebAssemblySqlite.csproj] EXEC : warning : undefined symbol: sqlite3_column_table_name (referenced by top-level compiled C/C++ code) [/BlazorWebAssemblySqlite.csproj] EXEC : warning : undefined symbol: sqlite3_snapshot_cmp (referenced by top-level compiled C/C++ code) [/BlazorWebAssemblySqlite.csproj] EXEC : warning : undefined symbol: sqlite3_snapshot_free (referenced by top-level compiled C/C++ code) [/BlazorWebAssemblySqlite.csproj] EXEC : warning : undefined symbol: sqlite3_snapshot_get (referenced by top-level compiled C/C++ code) [/BlazorWebAssemblySqlite.csproj] EXEC : warning : undefined symbol: sqlite3_snapshot_open (referenced by top-level compiled C/C++ code) [/BlazorWebAssemblySqlite.csproj] EXEC : warning : undefined symbol: sqlite3_snapshot_recover (referenced by top-level compiled C/C++ code) [/BlazorWebAssemblySqlite.csproj] ```

Currently, for a variadic function like: `int sqlite3_config(int, ...);` .. and multiple pinvokes like: ```csharp [DllImport(SQLITE_DLL, ExactSpelling=true, EntryPoint = "sqlite3_config", CallingConvention = CALLING_CONVENTION)] public static extern unsafe int sqlite3_config_none(int op); [DllImport(SQLITE_DLL, ExactSpelling=true, EntryPoint = "sqlite3_config", CallingConvention = CALLING_CONVENTION)] public static extern unsafe int sqlite3_config_int(int op, int val); [DllImport(SQLITE_DLL, ExactSpelling=true, EntryPoint = "sqlite3_config", CallingConvention = CALLING_CONVENTION)] public static extern unsafe int sqlite3_config_log(int op, NativeMethods.callback_log func, hook_handle pvUser); ``` .. we generate: ```c int sqlite3_config (int); int sqlite3_config (int,int); int sqlite3_config (int,int,int); ``` .. which fails to compile. Instead, this patch will generate a variadic declaration with one fixed parameter: ```c // Variadic signature created for // System.Int32 sqlite3_config_none(System.Int32) // System.Int32 sqlite3_config_int(System.Int32, System.Int32) // System.Int32 sqlite3_config_log(System.Int32, SQLitePCL.SQLite3Provider_e_sqlite3+NativeMethods+callback_log, SQLitePCL.hook_handle) int sqlite3_config (int, ...); ``` TODO: functions with different first argument

- For pinvokes with function pointers, *no* declaration is added to `pinvoke-table.h`, and a warning is raised: ``` warning : DllImports with function pointers are not supported. Calling them will fail. Managed DllImports: [/Users/radical/dev/r2/artifacts/bin/Wasm.Build.Tests/net6.0-Release/browser-wasm/g3acrk4b.a0o/variadic_g3acrk4b.a0o.csproj] warning : Type: Test, Method: System.Int32 using_sum_one(?) [/Users/radical/dev/r2/artifacts/bin/Wasm.Build.Tests/net6.0-Release/browser-wasm/g3acrk4b.a0o/variadic_g3acrk4b.a0o.csproj] ``` - Also, handle multiple pinvokes with the same number of params, but different types

- pinvokes with function pointers could be for variadic functions too - if there are multiple pinvokes for the same native function, but some of them have function pointers, then ignore those but generate the declaration for the rest - raise better warnings - and emit info about the variadic pinvokes in pinvoke-table.h

…e" check

…p-to-date" check" Newer timestamp, but unchanged file is an indication of a build issue, and doesn't need to be handled in this task. This reverts commit 712dbb3.

radical · 2021-09-27T23:26:50Z

Android failure is #59679
Coreclr failures are #10680

Unrelated to this PR.

Based on feedback from Zoltan

radical · 2021-09-28T06:24:36Z

wasm test failure is #48717

radical · 2021-09-28T20:55:49Z

Coreclr failures are #10680 , unrelated to this PR.

radical · 2021-09-29T21:26:02Z

Android failure is #59779 .

radical · 2021-09-30T04:57:36Z

/backport release/6.0

radical · 2021-09-30T04:58:36Z

/backport to release/6.0

github-actions · 2021-09-30T04:58:48Z

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1289670399

github-actions · 2021-09-30T05:00:16Z

@radical an error occurred while backporting to release/6.0, please check the run log for details!

Error: @radical is not a repo collaborator, backporting is not allowed.

@SteveSandersonMS

- Fix for a incremental build issue - Change native build defaults for `Debug` config, to make it faster (thanks @SteveSandersonMS, @mattleibow) - MonoAOTCompiler: - Skip unmanaged assemblies, and emit a warning - PInvokeTableGenerator: - Fix to not fail in presence of native variadic functions, and pinvokes with function pointers (eg. used by sqlite) Co-authored-by: Larry Ewing <lewing@microsoft.com> (cherry picked from commit 80f015d)

Backport of #59391 - Fix to handle pinvokes with function pointers, which resolves issue found testing new customer scenario with sqlite - Fixes version of System.Reflection.MetadataLoadContext bundled with WasmBuilderApp task, to use the same as other tasks - Fix for an incremental build issue where the compiled native .o files would incorrectly get deleted after linking. And that would cause them to be recompiled, unnecessarily increasing the build time. - Change the default optimization flag used when building for Debug config from -Oz to -O1 to improve development loop experience.

ghost added the area-Build-mono label Sep 21, 2021

radical added the arch-wasm WebAssembly architecture label Sep 21, 2021

radical force-pushed the wasm-improvements branch from 28130de to c85b5ca Compare September 21, 2021 06:07

radical marked this pull request as ready for review September 21, 2021 20:05

radical requested a review from marek-safar as a code owner September 21, 2021 20:05

radical requested review from steveisok, lewing and akoeplinger September 21, 2021 20:06

radical force-pushed the wasm-improvements branch 2 times, most recently from 8f566c3 to 0da5608 Compare September 22, 2021 06:42

radical changed the title ~~[wasm] Misc improvements~~ [wasm] Build improvements based on feedback Sep 22, 2021

radical requested a review from jonathanpeppers September 22, 2021 14:08

lewing reviewed Sep 22, 2021

View reviewed changes

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

radical commented Sep 22, 2021

View reviewed changes

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

radical and others added 15 commits September 25, 2021 19:05

Refactor to allow fast-path

5f569e3

[wasm] Change optimization flag defaults for Debug config

28fdcba

`WasmNativeStrip` -> `false` Compile, and link optimization flags for `emcc` to `-O1`

MonoAOTCompiler: Skip unmanaged assemblies, and emit a warning

08ae443

Apply suggestions from code review

7fe0e69

Co-authored-by: Larry Ewing <lewing@microsoft.com>

MonoAOTCompiler: write the cache even when some files fail to compile

1c0a340

- Ensure that the cache gets written even when some of the files failed to compile - And when some were skipped, and others failed

Don't set optimization defaults for Debug config, when publishing

7230cd3

Wasm.Build.Tests: Disable net5.0 because they can't be tested right now

c71f9c2

dotnet#59538

add missing variadic.{c,o}

aac1537

[wasm] Add test for issue dotnet#59255

f48d6b2

radical added 2 commits September 27, 2021 13:07

MonoAOTCompiler: Check the hash for the file also, for "all up-to-dat…

712dbb3

…e" check

Merge remote-tracking branch 'origin/main' into wasm-improvements

b23e11e

radical requested a review from vargaz September 27, 2021 18:50

Revert "MonoAOTCompiler: Check the hash for the file also, for "all u…

dc09bf6

…p-to-date" check" Newer timestamp, but unchanged file is an indication of a build issue, and doesn't need to be handled in this task. This reverts commit 712dbb3.

PInvokeTableGenerator: don't generate any decl for variadic functions

41c7476

Based on feedback from Zoltan

steveisok approved these changes Sep 28, 2021

View reviewed changes

runfoapp bot mentioned this pull request Sep 28, 2021

CopyToNanosecondsPresent_LowTempRes #48717

Closed

radical added 4 commits September 29, 2021 13:22

Add missing using for disposable objects.

dcbcc35

cleanup

1f57975

Merge remote-tracking branch 'origin/main' into rf-wasm-improvements

4519646

address feedback from @lewing

20a67a1

radical mentioned this pull request Sep 29, 2021

Android x64: Loader.classloader runtime tests failing with Error trying to get device: cmd: Can't find service: package #59779

Closed

lewing approved these changes Sep 30, 2021

View reviewed changes

radical merged commit 80f015d into dotnet:main Sep 30, 2021

radical deleted the wasm-improvements branch September 30, 2021 04:53

radical mentioned this pull request Sep 30, 2021

[release/6.0] Build improvements based on feedback #59671

Merged

This was referenced Sep 30, 2021

Test failure tracing/eventpipe/providervalidation/providervalidation/providervalidation.sh #59296

Closed

Test failure tracing/eventpipe/rundownvalidation/rundownvalidation/rundownvalidation.sh #54801

Closed

ghost locked as resolved and limited conversation to collaborators Nov 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[wasm] Build improvements based on feedback #59391

[wasm] Build improvements based on feedback #59391

Uh oh!

radical commented Sep 21, 2021 •

edited

Loading

Uh oh!

ghost commented Sep 21, 2021

Uh oh!

Uh oh!

Uh oh!

radical commented Sep 27, 2021 •

edited

Loading

Uh oh!

radical commented Sep 28, 2021

Uh oh!

radical commented Sep 28, 2021

Uh oh!

radical commented Sep 29, 2021

Uh oh!

radical commented Sep 30, 2021

Uh oh!

radical commented Sep 30, 2021

Uh oh!

github-actions bot commented Sep 30, 2021

Uh oh!

github-actions bot commented Sep 30, 2021

Uh oh!

Uh oh!

[wasm] Build improvements based on feedback #59391

[wasm] Build improvements based on feedback #59391

Uh oh!

Conversation

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

Uh oh!

Uh oh!

ghost commented Sep 21, 2021

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

radical commented Sep 28, 2021

Uh oh!

radical commented Sep 28, 2021

Uh oh!

radical commented Sep 29, 2021

Uh oh!

radical commented Sep 30, 2021

Uh oh!

radical commented Sep 30, 2021

Uh oh!

github-actions bot commented Sep 30, 2021

Uh oh!

github-actions bot commented Sep 30, 2021

Uh oh!

Uh oh!

radical commented Sep 21, 2021 •

edited

Loading

radical commented Sep 27, 2021 •

edited

Loading