Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wasm] Add incremental build support #57113

Merged
merged 30 commits into from
Aug 17, 2021
Merged

Conversation

radical
Copy link
Member

@radical radical commented Aug 10, 2021

  • Adds Inputs/Outputs to wasm targets, which msbuild can use for dependency checking

  • Updates tasks to write out to the files being used as inputs, only when there is a change

  • MonoAOTCompiler:

    • Compiles assemblies to .bc files.
    • Hashes for the .bc files are stored in a json cache file.
    • mono-aot-cross is used to compile .dll -> .bc.tmp
    • And the actual .bc file is overwritten only if the generated one is different
    • note: the cache is optional. this can be changed to be required, if iOS/android also adopt it
  • EmccCompile.cs: Support a %(Dependencies) metadata on the source
    files, to compile the files only when needed.

  • All the emcc args are written to rsp files, if different

    • and used as inputs/dependencies for the native output files
  • Adds some tests

  • Use msbuild's resource API, to request cores for building in parallel

  • Also, updates the build output:

  Wasm.Console.Sample -> /Users/radical/dev/r2/src/mono/sample/wasm/console/bin/browser-wasm/publish/
  AOT'ing 7 assemblies
  [1/7] Wasm.Console.Sample.dll -> Wasm.Console.Sample.dll.bc
  [6/7] skipped unchanged assemblies.
  Compiling native assets with emcc. This may take a while ...
  Compiling assembly bitcode files...
  [1/1] Wasm.Console.Sample.dll.bc -> Wasm.Console.Sample.dll.o [took 0.612s]
  Linking with emcc. This may take a while ...

Fixes #51013

@ghost
Copy link

ghost commented Aug 10, 2021

Tagging subscribers to this area: @directhex
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: radical
Assignees: -
Labels:

area-Infrastructure-mono

Milestone: -

MonoAOTCompiler:
- Compiles assemblies to .bc files.
- Hashes for the .bc files are stored in a json cache file.
- And uses .depfile generated by mono-aot-cross, to figure out the
  if any of the dependencies have changed
- Writes out the actual .bc file only if the assembly, or it's
  dependencies changed

EmccCompile.cs: Support a `%(Dependencies)` metadata on the source
files, to compile the files only when needed.
@radical radical added the arch-wasm WebAssembly architecture label Aug 10, 2021
@ghost
Copy link

ghost commented Aug 10, 2021

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

Issue Details
  • Adds Inputs/Outputs to wasm targets, which msbuild can use for dependency checking

  • Updates tasks to write out to the files being used as inputs, only when there is a change

  • MonoAOTCompiler:

    • Compiles assemblies to .bc files.
    • Hashes for the .bc files are stored in a json cache file.
    • And uses .depfile generated by mono-aot-cross, to figure out the
      if any of the dependencies have changed
    • Writes out the actual .bc file only if the assembly, or it's
      dependencies changed
  • EmccCompile.cs: Support a %(Dependencies) metadata on the source
    files, to compile the files only when needed.

  • All the emcc args are written to rsp files, if different

    • and used as inputs/dependencies for the native output files
  • Adds some tests

Fixes #51013

Author: radical
Assignees: -
Labels:

arch-wasm, area-Build-mono

Milestone: -

@radical radical marked this pull request as ready for review August 11, 2021 06:55
@radical
Copy link
Member Author

radical commented Aug 11, 2021

Splitting up the tests into separate jobs. So, disabled non-wasm jobs, to make sure that it's working.

@radical
Copy link
Member Author

radical commented Aug 11, 2021

Ready for review

@radical radical added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Aug 11, 2021
@radical radical removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Aug 11, 2021
`--depfile` isn't supported on aot config used by android, and fails
with:

```
* Assertion at /__w/1/s/src/mono/mono/mini/aot-compiler.c:14216, condition `acfg->aot_opts.llvm_only && acfg->aot_opts.asm_only && acfg->aot_opts.llvm_outfile' not met
```

Instead, use hashes of the .bc.tmp files generated, with the existing
.bc files.
@lewing
Copy link
Member

lewing commented Aug 15, 2021

failure in runtime (Libraries Test Run checked coreclr windows x86 Release) is

C:\h\w\A8F309AF\w\A8620915\e>"C:\h\w\A8F309AF\p\dotnet.exe" exec --runtimeconfig System.Diagnostics.Process.Tests.runtimeconfig.json --depsfile System.Diagnostics.Process.Tests.deps.json xunit.console.dll System.Diagnostics.Process.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing  
  Discovering: System.Diagnostics.Process.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Diagnostics.Process.Tests (found 254 of 278 test cases)
  Starting:    System.Diagnostics.Process.Tests (parallel test collections = on, max threads = 4)
    System.Diagnostics.Tests.ProcessModuleTests.LongModuleFileNamesAreSupported [FAIL]
      Assert.Contains() Failure
      Not found: C:\h\w\A8F309AF\t\ProcessModuleTests_otibjh1e.nwu\aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\LongPath.dll
      In value:  String[] ["C:\\h\\w\\A8F309AF\\p\\dotnet.exe", "C:\\Windows\\SYSTEM32\\ntdll.dll", "C:\\Windows\\System32\\KERNEL32.DLL", "C:\\Windows\\System32\\KERNELBASE.dll", "C:\\Windows\\System32\\ucrtbase.dll", ...]
      Stack Trace:
        /_/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs(127,0): at System.Diagnostics.Tests.ProcessModuleTests.LongModuleFileNamesAreSupported()
    System.Diagnostics.Tests.ProcessStartInfoTests.ShellExecute_Nano_Fails_Start [SKIP]
      Condition(s) not met: "IsWindowsNanoServer"
Invalid number of parameters
0 File(s) copied
  Finished:    System.Diagnostics.Process.Tests
=== TEST EXECUTION SUMMARY ===

@lewing
Copy link
Member

lewing commented Aug 15, 2021

created #57452

@lewing lewing merged commit 3e3b00c into dotnet:main Aug 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 16, 2021
@radical radical deleted the wasm-incremental-build branch August 8, 2023 21:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Build-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[wasm][aot] Incremental builds don't work correctly for AOT
2 participants