681867 - Scrolling a GMail email conversation is really slow and chunky

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I don't know if GMail has done something new (the floating top bar seems new) or if I'm just noticing this because I switched to Windows for my primary machine a few days ago, but scrolling a GMail email conversation is really bad. It's slow, jerks, lags way behind my mouse, and it seems to chew CPU. This new machine of mine has a pretty good graphics card and everything claims to be fully accelerated... Disabling the acceleration doesn't seem to make any difference.

Opening Chrome and scrolling is smooth and fast. What are we doing differently?

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Interesting... When the conversation has only a few emails in it scrolling isn't really bad.

When you have a lot of emails in one conversation GMail will group them all together and just show little lines for each message. Scrolling in this stage is worse than with only a few messages but not terrible.

Then when you click on the collection of lines they expand to show subject lines. Here scrolling is really awful.

As you click each subject line to expand the message scrolling seems to get better.

It's got to be something funny with the way they do those collapsed message subject line headers?

Comment 2

[:aceman]

Well, can we dupe this against bug 579260 or is there something specific in this bug report?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment #1 is useful. I want to work on this.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Hmm, has this gotten better by itself?

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I didn't get numbers before, but it feels a little better to me than it used to. But here are some real numbers:

I used this bookmarklet in Firefox and Chrome:

var startTime=Date.now(); var iters = 0; function doScroll() { var w = document.getElementById("canvas_frame").contentWindow; w.scrollTo(0, w.scrollY==200 ? 210 : 200); if (iters++ < 100) setTimeout(doScroll, 0); else alert(w.innerWidth + "x" + w.innerHeight + ": " + (Date.now() - startTime) + "ms"); } doScroll();

Content window for both browsers 1543x816

Scrolling dev.planning folder view:
Firefox trunk: 3050ms
Chrome: 1650ms

Scrolling "a five week release cycle?" thread view (nothing expanded):
Firefox trunk: 9750ms
Chrome: 2600ms

Scrolling "a five week release cycle?" thread view (subjects expanded):
Firefox trunk: 9200ms
Chrome: 6260ms

There's an additional problem where transitioning from "scrolling headers" to "fixed headers" as you scroll down is janky in Firefox. I'll look at that later.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I profiled "scrolling dev.planning folder view".

21% of the time under InvalidateFixedBackgroundFrames, called during every scrollTo. This builds a display list, computes visibility, then walks the list to see if there's anything visible that needs to be repainted whenever we scroll. I'm not sure what's triggering this in GMail ... it could be either text-overflow or background-attachment:fixed. We should be able to eliminate this somehow.

33% of the time is under FrameLayerBuilder::DrawThebesLayer, painting ThebesLayer contents. This seems excessive given we're only scrolling by 10 pixels. Maybe there's invalidation being caused by InvalidateFixedBackgroundFrames.

About 24% of the time is display list construction and layer building during painting.

Altogether 62% of the time is painting. About 5% compositing layers and other misc overhead.

The rest of the profile is frittered away on script execution, some event handlers, some random unassigned driver work, and various other overhead.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 1: fix text-overflow issues

SetHasFixedBackgroundFrame was being called for text-overflow elements (the label names in GMail's left-hand pane), which was forcing us to do those expensive InvalidateFixedBackgroundFrames and then forcing us to do unnecessary painting.

All we need to do for text-overflow is ensure that when the text-overflow's scrolling block is scrolled, the contents of the block are repainted, so let's do that explicitly using a state bit on the scrollframe.

This improves "scrolling dev.planning folder view" to 1550ms. I'm declaring victory on that one.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This improves "a five week release cycle? thread view (nothing expanded)" to 2580ms.

It improves "a five week release cycle? thread view (subjects expanded)" to about 1800ms.

I'll look into the "nothing expanded" case, we're already about as fast as Chrome there with the patch, but I'd like to know why it's slower than with subjects expanded ... it should be displaying less actual content.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: part 2: remove nsDisplayForcePaintOnScroll

No longer needed.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

In the "nothing expanded" case, we're spending a significant amount of time painting linear gradients, but I can't yet see what the gradient is being used for.

Comment 11

[:aceman]

OK, thanks for explaining.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #8)
> This improves "a five week release cycle? thread view (nothing expanded)" to
> 2580ms.

Retesting this morning, I'm getting more like 1800ms (quite variable, but always under 2000ms). So I'm declaring victory on this entire bug.

Comment 13

[Mats Palmgren (inactive)]

Comment on attachment 568300 [details] [diff] [review]
Part 1: fix text-overflow issues

Please update the DidProcessLines() doc comment in TextOverflow.h

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I filed bug 696248 on the last issue in comment #5.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: improved part 1

I realized we actually don't need DidProcessLines at all now.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: improved v2

Oops, that one didn't build.

Comment 17

[Mats Palmgren (inactive)]

Comment on attachment 568556 [details] [diff] [review]
improved v2

Please update the class doc comment in TextOverflow.h

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c27c06a3a50a
https://hg.mozilla.org/integration/mozilla-inbound/rev/af7752f0ae31

Comment 19

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/c27c06a3a50a
https://hg.mozilla.org/mozilla-central/rev/af7752f0ae31

Comment 20

[Ryan VanderMeulen [:RyanVM]]

roc, it appears you landed the original v2 patch instead of the improved one?

Comment 21

[Mats Palmgren (inactive)]

Looks correct to me.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Whoops, was equating v2 with part 2. Sorry for the spam.

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 568556 [details] [diff] [review]
improved v2

Review of attachment 568556 [details] [diff] [review]:
-----------------------------------------------------------------

Requesting Aurora approval. This fixes a performance regression due to text-overflow landing. It affects at least one very important site. The fix is very low risk ... any regression is likely to involve scrolling in an element with text-overflow, which is completely broken in most other browsers already.

Comment 25

[Mike Beltzner [:beltzner, not reading bugmail]]

This has been missed in triage a couple of times, and a dupe has been filed. It's a highly visible site to users, feels like big win and Roc's assertion of low risk in comment 23 gives me warm fuzzy feelings - any reason why triage has passed it by a few times?

Comment 26

[LegNeato]

Comment on attachment 568556 [details] [diff] [review]
improved v2

[triage comment]

Thanks for flagging. We slogged through all the beta stuff as it was higher priority but all those lists are back to zero so now we're on aurora :-)

Approved for aurora9, this looks like a great fix. Thanks!

Comment 27

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/90a4c98c1ae3

Comment 28

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This regression affects Firefox 8 as well. Don't suppose we can still get this in there?

Comment 29

[juan becerra [:juanb]]

Adding [qa+] to make sure it gets verified where it lands. Looks like a high impact bug that can be verified easily.

Comment 30

[u279076]

Seems to work well in Firefox 9.0b3. Marking verified.