Closed Bug 1390824 Opened 7 years ago Closed 1 year ago

3.07% tabpaint (linux64) regression on push 616afd45b6e3d765bbc6451bec03d99931cb5b67 (Fri Jul 28 2017)


(Firefox :: Tabbed Browser, defect, P3)

53 Branch



Tracking Status
firefox57 --- fix-optional


(Reporter: jmaher, Unassigned)



(Keywords: perf, regression, talos-regression)

Talos has detected a Firefox performance regression from push:

As author of one of the patches included in that push, we need your help to address this regression.


  3%  tabpaint summary linux64 opt e10s     65.42 -> 67.42

You can find links to graphs and comparison views for each of the above tests at:

On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the Talos jobs in a pushlog format.

To learn more about the regressing test(s), please see:

For information on reproducing and debugging the regression, either on try or locally, see:

*** Please let us know your plans within 3 business days, or the offending patch(es) will be backed out! ***

Our wiki page outlines the common responses and expectations:
this regression occurs during a large build breakage, so all the bugs related to this landed during the build breakage
Bug 1356271 only touched test code.
No longer blocks: 1356271
Blocks: 1373320
No longer blocks: 1386631
Bug 1385137 only touched test code as well.
No longer blocks: 1385137
Bug 1367551 only touched http/2 push code, which would not be exercised by tabpaint
No longer blocks: 1367551
Bug 1379270 seems most relevant from skimming the bug summaries. 

Bug 1385201 only changed a pref's type+name which is checked once during startup so doesn't seem relevant.
No longer blocks: 1385201
No longer blocks: 1385391
No longer blocks: 1384680
No longer blocks: 1376306
after backing out a couple of bugs on try, it looks like bug 1379270 is the culprit.  :bwinton, I see you are the patch author here- this is sort of late- please don't drop everything for this as you landed this code 2.5 weeks ago- could you look at this and see if there is a fix related to your code in this revision:
No longer blocks: 1172574, 1255227, 1383338, 1384241, 1384695, 1384792
Flags: needinfo?(bwinton)
Component: Untriaged → Tabbed Browser
Redirecting to mconley, cause he's a few orders of magnitude more familiar with the code and possible perf regressions than I am…  ;)

(Also, I've noticed that if we have the urlbar selected and open a new tab, the new tab's urlbar won't be selected, so if we ended up backing out that patch and going with a different, more functional fix, I wouldn't particularly mind.)
Flags: needinfo?(bwinton) → needinfo?(mconley)
Mike, did you get the time to look over this?
Was able to reproduce the regression locally and get profiles.

Before patch (no regression):

After patch (with regression):

What I'm noticing here is that we're causing a style flush due to the later focus call. This is probably because some DOM stuff changed between the last natural flush and the call to focus(). This wasn't the case before because the focus() call was being called earlier, before updateCurrentBrowser had an opportunity to dirty the DOM.

We intentionally moved the focus() call to after updateCurrentBrowser, and updateCurrentBrowser is going to update the DOM - there's no real way around that.

If we decide to do bug 1355424, what we might want to do is defer focusing the URL bar until either:

1) The DOM hasn't been dirtied and so no style flush is required (the helpers in BrowserUtils can help there)


2) We process a key input

In the event of a key input, it'll be necessary to cause any lazy flushes to complete in order to send the key event to the right thing.

I don't think this should block 57 going out, but I think we should see if we can do this if / when we do bug 1355424.
Depends on: 1355424
Flags: needinfo?(mconley)
Priority: -- → P3
Severity: normal → S3

It's been nearly 6 years, realistically we're not going to specifically address this tabpaint regression.

Closed: 1 year ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.