[release/6.0-rc2][wasm] Fixes, and improvements based on user feedback #59486

radical · 2021-09-22T17:31:18Z

This contains:

Fix to handle pinvokes with function pointers, which unblocks using native sqlite library
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 to the default optimization flag used when building for Debug config
from -Oz to -O1

Customer impact

Unblocks being able to use sqlite with blazorwasm
Improved incremental build experience.
Faster debug builds. Time for building a blazorwasm template
project with a native library goes down from 22s to 12s.

Testing

Manual testing, existing tests, and specific new tests.

Risk

Low. Improves build experience.

`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!

.. and remove the Blazor sdk targets hack used for testing, since the updated SDK has the fixes.

ghost · 2021-09-22T17:31:25Z

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

Issue Details

EmccCompile: Fix incremental build, in case of only partial rebuilds (reported by blazor team)
Change optimization flag defaults for Debug config which significantly speeds up Debug builds
- WasmNativeStrip -> false
- Compile, and link optimization flags for emcc to -O1

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

Co-authored-by: Larry Ewing <[email protected]>

radical · 2021-09-23T04:10:06Z

The test failures are due to missing Microsoft.NETCore.App.Runtime.browser-wasm version 5.0.11 on nuget.org . The version is embedded in the latest rc2 sdk, which we use for testing the workloads.
The latest version on nuget.org seems to be 5.0.10.

It fails to restore a blazorwasm project:

error NU1102: Unable to find package Microsoft.NETCore.App.Runtime.browser-wasm with version (= 5.0.11)
error NU1102:   - Found 1110 version(s) in dotnet6 [ Nearest version: 6.0.0-alpha.1.20423.5 ]
error NU1102:   - Found 20 version(s) in nuget.org [ Nearest version: 6.0.0-preview.1.21102.12 ]
Failed to restore

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

This reverts commit c2174f3.

Anipik · 2021-09-27T18:30:42Z

@marek-safar should we re-target this to 6.0 or is it blocking for rc2 ?

radical · 2021-09-27T19:29:35Z

cc @SteveSandersonMS

SteveSandersonMS · 2021-09-27T20:05:24Z

RC2 would definitely help us get better feedback about this feature. But if it’s a big problem I expect we could live with 6.0.

radical · 2021-09-27T20:26:15Z

Closing in favor of #59671 .

radical added 3 commits September 22, 2021 13:18

[wasm] Change optimization flag defaults for Debug config

ae21b90

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

Bump sdk for workload testing to 6.0.100-rc.2.21472.15

32f9504

.. and remove the Blazor sdk targets hack used for testing, since the updated SDK has the fixes.

radical added arch-wasm WebAssembly architecture area-Build-mono labels Sep 22, 2021

radical requested review from lewing, radekdoulik and steveisok September 22, 2021 17:31

radical requested a review from marek-safar as a code owner September 22, 2021 17:31

radical and others added 3 commits September 22, 2021 15:36

Apply suggestions from code review

2984d86

Co-authored-by: Larry Ewing <[email protected]>

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

67cda93

Merge remote-tracking branch 'origin/release/6.0-rc2' into fixes-rc2

948b3ce

radical added 10 commits September 23, 2021 16:25

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

56da550

dotnet#59538

Bump sdk for workload testing to 6.0.100-rc.2.21473.29

ed2cc52

Merge remote-tracking branch 'origin/release/6.0-rc2' into fixes-rc2

35d8bb8

add missing variadic.{c,o}

8d87404

disable unrelated builds

c2174f3

Merge remote-tracking branch 'origin/release/6.0-rc2' into fixes-rc2

e5199ed

radical mentioned this pull request Sep 25, 2021

[release/6.0-rc2][wasm] Bump sdk version for workload testing to 6.0.100-rc.2.21467.10 #59279

Closed

Revert "disable unrelated builds"

5413e54

This reverts commit c2174f3.

lewing changed the base branch from release/6.0-rc2 to release/6.0 September 27, 2021 19:42

lewing requested a review from BrzVlad as a code owner September 27, 2021 19:42

lewing requested review from eiriktsarpalis, lambdageek, layomia, steveharter, thaystg and vargaz as code owners September 27, 2021 19:42

radical changed the base branch from release/6.0 to release/6.0-rc2 September 27, 2021 19:48

radical closed this Sep 27, 2021

radical deleted the fixes-rc2 branch September 27, 2021 20:26

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!

[release/6.0-rc2][wasm] Fixes, and improvements based on user feedback #59486

[release/6.0-rc2][wasm] Fixes, and improvements based on user feedback #59486

Uh oh!

radical commented Sep 22, 2021 •

edited

Loading

Uh oh!

ghost commented Sep 22, 2021

Uh oh!

radical commented Sep 23, 2021 •

edited

Loading

Uh oh!

Anipik commented Sep 27, 2021

Uh oh!

radical commented Sep 27, 2021

Uh oh!

SteveSandersonMS commented Sep 27, 2021

Uh oh!

radical commented Sep 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[release/6.0-rc2][wasm] Fixes, and improvements based on user feedback #59486

[release/6.0-rc2][wasm] Fixes, and improvements based on user feedback #59486

Uh oh!

Conversation

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

Uh oh!

Customer impact

Testing

Risk

Uh oh!

ghost commented Sep 22, 2021

Uh oh!

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

Uh oh!

Uh oh!

Anipik commented Sep 27, 2021

Uh oh!

radical commented Sep 27, 2021

Uh oh!

SteveSandersonMS commented Sep 27, 2021

Uh oh!

radical commented Sep 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

radical commented Sep 22, 2021 •

edited

Loading

radical commented Sep 23, 2021 •

edited

Loading