Skip to content

Conversation

@radekdoulik
Copy link
Member

@radekdoulik radekdoulik commented Oct 19, 2021

Context: #60349

When we switched to Emscripten 2.0.21, we stopped using deprecated
--llvm-opts 2 option. This caused unwanted performance degradation.
Switching to -O2 get us similar performance as with --llvm-opts.

Blazor wasm test app Time to first UI benchmark times:

branch/commit        link option    time    dotnet.wasm size  .br size
--------------------+------------+--------+------------------+---------
release/6.0 92ff02       -Oz       491ms    2,430,639          850,109
release/6.0 92ff02       -O2       457ms    2,474,518          853,765
release/6.0 92ff02       -O3       444ms    2,555,824          857,961

Customer Impact

Fixes a startup time regression for blazorwasm between .NET 5 an .NET 6 by a reverting compiler option change that was accidentally introduced as part of other build changes.

Testing

Manual and Automated

Regression

Yes, startup time regression.

Risk

Low, switch to compiler options we're using in main.

Context: dotnet#60349

When we switched to Emscripten 2.0.21, we stopped using deprecated
`--llvm-opts 2` option. This caused unwanted performance degradation.
Switching to `-O2` get us similar performance as with `--llvm-opts`.

Blazor wasm test app `Time to first UI` benchmark times:

    branch/commit        link option    time    dotnet.wasm size
    --------------------+------------+--------+-----------------
    release/6.0 92ff02       -Oz       491ms    2,430,639
    release/6.0 92ff02       -O2       457ms    2,474,518
    release/6.0 92ff02       -O3       444ms    2,555,824
@radekdoulik radekdoulik requested review from lewing and radical October 19, 2021 16:17
@lambdageek lambdageek changed the title [wasm] Change dotnet.wasm link optimization [release/6.0][wasm] Change dotnet.wasm link optimization Oct 19, 2021
@lambdageek lambdageek added the Servicing-consider Issue for next servicing release review label Oct 19, 2021
@lewing lewing removed the Servicing-consider Issue for next servicing release review label Oct 19, 2021
@lewing
Copy link
Member

lewing commented Oct 19, 2021

we're going to let this bake for a bit before we take it in servicing

@radical radical added the arch-wasm WebAssembly architecture label Oct 19, 2021
@ghost
Copy link

ghost commented Oct 19, 2021

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

Issue Details

Context: #60349

When we switched to Emscripten 2.0.21, we stopped using deprecated
--llvm-opts 2 option. This caused unwanted performance degradation.
Switching to -O2 get us similar performance as with --llvm-opts.

Blazor wasm test app Time to first UI benchmark times:

branch/commit        link option    time    dotnet.wasm size
--------------------+------------+--------+-----------------
release/6.0 92ff02       -Oz       491ms    2,430,639
release/6.0 92ff02       -O2       457ms    2,474,518
release/6.0 92ff02       -O3       444ms    2,555,824
Author: radekdoulik
Assignees: -
Labels:

arch-wasm, area-Build-mono

Milestone: -

@lewing lewing added this to the 6.0.1 milestone Oct 21, 2021
@lewing lewing added the Servicing-consider Issue for next servicing release review label Oct 22, 2021
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Nov 2, 2021
@Anipik
Copy link
Contributor

Anipik commented Nov 9, 2021

@marek-safar @radical can you review this one ?

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. but I would also wait for an approval from @lewing

@Anipik Anipik merged commit 08e0023 into dotnet:release/6.0 Nov 10, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2021
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 Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants