Closed Bug 1578219 Opened 5 years ago Closed 5 years ago

DAMP Perf regression in debugger stepIn test (+40%)

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: jdescottes, Assigned: davidwalsh)

References

(Blocks 2 open bugs, Regression)

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

Performance regression on the debuger stepIn test, most likely related to Bug 1576679 (Enable Inline Preview for all channels).

Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=cd3f609f008112adf2fdba0180bfe1bb9802cfe8&tochange=8f134018ed03d592cd9dc27ef641565163472bbd

Alert summary:
== Change summary for alert #22876 (as of Wed, 28 Aug 2019 16:03:31 GMT) ==

Regressions:

34% damp custom.jsdebugger.stepIn.DAMP linux64-shippable-qr opt e10s stylo 386.42 -> 517.35
33% damp custom.jsdebugger.stepIn.DAMP windows10-64-shippable-qr opt e10s stylo 392.63 -> 520.27
30% damp custom.jsdebugger.stepIn.DAMP linux64-shippable opt e10s stylo 381.08 -> 494.02
9% damp custom.jsdebugger.stepOut.DAMP linux64-shippable opt e10s stylo 369.78 -> 402.31
3% damp custom.jsdebugger.stepOver.DAMP linux64-shippable-qr opt e10s stylo 259.25 -> 267.22

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=22876

Assignee: nobody → dwalsh
Blocks: dbg-71
Status: NEW → ASSIGNED
Whiteboard: [debugger-mvp]
Attachment #9090170 - Attachment description: WIP: Bug 1578219 - Fix stepping performance regression → Bug 1578219 - Improve performance regression for Inline Preview r=jlast
Priority: -- → P1
Depends on: 1580636
Depends on: 1579913
Depends on: 1580890
Attachment #9090170 - Attachment is obsolete: true

After much profiling and investigation I've isolated the problem to the following line: https://searchfox.org/mozilla-central/source/devtools/client/debugger/src/actions/pause/inlinePreview.js#66

await parser.getScopes(frame.location);

The parser worker is taking the contents of step-in-test.js, asking Babel to parse it, and return the scopes. Parsing and analyzing a 160kb file is causing the perf hit due to the about of time Babel requires. As I can't force Babel to go faster, we're in sort of a weird spot. We could find an alternative to Babel but that's obviously a much larger task.

A few ideas:

  1. Maybe we don't provide inline preview for files larger than a certain kb
  2. Greg mentioned that the profiler doesn't use workers because they can be slow; i.e. the time during postMessage calls is "wasted", whereas simply doing the work doesn't waste any time. We could try not using workers(?)

I don't think this is an issue with a simple quick fix.

It appears that Chrome uses acorn.js for parsing: https://github.com/acornjs/acorn

Attached image InlinePreviewDoesntBlockScopes.png (deleted) —

My console.logs make it obvious that the Scopes pane isn't waiting for inline preview to finish so generateInlinePreview, which calls the await parser.getScopes(frame.location) code, isn't holding up other parts of the debugger.

Attached file perf(debugger): Optimize stepping (obsolete) (deleted) —
  • Don't render if the props didn't change
  • Avoid accessing codeMirror getter inside loops
  • Don't compute the same information twice
  • Avoid memory allocations

I couldn't capture two identical enough profiles such that they can be easily compared.

Comment on attachment 9101662 [details]
perf(debugger): Optimize stepping

Revision D49477 was moved to bug 1550494. Setting attachment 9101662 [details] to obsolete.

Attachment #9101662 - Attachment is obsolete: true
Priority: P1 → P2
Whiteboard: [debugger-mvp]

Closing as we understand the impact from the test was due to an new parser.getScopes call and do not intend to change that.

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: