Closed Bug 1513095 Opened 2 years ago Closed 2 years ago
Loading breakpoints is slow on pages with many scripts
47 bytes, text/x-phabricator-request
|Details | Review|
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0 Steps to reproduce: The app I'm working on has about 250 scripts, which are not bundled at all in debug mode. Loading pages with any breakpoints set takes several seconds longer. I understand that I have a lot of scripts, but this still seems like too much time. Actual results: This is a fairly typical profile, with 5 breakpoints set: https://perf-html.io/public/fae3b88abc1c4e731cca8cd8180f8aaa39e92688/flame-graph/?globalTrackOrder=6-0-2-3-4-5-1-7-8-9-10-11&hiddenGlobalTracks=1-2-3-5-6-7-9-10&localTrackOrderByPid=16804-0~3108-0~&thread=11&transforms=ff-180~mf-170~mcn-combined-Bk_j_k_oz_s~mcn-combined-Bk_j_k_oN0&v=3 It shows several seconds spent in Debugger::findScripts(), and GetScriptLineExtent() in particular. Looking up the stack, it appears that _addSource() is called for every script, which calls _setBreakpointAtGeneratedLocation() for every breakpoint, which calls Debugger::findScripts(), which calls ScriptQuery::consider() for every script. This would mean that loading breakpoints for the entire page takes quadratic time with respect to the number of scripts. Expected results: I think that the slowdown could be avoided by filtering breakpoints down to those that match the script passed to _addSource(). This wouldn't stop multiple breakpoints in the same file all calling findScripts() for that file, but it would stop them calling it for every single script. This is more tentative, but I think the commonFilter check in consider() would benefit from being moved ahead of the line and possibly realm checks, as well. The URL and source comparisons seem like they should be pretty cheap and would have a very low false-positive rate.
Component: Untriaged → Debugger
Product: Firefox → DevTools
Only attempt to add breakpoints for the current source in Thread._addSource(), reducing the number of costly Debugger::findScripts() calls made when loading a page. In addition, speed up findScripts() itself by moving the cheaper URL/source checks in commonFilter() ahead of the line number checks.
I created a benchmark page that loads jquery 100 times: https://wartmanm.github.io/breakpoint-benchmark/ In each of these profiles, I set a breakpoint on line 10347 of jquery number 100 and reloaded the page. Here is the original: https://perfht.ml/2GqfSKQ 1600ms is spent in _addSource, 31% of which is in consider(). In this one, commonFilter() is moved to the top: https://perfht.ml/2GvR2Jl 1100ms is spent in _addSource, 1% of which is in consider(). I tested this without the _addSource change, because calling consider() 9900 fewer times would make the effect that much less noticeable. Finally, this one has commonFilter() moved, and filtering added to _addSource(): https://perfht.ml/2Gswxxl 700ms is spent in _addSource, almost all of which is spent in delazifying.
Thanks so much for the thorough investigation into all of this. This logic is definitely suboptimal at the moment, good find. If you're interested in contributing more, we have a Slack instance Devtools discussions: https://devtools-html-slack.herokuapp.com/
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/957cc2dce89c Improve performance when loading breakpoints on pages with many scripts r=loganfsmyth
Assignee: nobody → mattheww
You need to log in before you can comment on or make changes to this bug.