Talos sent an alert and here is the graph for a regression: http://graphs.mozilla.org/graph.html#tests=[[293,64,24]]&sel=1397423501000,1397596301000&displayrange=7&datatype=running This only seems to affect osx 10.8. I did some retriggers to verify this was the real push of concern: https://tbpl.mozilla.org/?tree=Fx-Team&fromchange=b8081c65fd70&tochange=12796fe3eb01&jobname=Rev5%20MacOSX%20Mountain%20Lion%2010.8%20fx-team%20talos%20svgr here is a link to datazilla: https://datazilla.mozilla.org/?start=1397073863&stop=1397678663&product=Firefox&repository=Mozilla-Inbound&os=mac&os_version=OS%20X%2010.8&test=tart&project=talos and these are the subtests which are regressed: simple-close-DPI1.all.TART icon-close-DPI2.all.TART icon-close-DPI1.all.TART

Dao, can you take a look at this patch and weigh in on what might be the problem?

(In reply to Joel Maher (:jmaher) from comment #0) > and these are the subtests which are regressed: > simple-close-DPI1.all.TART > icon-close-DPI2.all.TART > icon-close-DPI1.all.TART It's not clear to me what these subtests are doing...

(In reply to Dão Gottwald [:dao] from comment #2) > (In reply to Joel Maher (:jmaher) from comment #0) > > and these are the subtests which are regressed: > > simple-close-DPI1.all.TART > > icon-close-DPI2.all.TART > > icon-close-DPI1.all.TART > > It's not clear to me what these subtests are doing... I can answer that one. These subtests measure the average time it takes to paint the frames of the tab close animation when: 1) Closing a tab without a favicon at DPI1 (simple-close-DPI1) 2) Closing a tab with a favicon at DPI2 (icon-close-DPI2) 3) Closing a tab with a favicon at DPI1 (icon-close-DPI1). Because we're seeing a regression in the .all measures, but not the .half, it indicates that the expensive frames are in the first half of the animation. So something about the first frames of the close animation is slower to paint.

Ok, I have an idea what's going on.

So basically we were animating the label and close button for closing tabs even though they weren't rendered, because closing tabs have visibility:hidden. 10.8 has an enormous test queue. No idea when I will get results there. However, I see slightly improving tart numbers on all other platforms: https://tbpl.mozilla.org/?tree=Try&rev=b400f44887ed https://tbpl.mozilla.org/?tree=Try&rev=70e3a56e33dd http://compare-talos.mattn.ca/?oldRevs=b400f44887ed&newRev=70e3a56e33dd&server=graphs.mozilla.org&submit=true

:dao, thanks for taking a look at this. Every little fix makes a difference!

Comment on attachment 8408158 [details] [diff] [review] patch Review of attachment 8408158 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/browser.css @@ +132,5 @@ > > .tab-background:not([fadein]):not([pinned]) { > visibility: hidden; > + /* Closing tabs are completely hidden without a delay. */ > + transition: none; Why don't we just change the rule to the following: .tab-background[fadein] { transition: visibility 0ms 25ms; } .tab-close-button[fadein], .tab-label[fadein] { transition: opacity 70ms 230ms, visibility 0ms 230ms; }

According to the compare-talos, this approach will not gain us back the 4% that we've lost here...

Comment on attachment 8408885 [details] [diff] [review] patch v2 Review of attachment 8408885 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks!

:dao and :jaws, thanks for the patch and review. I know this isn't a 100% fix according to comment 9, but bringing us closer to fixing this is great to see.

Comment on attachment 8408885 [details] [diff] [review] patch v2 https://hg.mozilla.org/integration/fx-team/rev/f7320bbfb006

looking at graph server, I don't see a big win here: http://graphs.mozilla.org/graph.html#tests=[[293,64,24]]&sel=none&displayrange=7&datatype=running prior to the regression: ~11.0 after the patch landed: ~11.5 after this fix landed: ~11.4 It appears to be reduced slightly.

http://perf.snarkfest.net/compare-talos/index.html?oldRevs=f7320bbfb006&newRev=bde5e60ad7e6&submit=true I don't understand why this doesn't undo the regression. I'm starting to think that the tart test is measuring something wrong :/

Does "visibility: hidden" instead of display:none undo the regression?

(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #16) > Does "visibility: hidden" instead of display:none undo the regression? visibility:hidden on tab-stack should be redundant since the whole tab already has that while closing.

Bug 995626 has been backed out.