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

[browser] load core assemblies first #100141

Merged
merged 18 commits into from
May 15, 2024

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Mar 22, 2024

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
Copy link
Contributor

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

@lambdageek
Copy link
Member

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

@kg
Copy link
Contributor

kg commented Mar 22, 2024

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

@pavelsavara
Copy link
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
Copy link
Contributor

kg commented Mar 22, 2024

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
Copy link
Member

lewing commented Mar 22, 2024

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

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

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

@pavelsavara
Copy link
Member Author

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

@pavelsavara
Copy link
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
Copy link
Member

maraf commented May 8, 2024

  • make assets relative to / when not absolute

Related #99539

Copy link
Contributor

@kg kg left a comment

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
Copy link
Member

maraf commented May 13, 2024

This is now unblocked in SDK

Copy link
Member

@maraf maraf left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@pavelsavara
Copy link
Member Author

I filled #102196 and #102195

@pavelsavara
Copy link
Member Author

I filled #102251

@pavelsavara
Copy link
Member Author

/ba-g all issues are known

@pavelsavara pavelsavara merged commit 8698d3d into dotnet:main May 15, 2024
157 of 159 checks passed
@pavelsavara pavelsavara deleted the browser_core_assemblies_first branch May 15, 2024 10:12
@pavelsavara
Copy link
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

None yet

6 participants