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

[wasi+browser] bundle timezones into .wasm #82250

Merged
merged 26 commits into from
Mar 1, 2023

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Feb 16, 2023

Motivation

  • the VFS is slow in the browser. I expect to save 50ms of startup time
  • this will bring de-duplication of files with same hash and bit smaller overall download size.
  • it will enable memory snapshot of the initialized runtime
  • necessary for WASI with <WasmSingleFileBundle>

Changes

  • TimeZoneInfo will use this new embedded TZ database for browser and wasi OS
    • unless TZDIR env variable is set
    • if TZDIR is set it would fallback to unix-like file system approach (as before).
  • added mono_wasm_get_bundled_file and mono_wasm_add_bundled_file
  • mono_wasm_add_bundled_file is called with file names from generated mono_wasm_register_bundle_timezones
  • added SystemNative_GetTimeZoneData which calls new mono_wasm_get_bundled_file
  • in of <WasmSingleFileBundle> we also generate mono_wasm_register_bundle_assemblies for WASI
  • removed dotnet.timezones.blat from runtime pack
  • added wasm-bundled-timezones.a to workloads
  • fixed TARGET_BROWSER detection for native/libs
  • do not set TZ env variable to UTC as that's not valid timezone anyway. dotnet will fallback to UTC.
  • create usr/share on browser VFS because it's mapped to SpecialFolder.CommonApplicationData which would not exist otherwise.
  • rewrote EmitWasmBundleObjectFile as new EmitWasmBundleFiles
    • it can bundle any file not just assemblies
    • it can generate code to to wasi clang input stream or to files for emcc
    • it skips files which are already converted to
  • new Target GenerateTimezonesArchive in both wasi.proj and wasm.proj
    • find all files to bundle, get hash for them
    • pass the list to EmitWasmBundleFiles Task
      • [for wasi] object files will be produced
      • [for emcc] compile source files to obj file after this
    • remove old source and object files
  • updated Target _GenerateAssemblyObjectFiles to use new EmitWasmBundleFiles task

future work

  • this will need SDK change for blazor
  • next PR make BlazorEnableTimeZoneSupport work again,
  • make file bundling more incremental
  • similar bundling could be used for WASI and ICU data files

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture os-wasi Related to WASI variant of arch-wasm labels Feb 16, 2023
@pavelsavara pavelsavara added this to the 8.0.0 milestone Feb 16, 2023
@pavelsavara pavelsavara self-assigned this Feb 16, 2023
@ghost
Copy link

ghost commented Feb 16, 2023

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

Issue Details

work in progress

Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, os-wasi

Milestone: 8.0.0

@pavelsavara pavelsavara force-pushed the wasi_bundle_timezones branch 3 times, most recently from 18e709c to a45ecbf Compare February 21, 2023 17:00
@pavelsavara pavelsavara changed the title [wasi] bundle timezones into .wasm [wasi+browser] bundle timezones into .wasm Feb 21, 2023
@pavelsavara
Copy link
Member Author

pavelsavara commented Feb 22, 2023

I will need to deal with BlazorEnableTimeZoneSupport
See SDK and #37973

@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 27, 2023
@pavelsavara
Copy link
Member Author

@radical do you have further feedback ?

pavelsavara and others added 2 commits March 1, 2023 08:33
Co-authored-by: Ankit Jain <radical@gmail.com>
Copy link
Member

@radical radical left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Comment on lines +122 to +127
<RunWithEmSdkEnv Command="$(EmccCmd) -xc -c @$(_WasmTimezonesSourcesRsp)"
WorkingDirectory="$(WasmObjDir)"
EmSdkPath="$(EMSDK_PATH)"
ConsoleToMsBuild="true"
IgnoreStandardErrorWarningFormat="true">
</RunWithEmSdkEnv>
Copy link
Member

Choose a reason for hiding this comment

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

For a follow up PR, consider using EmccCompile task which compiles in parallel.

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@ilonatommy ilonatommy 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, I especially like the description ✨

src/mono/mono/utils/mono-dl-wasm.c Show resolved Hide resolved
@pavelsavara
Copy link
Member Author

CI failures are unrelated.

@pavelsavara
Copy link
Member Author

Original size of dotnet.timezones.blat was 336270 bytes.
The size of new data embedded in the dotnet.wasm is 221991 bytes.

Some of the startup speed improvement is also caused by it.
image

@ghost ghost locked as resolved and limited conversation to collaborators Apr 2, 2023
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 os-wasi Related to WASI variant of arch-wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants