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

[browser][perf] Blazor startup regression 2023-04-21 #89631

Closed
maraf opened this issue Jul 28, 2023 · 8 comments
Closed

[browser][perf] Blazor startup regression 2023-04-21 #89631

maraf opened this issue Jul 28, 2023 · 8 comments
Assignees
Labels
Milestone

Comments

@maraf
Copy link
Member

maraf commented Jul 28, 2023

cc @radekdoulik

@maraf maraf added arch-wasm WebAssembly architecture tenet-performance Performance related issue labels Jul 28, 2023
@maraf maraf added this to the 8.0.0 milestone Jul 28, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 28, 2023
@ghost
Copy link

ghost commented Jul 28, 2023

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

Issue Details

cc @radekdoulik

Author: maraf
Assignees: -
Labels:

arch-wasm, tenet-performance

Milestone: 8.0.0

@maraf
Copy link
Member Author

maraf commented Jul 28, 2023

I wasn't able to reproduce the regression locally. Local runs for commits from mentioned PRs had variations

@maraf maraf changed the title [browser][pref] Blazor startup regression [browser][perf] Blazor startup regression Jul 28, 2023
@pavelsavara pavelsavara added area-System.Runtime.InteropServices.JavaScript and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 28, 2023
@lewing
Copy link
Member

lewing commented Jul 31, 2023

Were there any System.Text.Json commits nearby in the history but outside the range?

@pavelsavara
Copy link
Member

Right now I think that the regression could be caused by postponing the start of the DLL downloads until dotnet.runtime.js is loaded , which happened in dotnet/aspnetcore#47752

I hope I mostly fixed that in my #89857

@pavelsavara
Copy link
Member

pavelsavara commented Aug 11, 2023

In Net7 Blazor didn't have any download throttling at all.
After unification of the loader, we throttle downloads to max 16 in parallel.
We do it in order to make the startup more stable in low resource environments, such as codespaces and helix.
Maybe also mobile phones benefit, but I don't have data for that.

maxParallelDownloads: 16,

But maybe it's impacting the perf benchmark because it's running on beefy machine with local web server.

We could

  • change the default to something more aggressive, like 32 parallel or even unlimited (and just set it low for our unit tests)
  • admit that measuring startup time on fast machine with local server is not realistic. Leave it as is, or even add server side network throttling.
  • disable our throttling just for Blazor

Note, I'm not 100% sure that this is the regression we see here.

@maraf
Copy link
Member Author

maraf commented Aug 24, 2023

@maraf maraf changed the title [browser][perf] Blazor startup regression [browser][perf] Blazor startup regression 2023-04-21 Aug 24, 2023
@maraf
Copy link
Member Author

maraf commented Aug 29, 2023

Changes from #90388 seem promising
image

@maraf
Copy link
Member Author

maraf commented Oct 31, 2023

We have gained something back with mentioned PR matching values from March

@maraf maraf closed this as completed Oct 31, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Nov 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants