[WASM] Major size regression in dotnet.wasm

[eerhardt]

I'm seeing a 40KB regression on https://aka.ms/dotnetperfstatus:

Which all appears to be in the dotnet.wasm file:

It went from 782,610 bytes on aab9aa6 to 824,698 bytes on 8dd8a43.

There were 2 changes that went in during that range, both of them could be possibilities:

• Improve time zone display names on Unix #48931
• Don't bundle the LLVM AOT optimizer on wasm we use the emscripten version #50132

cc @mattjohnsonpint @lewing @CoffeeFlux

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

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

Issue Details

---

I'm seeing a 40KB regression on https://aka.ms/dotnetperfstatus:

Which all appears to be in the dotnet.wasm file:

It went from 782,610 bytes on aab9aa6 to 824,698 bytes on 8dd8a43.

There were 2 changes that went in during that range, both of them could be possibilities:

• Improve time zone display names on Unix #48931
• Don't bundle the LLVM AOT optimizer on wasm we use the emscripten version #50132

cc @mattjohnsonpint @lewing @CoffeeFlux

Author:	eerhardt
Assignees:	-
Labels:	arch-wasm
Milestone:	-

[lewing]

@eerhardt pretty sure it was #48931 my pr just removed completely unused files from a package only used by AOT tooling.

[mattjohnsonpint]

I can look into it. Curious what amount of increase is acceptable, and what should I be looking to optimize? Is it just total number of instructions? There were a few strings and string arrays added as well. We had some discussion on that PR about whether a simple string array is better or worse than searching through one big string - not sure if either would have a size difference for wasm.

[mattjohnsonpint]

Just to double check - we don't have ICU in the browser, right? I don't believe that is possible, but want to confirm.

Assuming not - I think it might be a good idea to have a TimeZoneInfo.Browser.cs split out from TimeZoneInfo.Unix.cs with only the parts needed for the browser. That should trim down the size considerably.

[lewing]

we have icu in the browser https://github.com/dotnet/icu/

[mattjohnsonpint]

Does it pass through to ICU used by the browser somehow? Or we are bundling it in the wasm ourself?

[lewing]

we bundle icu and a filtered set of data, the globalization available via javascript isn't sufficient to support the .NET apis. It could be possible in some cases to fall back to the browser apis for certain specific data if it is available

[mattjohnsonpint]

Ok. I'll see what else I can think of.

[mattjohnsonpint]

Actually, since it appears we're only using the IANA ID and abbreviation for the time zone display names, it still makes sense to me to conditionally build TimeZoneInfo to trim out the ICU bits. Unless we are including time zone display names in the ICU data, but I don't think we are since the current tests show otherwise.

[lewing]

That is definitely an option. It's also possible that we can just hide a the new logic behind a feature flag that the illinker understands and we can break the reference chain.

[EgorBo]

From my understanding, all ICU data files for Browser currently strip everything related to timezones (including names for them), e.g.: https://github.com/dotnet/icu/blob/maint/maint-67/icu-filters/icudt.json#L198-L199

so all of those ICU functions must be linked out

[mattjohnsonpint]

Working on this. Got a local wasm build going and doing some before/after size comparisons per @eerhardt's instructions. I'll report back shortly.

[eerhardt]

With the latest runs on https://aka.ms/dotnetperfstatus, I can confirm this is now fixed, and we are now slightly lower than the size before the regression came in:

Thanks again @mattjohnsonpint!

[mattjohnsonpint]

Glad to help!