Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Performance regression when loading sources #4692

Closed
juliandescottes opened this issue Nov 14, 2017 · 3 comments · Fixed by #4735
Closed

Performance regression when loading sources #4692

juliandescottes opened this issue Nov 14, 2017 · 3 comments · Fixed by #4735

Comments

@juliandescottes
Copy link
Member

juliandescottes commented Nov 14, 2017

Performance regression spotted with the 11-7 bundle release.

See screenshot of DAMP perf charts, reload regressed of 10%:
image

The test case from https://bugzilla.mozilla.org/show_bug.cgi?id=1366693 really highlights the issue. The test page is simply sequentially loading scripts as fast as possible and measuring the time it takes to load all scripts.

On my machine:

  • without devtools opened: < 1s
  • with debugger opened (version 10-25): 3s (profile)
  • with debugger opened (version 11-7): 10s (profile)

The test case is a zip at https://bugzilla.mozilla.org/attachment.cgi?id=8869957

Precise STRs:

  • serve the content of the zip e.g. on http://127.0.0.1:8080
  • open http://127.0.0.1:8080
  • (the page should display "Loaded in X seconds", X hopefully below 1)
  • open the debugger (be careful about opening only the debugger and not any other tools as they can impact the perf)
  • reload the page
  • page will now take several seconds to load
@claim claim bot added the not-available label Nov 14, 2017
@juliandescottes
Copy link
Member Author

The performance regression comes from 7ad9038 (which improves JSX syntax highlighting).

On the second profile you can see a lot of time is spent in codemirror (even though no source is actually opened in the UI). Looks like we rerender the Editor component more than we used to?

@nyrosmith
Copy link
Contributor

I can't verify by numbers but I think this should no longer be an issue because of #4653 and #4677

@juliandescottes
Copy link
Member Author

@nyrosmith just tested with the latest revision of master and the issue is still there, sorry :/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants