Skip to content

[browser] load core assemblies first#100141

Merged
pavelsavara merged 18 commits into
dotnet:mainfrom
pavelsavara:browser_core_assemblies_first
May 15, 2024
Merged

[browser] load core assemblies first#100141
pavelsavara merged 18 commits into
dotnet:mainfrom
pavelsavara:browser_core_assemblies_first

Conversation

@pavelsavara

@pavelsavara pavelsavara commented Mar 22, 2024

Copy link
Copy Markdown
Member

start MonoVM when essential DLLs are downloaded and continue downloading the rest of the assets

  • System.Private.CoreLib
  • System.Collections
  • System.Runtime.InteropServices.JavaScript
  • runtimeconfig.bin
  • dotnet.native.js.symbols if present

I estimate that this could shave 50ms from cold start.

other changes

  • fix virtualWorkingDirectory
  • make assets relative to / when not absolute
  • when only config.assets

Resurrection of #93480

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm labels Mar 22, 2024
@pavelsavara pavelsavara added this to the 9.0.0 milestone Mar 22, 2024
@pavelsavara pavelsavara requested a review from kg March 22, 2024 12:47
@pavelsavara pavelsavara self-assigned this Mar 22, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@lambdageek

Copy link
Copy Markdown
Member

@kg this is close to what you've been thinking about recently

@kg

kg commented Mar 22, 2024

Copy link
Copy Markdown
Contributor

thanks for doing the hard work on this! i agree on your estimated startup reduction, should be a win

Comment thread src/mono/mono/metadata/bundled-resources.c Outdated
@pavelsavara

Copy link
Copy Markdown
Member Author

we may need runtimeconfig.bin to be loaded into VFS before we start mono. And I'm not sure about ICU data.

@kg

kg commented Mar 22, 2024

Copy link
Copy Markdown
Contributor

we may need runtimeconfig.bin to be loaded into VFS before we start mono. And I'm not sure about ICU data.

ICU data will only be needed if any runtime startup loads it. JS interop does with an accidental string operation right now. We could probably make it assert if there's an attempt to load ICU during this warming phase

@lewing

lewing commented Mar 22, 2024

Copy link
Copy Markdown
Member

I'm not sure if my recollection is correct but I think we explicitly load the icu data earlier than we need to as well. It might be worth exploring if we can delay it longer.

@pavelsavara

Copy link
Copy Markdown
Member Author

I'm not sure if my recollection is correct but I think we explicitly load the icu data earlier than we need to as well. It might be worth exploring if we can delay it longer.

This currently postpones all but core DLLs

Comment thread src/mono/browser/runtime/loader/assets.ts Outdated
Comment thread src/mono/browser/runtime/loader/assets.ts Outdated
@pavelsavara

Copy link
Copy Markdown
Member Author

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara

Copy link
Copy Markdown
Member Author

Right now, this PR is not AOT friendly because AOT assumes that metadata would be available for all images.

@pavelsavara

Copy link
Copy Markdown
Member Author

we need System.Threading.Channels to be part of the core for MT build. @maraf

@pavelsavara

Copy link
Copy Markdown
Member Author

Current CI fails with

[20:31:58] fail: [0x0118c038-dpty 20:31:58.466] MONO_WASM: start_runtime() failed {}

There is mono_runtime_class_init_full on stack for most of them.

@maraf

maraf commented May 8, 2024

Copy link
Copy Markdown
Member
  • make assets relative to / when not absolute

Related #99539

Comment thread src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.cs Outdated
Comment thread src/tasks/WasmAppBuilder/WasmAppBuilder.cs Outdated

@kg kg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parts I understand look good, but I don't totally grasp all the threading and asset load parts

@maraf

maraf commented May 13, 2024

Copy link
Copy Markdown
Member

This is now unblocked in SDK

@maraf maraf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

Comment thread src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.cs Outdated
@pavelsavara

Copy link
Copy Markdown
Member Author

I filled #102196 and #102195

@pavelsavara

Copy link
Copy Markdown
Member Author

I filled #102251

@pavelsavara

Copy link
Copy Markdown
Member Author

/ba-g all issues are known

@pavelsavara pavelsavara merged commit 8698d3d into dotnet:main May 15, 2024
@pavelsavara pavelsavara deleted the browser_core_assemblies_first branch May 15, 2024 10:12
@pavelsavara

Copy link
Copy Markdown
Member Author

60ms for firefox and 30ms for chrome during the cold start.
image

Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this pull request May 30, 2024
Co-authored-by: Ilona Tomkowicz <itomkowicz@microsoft.com>
Co-authored-by: Marek Fišera <mara@neptuo.com>
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants