783748 - Only decode images for about 50ms during tab switch

Status: RESOLVED WORKSFORME

(Core :: Graphics: ImageLib, defect)

Description

[Alice0775 White]

Attachment: sample, please open Folder in Bookmarks Menu

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/a79132ac2f05
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120817030555

Step To Reproduce:

1. Open several tabs and reload all tabs if necessary
2. Hold Ctrl+Tab
3. 

Actual Results:
   Tab switching is very slow

Expected Results:
   Tab switching should be smooth.

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/3b46b03dff5c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120815053101
Bad:
http://hg.mozilla.org/mozilla-central/rev/d67c02074ced
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120815065301
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3b46b03dff5c&tochange=d67c02074ced

Triggered by: Bug 685516

Comment 1

[Alice0775 White]

I can reproduce the delay and confirmed the regression range with local build.

Last Good: a33215aa549c
First Bad: d3829aea9cd1

Comment 2

[Alice0775 White]

And 
@TheVisitor provided better STR is in http://forums.mozillazine.org/viewtopic.php?p=12208739#p12208739

STR:
0. Open http://forums.mozillazine.org/viewtopic.php?f=23&t=2516363
1. Open http://www.nbcnews.com
2. Open http://www.cnn.com
3. have forum open in a tab
4. switching from the mz forum to either tab 1 or 2 will show about 1 sec delay in switching.
5. if you then quickly switch between the forum and CNN for example - switching is pretty fast.
6. switch to forum tab - wait 10-15 seconds then switch to either CNN or NBCnews - note it lags

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

In retrospect this isn't that surprising.  We now try to decode every image on a page for 5 ms when switching tabs.  On a page with lots of large images this could be a pretty significant regression, because we used to not decode any of them immediately.  I've noticed this on flickr pages with lots of images.

What we could do is bring back the old "don't sync decode if over a certain size" metric, but keep the "only sync decode for N ms" part for smaller images.  How does that sound, Joe?

/me wishes we had OMTD.

Comment 4

[Joe Drew (not getting mail)]

Mentioned this to Kyle on irc, but I'll repeat it here: We should have a global limit on the amount of decoding done synchronously on tab switch, with the 5ms adding up to 250 or 500 ms (or whatever, determined by the UX and specified with a pref) and then just queuing up totally-asynchronous decodes after that is reached.

Comment 5

[Maxime]

Attachment: Slow image display

Is this somehow related?
The test displays 160 small thumbnails.
Rebuilding the image list (innerHTML=) takes almost one second in Nightly, compared to ~30ms in Aurora.

Comment 6

[(dormant account)]

I think comment https://blog.mozilla.org/tglek/2012/08/16/snappy-36/comment-page-1/#comment-35935 is referring to this problem.
"
Something unrelated to this news: for some days I get real bad hangs, sometimes lasting seconds, mainly while changing tabs. I also tried with a new profile. The gecko profiler always shows something like this:
http://people.mozilla.com/~bgirard/cleopatra/?report=542efa04d60977a067a3623a00765d837cb952ba
"

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Yes, that's correct.

Comment 8

[Mardeg]

What should happen to bug 595927 with this bug open?

Comment 9

[Marco Castelluccio [:marco]]

(In reply to Mardeg from comment #8)
> What should happen to bug 595927 with this bug open?

I think they're different bugs. This is a regression from bug 685516, the other one is an older regression, without a regression window.

Comment 10

[Marco Castelluccio [:marco]]

Is disabling animations until we fix them a viable option?

Comment 11

[Marco Castelluccio [:marco]]

(Currently with animations, if you enable nglayout.debug.paint_flashing, you can see a lot of repaints of the entire chrome when you open or close a tab, without animations there's only a single repaint)

Comment 12

[Dão Gottwald [:dao]]

Please stop spamming this bug with unrelated stuff.

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

This turned out to be caused by bug 784756, which is now fixed, not the scenario in comment 3.  Comment 3 is still a theoretical possibility, but I'm not sure that it's common enough for us to invest the effort needed to implement comment 4's solution.  I would just WFM this, but I'll give the Snappy folks another chance to weigh in.

Comment 14

[(dormant account)]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #13)
> This turned out to be caused by bug 784756, which is now fixed, not the
> scenario in comment 3.  Comment 3 is still a theoretical possibility, but
> I'm not sure that it's common enough for us to invest the effort needed to
> implement comment 4's solution.  I would just WFM this, but I'll give the
> Snappy folks another chance to weigh in.

We can't decide on importance of doing comment 4 without someone diving in for analysis of how much overhead is taken up by image decoding on tab switch. Since bug 753127 is not done, it's not clear whether we regressed our tabswitch speed in bug 685516.

Comment 15

[Joe Drew (not getting mail)]

Madhava tells me that, for an interaction to feel instantaneous, the right order of magnitude of time is 100-200ms (depending on what's changing), so I do feel that a 50ms decoding limit is appropriate.

Comment 16

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Since we don't have any data to indicate that this is a priority, I doubt we will implement this before off-main-thread-decoding.  I'm going to close this as WONTFIX, but feel free to reopen if you disagree.

Comment 17

[Jeff Muizelaar [:jrmuizel]]

I have some data (albeit preliminary on nytimes.com) that suggests that tab switch performance can be dominated by image decoding.

Comment 18

[Lukas Blakk [:lsblakk] use ?needinfo]

Please re-nominate if there's enough data to show significant user pain caused by this.

Comment 19

[u279076]

Alice, does this still reproduce?

Comment 20

[Alice0775 White]

With e10s, I can not test this due to Bug 1185667.
Without e10s, It seems to have been improved.