Closed Bug 1358709 Opened 3 years ago Closed 3 years ago

Avoid burning CPU before showing the slow script infobar for callstacks from minified scripts by calling PCToLineNumber with no good reason

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(3 files)

See the profile in bug 1330231 (https://clptr.io/2jC9bsJ).  We spend 15 seconds calling PCLineToNumber() in the content process before showing the slow script info bar.  I looked into where this data goes as we don't show it in the UI, and it is currently stored in HangMonitoredProcess::mHangData's lineno member, but nobody reads from it.  Basically the consumer is dead code.  So we can just not compute this in the first place!
This is dead code, and removing it is harmless.
Attachment #8860593 - Flags: review?(wmccloskey)
Attachment #8860593 - Flags: review?(wmccloskey) → review+
Attachment #8860594 - Flags: review?(wmccloskey) → review+
Comment on attachment 8860595 [details] [diff] [review]
Part 3: Avoid calling PCToLineNumber before showing the slow script info bar in the content process

Review of attachment 8860595 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsGlobalWindow.cpp
@@ +11592,5 @@
> +  // parent process, so we avoid computing it elsewhere.  This gives us most of
> +  // the wins we are interested in, since the source of the slowness here is
> +  // minified scripts which is more common in Web content that is loaded in the
> +  // content process.
> +  unsigned* linenumber = XRE_IsParentProcess() ? &lineno : nullptr;

I think linenop would be a better variable name. That's how we do it in the JS engine at least.
Attachment #8860595 - Flags: review?(wmccloskey) → review+
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/09fa7049871d
Part 1: Remove nsIHangReport::GetScriptLineNo; r=billm
https://hg.mozilla.org/integration/mozilla-inbound/rev/298e509e123b
Part 2: Remove the SlowScriptData.lineno IPDL field and the glue code; r=bill
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee3a2d1d6cbf
Part 3: Avoid calling PCToLineNumber before showing the slow script info bar in the content process; r=billm
https://hg.mozilla.org/mozilla-central/rev/09fa7049871d
https://hg.mozilla.org/mozilla-central/rev/298e509e123b
https://hg.mozilla.org/mozilla-central/rev/ee3a2d1d6cbf
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.