Last Comment Bug 843853 - Newtab page slows down tab-open animation
: Newtab page slows down tab-open animation
Status: RESOLVED FIXED
[snappy:p1]
:
Product: Firefox
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal with 2 votes (vote)
: Firefox 24
Assigned To: Tim Taubert [:ttaubert]
:
Mentors:
Depends on: 853833 791670 850163
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-21 15:13 PST by Avi Halachmi (:avih)
Modified: 2013-08-05 12:06 PDT (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
delay rendering the newtab page until the tabopen animation has finished (7.33 KB, patch)
2013-04-09 10:58 PDT, Tim Taubert [:ttaubert]
no flags Details | Diff | Splinter Review
(WIP) convert the newtab page to XHTML and use the CSS3 flexbox model (28.52 KB, patch)
2013-04-09 11:03 PDT, Tim Taubert [:ttaubert]
no flags Details | Diff | Splinter Review

Description Avi Halachmi (:avih) 2013-02-21 15:13:19 PST
Tab animation smoothness telemetry suggests that the newtab page with thumbnails preview hurts animation smoothness.

Telemetry: http://goo.gl/LHjTZ (top: no newtab open without preview, bottom: with thumbnails preview).

There's also tab animation logs on bug 752839 comment #11 with and without thumbnails preview on slow/fast systems, with and without newtab preload.

The data suggests that on slow systems the FPS is really bad when preview is enabled, regardless of preload.
Comment 1 Johnathan Nightingale [:johnath] 2013-02-22 07:29:42 PST
Is this effect changed (for better or worse) by toggling browser.newtab.preload?
Comment 2 Johnathan Nightingale [:johnath] 2013-02-22 07:30:05 PST
(sorry, never mind me, I clearly can't read comment 0)
Comment 3 Tim Taubert [:ttaubert] 2013-02-28 14:37:56 PST
So I tested a couple of things with my brand-new (and slow) netbook. I rewrote the benchmark script a little to show the median of frames the tabopen animation consisted of. The more frames, the better.

1) Having about:blank as the new tab page yields 15 frames. That's obviously the goal.
2) The unchanged version of about:newtab (the one we currently ship) yields 3 frames (boo).
3) The thumbnail grid is inserted into the DOM dynamically. If I comment out this part so that the grid will never render we go up to 8 frames.
4) Keeping the change from (3) and commenting out the static XUL/HTML - the scrollbox and margins - we go up to 14 frames. Looks like there's a whole lot of overhead involved in layout/rendering the XUL flexbox elements here.

I wonder if the CSS3 flexbox model might be a better fit here? Is that optimized somehow? Another solution may be to defer rendering the page until the tabopen animation has finished? I'm not sure how this feels when opening a new tab. Also, preloading the new tab page does not seem an option then anymore.
Comment 4 Jeff Muizelaar [:jrmuizel] 2013-02-28 14:45:37 PST
(In reply to Tim Taubert [:ttaubert] from comment #3)
> I wonder if the CSS3 flexbox model might be a better fit here? Is that
> optimized somehow?

It seems like something is going wrong here. I'd suggest profiling the different cases to try to find out where the extra time is actually being spent and hopefully that will give a better indication of what to fix.
Comment 5 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2013-03-04 02:52:19 PST
Rewriting the tab page to use CSS3 flexbox is a good idea since that's the future anyway. I'd rather optimize that layout code than the XUL code, which is both obsolete and insane.
Comment 6 Tim Taubert [:ttaubert] 2013-03-11 11:26:12 PDT
Ok, something regressed this even more on my netbook. Using about:blank once yielded 15 frames for the tabopen animation to finish and I can still achieve this with a build from Feb 27th. With a current trunk build we're down to 7 frames :(
Comment 7 Tim Taubert [:ttaubert] 2013-03-11 11:45:54 PDT
Looks like this is caused by bug 844466.
Comment 8 Tim Taubert [:ttaubert] 2013-03-11 11:53:00 PDT
What bug 844466 does is start the tabopen animation synchronously instead off a timeout. I have no idea why that would regress on my slow netbook but I'm still seeing the 15 frames on my quad core machine, so there must be something that hurts us.
Comment 9 (dormant account) 2013-03-11 13:25:20 PDT
(In reply to Tim Taubert [:ttaubert] from comment #8)
> What bug 844466 does is start the tabopen animation synchronously instead
> off a timeout. I have no idea why that would regress on my slow netbook but
> I'm still seeing the 15 frames on my quad core machine, so there must be
> something that hurts us.

Avi we should setup an automatic test for this
Comment 10 Avi Halachmi (:avih) 2013-03-11 18:49:18 PDT
(In reply to Taras Glek (:taras) from comment #9)
> Avi we should setup an automatic test for this

Bug 848358.
Comment 11 Tim Taubert [:ttaubert] 2013-04-09 10:58:24 PDT
Created attachment 735295 [details] [diff] [review]
delay rendering the newtab page until the tabopen animation has finished

Here's a proposed fix that delays the rendering of about:newtab until after the tabopen animation has finished. The perf measurement values are close to about:blank with that on my machine but it doesn't look quite as nice when playing around with it. Although it's definitely an improvement over the current state of things.
Comment 12 Tim Taubert [:ttaubert] 2013-04-09 11:03:39 PDT
Created attachment 735299 [details] [diff] [review]
(WIP) convert the newtab page to XHTML and use the CSS3 flexbox model

This patch is work in progress. It converts newTab.xul to an XHTML page and uses the CSS3 flexbox model. Interestingly, we're a little faster in painting about:newtab but the tabclose animation is totally botched. I have no idea if we maybe miss a couple of optimizations when destroying HTML flexbox layouts?
Comment 13 Tim Taubert [:ttaubert] 2013-04-09 11:05:01 PDT
Comment on attachment 735295 [details] [diff] [review]
delay rendering the newtab page until the tabopen animation has finished

Try builds should be available soon:

https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ttaubert@mozilla.com-2875261de9fa/
Comment 14 Avi Halachmi (:avih) 2013-04-09 14:38:22 PDT
(In reply to Tim Taubert [:ttaubert] from comment #13)
> Comment on attachment 735295 [details] [diff] [review]
> delay rendering the newtab page until the tabopen animation has finished

Tim and I discussed this a bit on IRC. We thought the delay approach could be useful on slow systems, but we don't want to delay rendering on fast systems - where it's not required. Also, on slow systems, we might want to also disable newtab preload (waiting on bug 791670), because measurements show that it doesn't help on slow systems (but then again, it might also not matter at all when delaying the rendering).

If this is how we decide to go forward, then we'll need to instrument tab animation at runtime, and decide if we do or don't use delay - based on those results. Or we could use HW blacklisting, but I personally prefer run time instrumentation, especially since the infrastructure (and telemetry - bug 828097) is already there.

There's already bug 787698, which also assumes performance based decision, but on that bug, the suggestion is to disable tab animation altogether on slow system.

I personally prefer delayed preview rendering over disabled animation. Any comments on this approach? If we decide on runtime detection, then I'll file a bug for it.
Comment 15 Avi Halachmi (:avih) 2013-04-09 16:16:19 PDT
(In reply to Tim Taubert [:ttaubert] from comment #12)
> Created attachment 735299 [details] [diff] [review]
> (WIP) convert the newtab page to XHTML and use the CSS3 flexbox model
> ... It converts newTab.xul to an XHTML page and
> uses the CSS3 flexbox model. Interestingly, we're a little faster in
> painting about:newtab ...

How much faster?

Also, taras reminded me that we still don't really understand what's so slow with the current newtab rendering. It could be an existing bug someplace, or some other deficiency which could be improved/bypassed, but until we actually know what's slow there, we can't approach this efficiently.

Delaying the rendering could be a reasonable workaround if we decide it can't be improved, but finding the actual culprit would probably be a better first step.
Comment 16 :Gavin Sharp [email: gavin@gavinsharp.com] 2013-05-25 17:16:51 PDT
This depends on bug 791670 now, right?
Comment 17 Tim Taubert [:ttaubert] 2013-05-25 23:43:24 PDT
Yes, exactly.
Comment 18 Tim Taubert [:ttaubert] 2013-05-26 09:43:38 PDT
Adding bug 853833 as a dependency (which in turn depends on bug 719177). Hovering one of the thumbnails in about:newtab triggers a reflow (it shouldn't). This potentially also affects the tabopen animation if the cursor happens to be in the area of one of the thumbnails.
Comment 19 :Gavin Sharp [email: gavin@gavinsharp.com] 2013-06-02 17:52:38 PDT
Tim: do you have numbers for the impact of bug 791670 on this, now that all the reflow-causing things have been fixed?
Comment 20 Tim Taubert [:ttaubert] 2013-07-12 07:53:16 PDT
Measured with a build from today on my slow netbook:

about:blank  = Interval: 20.3ms, Paint:  8.1ms, Frames: 12
about:newtab = Interval: 21.3ms, Paint: 34.0ms, Frames: 11

We started with a frame count of 3 without newtab page preloading. We're almost on a par with about:blank now. We need a little more time to paint which is understandable though because there actually is something to be painted in about:newtab. Resolving as fixed.

Note You need to log in before you can comment on or make changes to this bug.