1578790 - Project Search Performance Regressions

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[David Walsh :davidwalsh]

Attachment: ProjectSearchRegression.png

Project search has regressed multiple times of the past few weeks. Image attached.

Push logs which appear to have created the regressions:

• https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fa0fc354ee09a6a027229b5c0c9f26efc808fd35&tochange=efe4a620841ca7032485544176fb051a642c01bc
  • https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=257776683ca2ea65d61b7e3cc1100e43a93cd6c3&tochange=1cca4f1af964c1214bc04bd9d5b34fee4023ed87

Comment 1

[Jason Laster [:jlast]]

Logan, it looks like the first regression might have been caused by Bug 1541631. I wonder if there was a regression in loading sources after the memoization work landed?

Comment 3

[Brian Hackett [Laid off!]]

With some preliminary investigation here the regression is definitely real. If I isolate this DAMP test I get 3860 ms (averaged across 5 runs) in a revision from mid-July, and 9002 ms currently (2.3x worse). That time corresponds with how long the search takes to complete, and watching the browser things are a lot slower now.

The issue here is that doing a text search of the project loads the text of every source, and in doing so also loads the breakpoint positions of all those sources. Breakpoint position loading has changed quite a bit in the last couple months, and new features like bug 1572280 add overhead that will be hard to remove. I think the problem here is that doing a text search of the project doesn't actually need to know where the breakpoint positions are, and fetching the breakpoint positions for all sources when doing a search is wasted work. If I take a current build and comment out the call to setBreakableLines() in loadSourceTextPromise(), the average time comes down to 3342 ms, 15% better than it was in mid-July. Eventually we might want to make getting breakpoint positions faster, but when doing it only for sources that the user is actually viewing the cost of doing this should be a lot less.

Comment 4

[Brian Hackett [Laid off!]]

Attachment: Bug 1578790 - Don't fetch breakpoint positions unless they are needed, r=jlast.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/057b59fdadad
Don't fetch breakpoint positions unless they are needed, r=jlast.

Comment 6

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/057b59fdadad