Last Comment Bug 752837 - Tab closing animation sluggish when closing tab since F13
: Tab closing animation sluggish when closing tab since F13
Status: RESOLVED WORKSFORME
[Snappy:P1]
: regression
Product: Firefox
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: 13 Branch
: All All
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
Depends on:
Blocks: 729878
  Show dependency treegraph
 
Reported: 2012-05-08 02:17 PDT by Virgil Dicu [:virgil] [QA]
Modified: 2012-09-05 13:48 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Telemetry screenshots for tab closing/opening (157.92 KB, application/zip)
2012-05-10 06:28 PDT, Virgil Dicu [:virgil] [QA]
no flags Details

Description Virgil Dicu [:virgil] [QA] 2012-05-08 02:17:13 PDT
Mozilla/5.0 (Windows NT 5.1; rv:13.0) Gecko/20100101 Firefox/13.0

http://vid.ly/5g9s7p

Screencast of tab closing with about:newtab and about:blank. When closing a tab, the animation is sluggish (screencast taken with Firefox 13 beta 2) - the tab (without text) remains displayed for a second after closing it via x button. 

1. Open a few tabs
2. Load a few pages in each tab. 
3. Close a middle focused tab.

Video taken on a Windows XP with Intel(R) 82945G Express Chipset Family (blocked graphics driver) using Firefox 13 beta2. 

Initially commented here:
https://bugzilla.mozilla.org/show_bug.cgi?id=724349#c14
Comment 1 (dormant account) 2012-05-09 15:14:50 PDT
What does telemetry report for how long the animation takes on XP? What cpu are you using?
Comment 2 (dormant account) 2012-05-09 15:16:36 PDT
Also, bug 753448 might help with this.
Comment 3 Tim Taubert [:ttaubert] 2012-05-09 15:23:36 PDT
(In reply to Taras Glek (:taras) from comment #2)
> Also, bug 753448 might help with this.

This shouldn't make a difference as it will only affect the opening of new tabs.
Comment 4 Marco Castelluccio [:marco] 2012-05-09 15:48:08 PDT
This is exactly what I saw on the Windows XP machine in bug 724349 comment 11. As I said, I don't know when the regression occured.
I'll see the telemetry reports on that machine as soon as I'll use it.
Comment 5 Virgil Dicu [:virgil] [QA] 2012-05-10 06:04:51 PDT
(In reply to Taras Glek (:taras) from comment #1)
> What does telemetry report for how long the animation takes on XP? What cpu
> are you using?

Intel(R) 82945G Express Chipset Family, Windows XP, Pentium 4, CPU 3.00 GHz

Used about:telemetry with F13beta 3 and Firefox 12.0-clean and a used profile (not very rich - about 100 entries in history) for both.

Results - average for Tab closing, respectively tab opening animation:
F13 clean profile  274,9    318.7
F13 used profile   529.5    591.7
F12 clean profile  241.1    271.9
F12 used profile   264      234.9

I've mostly used about the same pages for both (planet mozilla, addons.mozilla.org, cnn.com, bbc, nytimes or simple tabs)
Comment 6 Virgil Dicu [:virgil] [QA] 2012-05-10 06:28:26 PDT
Created attachment 622704 [details]
Telemetry screenshots for tab closing/opening
Comment 7 Marco Castelluccio [:marco] 2012-05-10 07:17:27 PDT
Comment on attachment 622704 [details]
Telemetry screenshots for tab closing/opening

Ahah, I thought it was an image :)
Comment 8 Virgil Dicu [:virgil] [QA] 2012-05-10 07:28:08 PDT
It is, archive with images. (forgot about the automatically detect part in the attachment)

Changing OS to all, as this occurs on different platforms.
Comment 9 (dormant account) 2012-05-10 18:23:01 PDT
Virgil, that's really good data. Do you have ability to narrow this down to a specific ff13 nightly?
Comment 10 (dormant account) 2012-05-10 18:29:55 PDT
(In reply to Taras Glek (:taras) from comment #9)
> Virgil, that's really good data. Do you have ability to narrow this down to
> a specific ff13 nightly?

I'm suspecting bug 598482. If this is a regression from that bug, then build from 2012-01-17 should exhibit this regression and 2012-01-15 should not.

You can also try this on a recently nightly with varying values of layout.frame_rate (10, 120, 240). Maybe reducing it to 10 results in faster animation completion.
Comment 11 Virgil Dicu [:virgil] [QA] 2012-05-11 05:53:25 PDT
Regression window:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2012-03-13&enddate=2012-03-14


I got the following results in about:telemetry for tab closing/opening (the same machine as in comment 5, the same profile):
Nightly from 13 March --> 254, 231
Nightly from 14 March --> 562, 586

Opened 10 tabs, closed 10 tabs for each build, same old profile (about 100 items in history).

I had approximately the same values (around 250) for every build i checked until the first new tab layout Nightly.
Comment 12 Virgil Dicu [:virgil] [QA] 2012-05-11 06:43:01 PDT
(In reply to Taras Glek (:taras) from comment #10)
> (In reply to Taras Glek (:taras) from comment #9)
> > Virgil, that's really good data. Do you have ability to narrow this down to
> > a specific ff13 nightly?
> You can also try this on a recently nightly with varying values of
> layout.frame_rate (10, 120, 240). Maybe reducing it to 10 results in faster
> animation completion.

I get approximately the same values on the current Nightly with different values for the pref (tried with default[-1], 10, 240): around 450 for tab closing and around 600 for tab opening.
Comment 13 Dão Gottwald [:dao] 2012-05-11 07:23:00 PDT
(In reply to Virgil Dicu [:virgil] [QA] from comment #11)
> Regression window:
> http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2012-03-
> 13&enddate=2012-03-14

You need to use the changeset ids of those builds in order to get an accurate range.
Comment 14 Virgil Dicu [:virgil] [QA] 2012-05-11 08:42:26 PDT
(In reply to Dão Gottwald [:dao] from comment #13)
> (In reply to Virgil Dicu [:virgil] [QA] from comment #11)
> > Regression window:
> > http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2012-03-
> > 13&enddate=2012-03-14
> 
> You need to use the changeset ids of those builds in order to get an
> accurate range.

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5ec9524de1af&tochange=ee4e0c98cb02
Comment 15 (dormant account) 2012-05-21 13:39:42 PDT
(In reply to Virgil Dicu [:virgil] [QA] from comment #14)
> (In reply to Dão Gottwald [:dao] from comment #13)
> > (In reply to Virgil Dicu [:virgil] [QA] from comment #11)
> > > Regression window:
> > > http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2012-03-
> > > 13&enddate=2012-03-14
> > 
> > You need to use the changeset ids of those builds in order to get an
> > accurate range.
> 
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=5ec9524de1af&tochange=ee4e0c98cb02

Dao, Tim,
I see a bunch of fx landings in that period and bunch of platform stuff, do we need to narrow this down further?
Can QA narrow this down further?
Comment 16 Jason Smith [:jsmith] 2012-05-21 13:42:27 PDT
Anthony - Can we get someone from QA to investigate this?
Comment 17 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-05-22 10:22:04 PDT
(In reply to Jason Smith [:jsmith] from comment #16)
> Anthony - Can we get someone from QA to investigate this?

QA is already on this bug (Virgil).
Comment 18 Virgil Dicu [:virgil] [QA] 2012-05-23 09:05:23 PDT
Taras, is there any possibility you can put up some try server builds from the above mentioned culprit window? 

I have no chance to build Firefox on the Windows XP machine where the issue is easily spotted (time and machine characteristics).
I was using hg bisect today on Ubuntu, but the tab closing differences on Linux are much smaller to easily catch an accurate range. I'll still finish off the investigation on Ubuntu and see where that leads, but it would be great to have a chance to double check on Windows.
Comment 19 (dormant account) 2012-05-23 10:01:33 PDT
(In reply to Virgil Dicu [:virgil] [QA] from comment #18)
> Taras, is there any possibility you can put up some try server builds from
> the above mentioned culprit window? 
> 
> I have no chance to build Firefox on the Windows XP machine where the issue
> is easily spotted (time and machine characteristics).
> I was using hg bisect today on Ubuntu, but the tab closing differences on
> Linux are much smaller to easily catch an accurate range. I'll still finish
> off the investigation on Ubuntu and see where that leads, but it would be
> great to have a chance to double check on Windows.

Don't waste your time on Linux. I down own this code, I'm just trying to keep stuff moving her. Tim/Dao, can one of you provide some try builds for Virgil?
Comment 20 Tim Taubert [:ttaubert] 2012-06-29 02:27:52 PDT
I tried to reproduce this on any of my machines (OS X, Win and Linux) but wasn't able to. I think 'remote bisecting' with Virgil's machine might be the only choice we have. I tried to 'hg bisect' and push it to try but unfortunately the builds are failing:

https://tbpl.mozilla.org/?tree=Try&rev=d5dc9e68abcd

Have there been any build system changes since Fx 13 that might prevent us from creating try builds for those revisions on Windows?
Comment 21 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-29 09:26:25 PDT
Probably! Can you package your local builds and send them to Virgil for testing?
Comment 22 Tim Taubert [:ttaubert] 2012-06-29 09:31:26 PDT
Do they "just work" on XP if I build them on my Win 7 machine?
Comment 23 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-29 09:37:12 PDT
I think so... Might depend on your toolchain and build options, but it's worth a shot.
Comment 24 Tim Taubert [:ttaubert] 2012-07-13 12:03:27 PDT
I finally got a build and it will hopefully run on XP!

http://dl.dropbox.com/u/25140570/firefox-13.0a1.en-US.win32-621b16542cd5.zip

Virgil, please report back if you still see the issue with the provided build. You might want to check if you're still seeing it with the 2012-03-14 build.
Comment 25 Tim Taubert [:ttaubert] 2012-07-13 13:17:38 PDT
If the issue is still present in the build from comment #24, try this one please:

http://dl.dropbox.com/u/25140570/firefox-13.0a1.en-US.win32-85ffbb752398.zip

If you don't see the issue, please try this build

http://dl.dropbox.com/u/25140570/firefox-13.0a1.en-US.win32-95a8eba120fe.zip
Comment 26 Virgil Dicu [:virgil] [QA] 2012-07-17 08:45:10 PDT
Had a scare for a moment - the previous machine where I could reproduce got replaced and moved -, but found another 3 Ghz machine where this is as easy to spot as the last one.

I can't see the issue with the build from comment 24 and with the second build from comment 25. The about:telemetry values for tab closing animation (and tab opening) are normal, around 280.

Could previously reproduce the issue on the latest Nightly and on the ee4e0c98cb02 build (values of around 500).

I think that would make the new changeset:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=95a8eba120fe&tochange=ee4e0c98cb02

Again, the new tab layout implementation bugs are also present in this range.
Comment 27 Tim Taubert [:ttaubert] 2012-07-17 09:30:16 PDT
Ok, so... how about this one?

http://dl.dropbox.com/u/25140570/firefox-13.0a1.en-US.win32-1ca7a94573f2.zip
Comment 28 Virgil Dicu [:virgil] [QA] 2012-07-18 01:01:17 PDT
(In reply to Tim Taubert [:ttaubert] from comment #27)
> Ok, so... how about this one?
> 
> http://dl.dropbox.com/u/25140570/firefox-13.0a1.en-US.win32-1ca7a94573f2.zip

Values of 260 for tab closing - another good build.
Comment 29 Tim Taubert [:ttaubert] 2012-07-18 03:00:04 PDT
Alright... this one?

http://dl.dropbox.com/u/25140570/firefox-13.0a1.en-US.win32-a0fa0eb17298.zip
Comment 30 Virgil Dicu [:virgil] [QA] 2012-07-18 04:12:07 PDT
(In reply to Tim Taubert [:ttaubert] from comment #29)
> Alright... this one?
> 
> http://dl.dropbox.com/u/25140570/firefox-13.0a1.en-US.win32-a0fa0eb17298.zip

Good one - normal values - around 270.

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a0fa0eb17298&tochange=ee4e0c98cb02
Comment 31 Tim Taubert [:ttaubert] 2012-07-18 04:39:04 PDT
So looks like this is indeed caused by the new newtab layout:

http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=ee4e0c98cb02

I have no idea why the newtab page should affect the tab closing animation. Sure, there's more stuff to destroy but if you're navigating away this shouldn't be able to cause it anymore, right?
Comment 32 Virgil Dicu [:virgil] [QA] 2012-07-18 05:06:42 PDT
The regressions for tab opening and tab closing seem to go head to head. Might be worth trying to see if bug 753448 would solve this one too. 

Is there anything else we can do here to find the root cause? Bisecting through that range, building without some lines of code...
Comment 33 Tim Taubert [:ttaubert] 2012-07-18 05:13:05 PDT
(In reply to Virgil Dicu [:virgil] [QA] from comment #32)
> The regressions for tab opening and tab closing seem to go head to head.
> Might be worth trying to see if bug 753448 would solve this one too. 

Well, this should certainly speed up tab opening but not closing.

> Is there anything else we can do here to find the root cause? Bisecting
> through that range, building without some lines of code...

Hm, not sure that's helpful. I'd say it's caused by the new layout as a whole...

BTW, did you try to reproduce this with one of the current nightlies? It would be interesting to see if bug 754495 had some effect here?
Comment 34 Virgil Dicu [:virgil] [QA] 2012-07-18 05:24:11 PDT
(In reply to Tim Taubert [:ttaubert] from comment #33)
> (In reply to Virgil Dicu [:virgil] [QA] from comment #32)
> > The regressions for tab opening and tab closing seem to go head to head.
> > Might be worth trying to see if bug 753448 would solve this one too. 
> 
> Well, this should certainly speed up tab opening but not closing.

Another thing to note is that tab animation values are high only with about:newtab. Hiding the grid solves the closing animation problem.
 
> > Is there anything else we can do here to find the root cause? Bisecting
> > through that range, building without some lines of code...
> 
> Hm, not sure that's helpful. I'd say it's caused by the new layout as a
> whole...
> 
> BTW, did you try to reproduce this with one of the current nightlies? It
> would be interesting to see if bug 754495 had some effect here?

Yes, still very high values (500-600 for both measurements) while using Firefox 17 latest.
Comment 35 :Felipe Gomes (needinfo me!) 2012-07-19 12:38:10 PDT
(In reply to Tim Taubert [:ttaubert] from comment #31)
> I have no idea why the newtab page should affect the tab closing animation.
> Sure, there's more stuff to destroy but if you're navigating away this
> shouldn't be able to cause it anymore, right?

I wonder if it is related to the unload handler in the newtab page
Comment 36 :Felipe Gomes (needinfo me!) 2012-07-19 21:44:08 PDT
I sent to tryserver a build that just removes the unload listener to see if it is what affects the close animation. Virgil, could you try it when the build finishes and compare to a current nightly?

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/felipc@gmail.com-d09b47270c66

https://tbpl.mozilla.org/?tree=Try&rev=d09b47270c66
Comment 37 Virgil Dicu [:virgil] [QA] 2012-07-20 07:17:58 PDT
(In reply to :Felipe Gomes [offline 20-24, slow resp. 25-28] from comment #36)
> I sent to tryserver a build that just removes the unload listener to see if
> it is what affects the close animation. Virgil, could you try it when the
> build finishes and compare to a current nightly?
> 
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/felipc@gmail.com-
> d09b47270c66
> 
> https://tbpl.mozilla.org/?tree=Try&rev=d09b47270c66

Still 500+values on the try build , so that didn't do the trick unfortunately.
Comment 38 Virgil Dicu [:virgil] [QA] 2012-09-04 08:32:07 PDT
For a time now, I've noticed that both the opening and closing tab animation looks smoother. As the difference between telemetry values isn't big enough on regular PCs, investigated with the old machine where I could reproduce the issue reported here.

With current Nightly, both animation values take under 300 ms while following the same old  steps. That means a drop of over 200 ms for the tab animations.

This was fixed between the 1st and the 2nd of August.
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=582d4c67b3a7&tochange=588424024294
Comment 39 Virgil Dicu [:virgil] [QA] 2012-09-04 08:35:07 PDT
If anybody has a clue what fixed this in that range, it would be a great piece of information. 

If anybody watching the telemetry data can confirm this, it would also be great. I won't close this until then, but this is definitely fixed here.
Comment 40 (dormant account) 2012-09-05 12:10:56 PDT
Tim did any of your changes land in that timeframe?
Comment 41 (dormant account) 2012-09-05 13:48:12 PDT
(In reply to Virgil Dicu [:virgil] [QA] from comment #39)
> If anybody has a clue what fixed this in that range, it would be a great
> piece of information. 
> 
> If anybody watching the telemetry data can confirm this, it would also be
> great. I won't close this until then, but this is definitely fixed here.

I checked, and it looks like Firefox 18 is much better in terms of the longtail on tab animations on telemetry than the last couple of releases. It's still not as good at keeping animations ~100ms as older releases were.

Lets close this bug and focus on tab animation speed elsewhere.

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