Scrolling a GMail email conversation is really slow and chunky




6 years ago
6 years ago


(Reporter: Ben Turner (not reading bugmail, use the needinfo flag!), Assigned: roc)



Windows 7

Firefox Tracking Flags

(firefox8? affected, firefox9 fixed)


(Whiteboard: [qa!])


(3 attachments, 1 obsolete attachment)

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?
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

6 years ago
Well, can we dupe this against bug 579260 or is there something specific in this bug report?
Comment #1 is useful. I want to work on this.
Assignee: nobody → roc
Hmm, has this gotten better by itself?
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; 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 + ": " + ( - 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.
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.
Created attachment 568300 [details] [diff] [review]
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.
Attachment #568300 - Flags: review?(matspal)
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.
Created attachment 568304 [details] [diff] [review]
part 2: remove nsDisplayForcePaintOnScroll

No longer needed.
Attachment #568304 - Flags: review?(matspal)
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

6 years ago
OK, thanks for explaining.
Assignee: roc → nobody
Component: Graphics → Layout
QA Contact: thebes → layout
(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 on attachment 568300 [details] [diff] [review]
Part 1: fix text-overflow issues

Please update the DidProcessLines() doc comment in TextOverflow.h
Attachment #568300 - Flags: review?(matspal) → review+
Attachment #568304 - Flags: review?(matspal) → review+
Assignee: nobody → roc
I filed bug 696248 on the last issue in comment #5.
Created attachment 568553 [details] [diff] [review]
improved part 1

I realized we actually don't need DidProcessLines at all now.
Attachment #568553 - Flags: review?(matspal)
Created attachment 568556 [details] [diff] [review]
improved v2

Oops, that one didn't build.
Attachment #568553 - Attachment is obsolete: true
Attachment #568553 - Flags: review?(matspal)
Attachment #568556 - Flags: review?(matspal)
Comment on attachment 568556 [details] [diff] [review]
improved v2

Please update the class doc comment in TextOverflow.h
Attachment #568556 - Flags: review?(matspal) → review+
Whiteboard: [inbound]
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla10
roc, it appears you landed the original v2 patch instead of the improved one?
Looks correct to me.
Whoops, was equating v2 with part 2. Sorry for the spam.
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.
Attachment #568556 - Flags: approval-mozilla-aurora?
Duplicate of this bug: 695350
status-firefox9: --- → affected
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

6 years ago
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!
Attachment #568556 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox9: affected → fixed
Blocks: 312156
This regression affects Firefox 8 as well. Don't suppose we can still get this in there?
status-firefox8: --- → affected
tracking-firefox8: --- → ?
Adding [qa+] to make sure it gets verified where it lands. Looks like a high impact bug that can be verified easily.
Whiteboard: [qa+]
Seems to work well in Firefox 9.0b3. Marking verified.
Keywords: verified-beta
Whiteboard: [qa+] → [qa!]
You need to log in before you can comment on or make changes to this bug.