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.

Sign up for GitHub

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

Jump to bottom

[wasm] Introduce <InvariantTimezone> build flag #87284

Merged
merged 10 commits into from
Jul 1, 2023
Merged

[wasm] Introduce <InvariantTimezone> build flag #87284

merged 10 commits into from
Jul 1, 2023

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Jun 8, 2023 •
edited
Loading

In Net8 we bundle the timezone database into .wasm file by default. It's not separate download anymore.

To to make download size of browser/wasi application smaller, we could link it out.
The runtime would behave as if there was no timezones in the database and use UTC.
For WASI, it doesn't try to load it from host FS unless you also set TZDIR env variable.

In the project file

<InvariantTimezone>true</InvariantTimezone>

Requires re-linking with wasm workload.

Also backward compatible with BlazorEnableTimeZoneSupport but with workload.

Fixes #37973 for wasm

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-Build-mono size-reduction Issues impacting final app size primary for size sensitive workloads labels Jun 8, 2023
@pavelsavara pavelsavara added this to the 8.0.0 milestone Jun 8, 2023
@pavelsavara pavelsavara self-assigned this Jun 8, 2023
@ghost
Copy link

ghost commented Jun 8, 2023

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

Issue Details

In Net8 we bundle the timezone database into .wasm file by default. It's not separate download anymore.

To to make download size of wasm/wasi application smaller, we could link it out.
The runtime would behave as if there was no timezones in the database and use UTC.
For WASI, it not try to load it from host FS unless you also set TZDIR env variable.

In the project file

<InvariantTimezone>true</InvariantTimezone>

Requires re-linking with wasm workload.

Also backward compatible with BlazorEnableTimeZoneSupport but with workload.

Fixes #37973 for wasm

Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, area-Build-mono, size-reduction
Milestone: 8.0.0

@maraf
Copy link
Member

maraf commented Jun 8, 2023

Is it the right time to have two WASM flavors/files in the runtime pack and choose between them during client app build?

@pavelsavara
Copy link
Member Author

I don't have bandwidth to implement it.

@pavelsavara pavelsavara mentioned this pull request Jun 12, 2023
Merged
@maraf maraf mentioned this pull request Jun 21, 2023
Closed
@pavelsavara pavelsavara force-pushed the wasm_invariant_timezone branch from 756d84c to cda65db Compare June 26, 2023 15:15
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@build-analysis build-analysis bot mentioned this pull request Jun 26, 2023
Closed
@pavelsavara
Merge branch 'main' into wasm_invariant_timezone
37b89ac
@pavelsavara
WBT
75a14d1
@pavelsavara
Merge branch 'main' into wasm_invariant_timezone
d6aa491 
# Conflicts:
#	src/mono/sample/wasm/browser-advanced/Wasm.Advanced.Sample.csproj
@pavelsavara
Copy link
Member Author

image

@pavelsavara pavelsavara marked this pull request as ready for review June 28, 2023 07:04
@pavelsavara
Copy link
Member Author

/azp run runtime-wasm-optional

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

maraf
maraf approved these changes Jun 28, 2023
View reviewed changes
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 👍

@kg
Copy link
Member

kg commented Jun 28, 2023

How big is the size savings from this? Just curious

kg
kg reviewed Jun 28, 2023
View reviewed changes
docs/design/features/timezone-invariant-mode.md Outdated

Author: [Pavel Savara](https://github.com/pavelsavara)

It's currently only available for Browser OS
Copy link
Member

Choose a reason for hiding this comment

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

Maybe specifically call out that it strips the tz data from the binary

@pavelsavara
Copy link
Member Author

How big is the size savings from this? Just curious

over 200KB

@pavelsavara
Merge branch 'main' into wasm_invariant_timezone
28bdd49 
# Conflicts:
#	src/mono/wasm/build/WasmApp.targets
@pavelsavara
whitespace
30fc179
@pavelsavara pavelsavara merged commit d388585 into dotnet:main Jul 1, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 31, 2023
@pavelsavara pavelsavara deleted the wasm_invariant_timezone branch September 2, 2024 15:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mdh1418 mdh1418 mdh1418 left review comments

@radical radical radical left review comments

@ilonatommy ilonatommy ilonatommy approved these changes

@maraf maraf maraf approved these changes

@kg kg kg approved these changes

@lewing lewing Awaiting requested review from lewing lewing is a code owner

@marek-safar marek-safar Awaiting requested review from marek-safar

Assignees

@pavelsavara pavelsavara

Labels
arch-wasm WebAssembly architecture area-Build-mono size-reduction Issues impacting final app size primary for size sensitive workloads
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

Add feature-switch for invariant TimeZone support
6 participants
@pavelsavara @maraf @kg @radical @mdh1418 @ilonatommy