Closed Bug 1590003 Opened 6 years ago Closed 6 years ago

DAMP Perf regression in inspector cold open test (+9%)

Categories

(DevTools :: Inspector: Compatibility, task, P2)

task

Tracking

(firefox71 fixed, firefox72 fixed)

RESOLVED FIXED
Firefox 72
Tracking Status
firefox71 --- fixed
firefox72 --- fixed

People

(Reporter: jdescottes, Assigned: daisuke)

References

Details

Attachments

(2 files)

Performance regression from Bug 1587690.

The changes that landed in Bug 1587690 introduced a 25% increase in the total number of characters loaded for the inspector, and a 9% slowdown on first open.

In particular the reducer should probably introduce lazy loading, and stop instantiating MDNCompatibility on load.

Hi Daisuke, do you want to take a look at this performance regression? It would be nice to uplift the fix, since we just missed the merge.
Thanks!

Flags: needinfo?(daisuke)

Thank you very much for letting me know, Julian!
Let me take a look!

Assignee: nobody → daisuke
Status: NEW → ASSIGNED
Flags: needinfo?(daisuke)
Pushed by dakatsuka.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8d16e7a647e8 Lazily load the compatibility lib and the dataset. r=jdescottes
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72

Comment on attachment 9103433 [details]
Bug 1590003: Lazily load the compatibility lib and the dataset. r?jdescottes

Beta/Release Uplift Approval Request

  • User impact if declined: DevTools' initial booting will take time a bit.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This patch changed only a way to load the data diagnosis. And this affects only DevTools.
  • String changes made/needed:
Attachment #9103433 - Flags: approval-mozilla-beta?
Attached image image.png

Thanks for the fix Daisuke!

Just wanted to mention that we can already see on the performance dashboard that the performance regression has been fixed on inspector cold open:
https://firefox-dev.tools/performance-dashboard/tools/inspector.html?days=14&filterstddev=true&ignoreFlags=true

Comment on attachment 9103433 [details]
Bug 1590003: Lazily load the compatibility lib and the dataset. r?jdescottes

Low risk perf fix in devtools confirmed by our perf automation, uplift approved for 71 beta 5, thanks!

Attachment #9103433 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: