Closed DrewScoggins closed 3 years ago
Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT, @marek-safar See info in area-owners.md if you want to be subscribed.
Author: | DrewScoggins |
---|---|
Assignees: | - |
Labels: | `size-reduction`, `tenet-performance`, `tenet-performance-benchmarks` |
Milestone: | - |
@DrewScoggins - there are 146 commits in the Diff at the top of this issue. Any way we can narrow them down?
The 5% regression in System.Text.Json in the PizzaApp, it appears to maybe be caused by https://github.com/mono/linker/pull/2125.
Looking at a diff of System.Text.Json before and after I'm seeing:
Note, the PizzaApp does use System.Linq.Expressions. Looking at ILSpy, I see it being used by the AddressEditor:
cc @sbomer @vitek-karas - FYI
The even worse regression here is:
SOD - Minimum Blazor Template - Publish - pub/wwwroot/_framework/dotnet.wasm.br | 464.13 KB | 779.49 KB | 1.68 | 0.00 | True | -- | -- | -- | -- | -- | -- | --@SamMonoRT @lewing - did something break here that we lost all the re-linking work for the "Minimum" app (i.e. Invariant Globalization mode)?
@eerhardt right now all the components are being linked in unconditionally (hot-reload and the debugger) fix coming soon.
Tagging subscribers to this area: @thaystg See info in area-owners.md if you want to be subscribed.
Author: | DrewScoggins |
---|---|
Assignees: | - |
Labels: | `area-Debugger-mono`, `area-Diagnostics-coreclr`, `in pr`, `size-reduction`, `tenet-performance`, `tenet-performance-benchmarks` |
Milestone: | 6.0.0 |
Tagging subscribers to this area: See info in area-owners.md if you want to be subscribed.
Author: | DrewScoggins |
---|---|
Assignees: | - |
Labels: | `area-VM-meta-mono`, `in pr`, `size-reduction`, `tenet-performance`, `tenet-performance-benchmarks` |
Milestone: | 6.0.0 |
System.Text.Json in the PizzaApp
@eerhardt - Larry's PR change closed this issue which covered both STJ and dotnet.wasm regressions. Are we tracking this STJ regression outside of this issue.
The components are being correctly linked out and the runtime is being relinked but it looks like something is retaining icu in invariantmode now.
The 5% regression in System.Text.Json in the PizzaApp, it appears to maybe be caused by mono/linker#2125.
I believe that STJ regression is expected, given that the PizzaApp is using System.Linq.Expressions
. That was the tradeoff we made in mono/linker#2125 to make System.Linq.Expressions
safe to use in a trimmed app.
Found the runtime issue, fix incoming. You will like the result.
@radical - didn't see any decrease in size for dotnet.wasm in the runs with the change. Please take a look. @DrewScoggins is your main contact for dashboard questions. This is the size regression in the minimal wasm app.
Are these projects built with Debug config? Larry's PR (#55939) affects Release builds. In Release config, we use native relinking is enabled.
This is for a template blazorwasm project with InvariantGlobalization=true
:
Debug: 799362 bin/Debug/net6.0/publish/wwwroot/_framework/dotnet.wasm.br
Release: 351995 bin/Release/net6.0/publish/wwwroot/_framework/dotnet.wasm.br
@radical @DrewScoggins - Looking at https://github.com/dotnet/performance/blob/da8322773787ba87b2305899095fc587d5668929/src/scenarios/blazorminapp/pre.py seems like InvariantGlobalization is set to true correctly for the min app. What is interesting is searching for the workload name, I see https://github.com/dotnet/performance/search?q=microsoft-net-sdk-blazorwebassembly-aot - should that be updated to the new name wasm-tools ? (https://github.com/dotnet/runtime/pull/55413)
should that be updated to the new name wasm-tools
Yeah, that makes sense.
Drew's PR to udpate workload name should hopefully fix the dotnet.wasm.br regression
@radical @lewing - From @DrewScoggins :: We are at 1.11MB with the updated workload name. We don't have the breakdown of file size as of now. I assume the PowerBI won't be updated till the actual fix is merged.
[1:37 AM] Drew Scoggins [2021/07/21 22:35:22][INFO] Metric |Average |Min |Max[2021/07/21 22:35:22][INFO] ------------------------------------------------------------------------------------|------------------|------------------|------------------[2021/07/21 22:35:22][INFO] MinApp |6609478.000 bytes |6609478.000 bytes |6609478.000 bytes[2021/07/21 22:35:22][INFO] Total Uncompressed _framework |3194462.000 bytes |3194462.000 bytes |3194462.000 bytes[2021/07/21 22:35:22][INFO] Total Uncompressed _framework - Count |34.000 count |34.000 count |34.000 count[2021/07/21 22:35:22][INFO] Synthetic Wire Size - .br |1111486.000 bytes |1111486.000 bytes |1111486.000 bytes[2021/07/21 22:35:22][INFO] Synthetic Wire Size - .br - Count |34.000 count |34.000 count |34.000 count[2021/07/21 22:35:22][INFO] Synthetic Wire Size - .gz |1339530.000 bytes |1339530.000 bytes |1339530.000 bytes[2021/07/21 22:35:22][INFO] Synthetic Wire Size - .gz - Count |35.000 count |35.000 count |35.000 count[2021/07/21 22:35:22][INFO] MinApp - Count |121.000 count |121.000 count |121.000 count
[1:37 AM] Drew Scoggins This is the take away line
[1:38 AM] Drew Scoggins Synthetic Wire Size - .br |1111486.000 bytes |1111486.000 bytes |1111486.000 bytes
[1:38 AM] Drew Scoggins Somehow the package that we are building does not have some parts that are needed. So i rigged together a local run with the parts we needed to get that number
[1:39 AM] Drew Scoggins I will make a permanent fix when I get back from vacation.
We can close this issue, as the core regression is addressed.
Run Information
Regressions in SOD - Minimum Blazor Template - Publish
Historical Data in Reporting System
Repro
Run Information
Regressions in SOD - Pizza App - Publish
Historical Data in Reporting System
Repro
Run Information
Regressions in SOD - New Blazor Template - Publish
Historical Data in Reporting System
Repro