1059558 - New tab page takes over 1 second to load

Status: RESOLVED WONTFIX

(Firefox :: New Tab Page, defect)

Description

[Benoit Girard (:BenWa)]

Profile:
http://people.mozilla.org/~bgirard/cleopatra/#report=7aec405a5e2ab296eb60688818bb57f7cd35243d

We're spending over 15ms *per display item* on a desktop machine.

STR:
1) Open the new tab page. Some of the time it will take over 1 second to load

We need to avoid hitting the CreateSamplingRestricedDrawable. I believe it has to do with the way the images are styled to avoid repeating.

[Tracking Requested - why for this release]:
Performance is unacceptable on a high end laptop.

Comment 1

[Avi Halachmi (:avih)]

15ms per item (tile/thumb?), while certainly suboptimal, doesn't account for 1000ms for 15 tiles at most.

I believe vladan also experienced a similar issue on his T100 small laptop few months ago.

Vladan, do you recall what it was?

Comment 2

[Ed Lee :Mardak]

Is this with a new profile (so seeing directory tiles/logos) or profile with history/thumbnails?

Comment 3

[Benoit Girard (:BenWa)]

A profile with history

Comment 4

[Benoit Girard (:BenWa)]

(In reply to Avi Halachmi (:avih) from comment #1)
> 15ms per item (tile/thumb?), while certainly suboptimal, doesn't account for
> 1000ms for 15 tiles at most.

I guess the profile I collected shows 600ms. There's much more than just painting but the profile contains the exact breakdown.

Comment 5

[Vladan Djeric (:vladan)]

I'll grab a profile of opening the newtab page with 9 history tiles on my T100 tomorrow

Comment 6

[Vladan Djeric (:vladan)]

I was able to reproduce the newtab animation suck on an Asus T100 netbook running today's Nightly on Win 8.1. Note that my profile had 9 directory tiles instead of history tiles.

http://people.mozilla.org/~bgirard/cleopatra/#report=7a194b017c8efe0a4d7acdbe760ece7eb66e3300&select=1690,2980

- For the first frame, it takes 44 ms to build a display list and 330ms to rasterize(!). This is probably related to CreateSamplingRestricedDrawable mentioned in Benoit's comment 1. He thinks it might have to do with some of the CSS applied to the tiles

- It takes 600ms to run browserOpenTab(), including 65ms spent in DT_updateMenuCheckbox() @ gDevTools.jsm:1006. I'm guessing it's related to the following:

http://mxr.mozilla.org/mozilla-central/source/browser/devtools/framework/gDevTools.jsm#689

Rob, can you confirm?

Comment 7

[Vladan Djeric (:vladan)]

Ed, do you want me to lend my T100 to one of your people in Toronto so they can try measuring the benefit of removing individual CSS properties on the newtab animation? Avi and I can help get them started

Comment 8

[Avi Halachmi (:avih)]

Does the browser hang for 1 second? or maybe the tab open smoothly, the browser is responsive, but the newtab's page is white for 1s and then the content appears? or does it appear gradually over 1 second (i.e. you see the tiles render one after the other over 1s)? or some other behavior?

IOW, what does "take 1s to load" mean in terms of user experience?

Comment 9

[Avi Halachmi (:avih)]

(In reply to Benoit Girard (:BenWa) from comment #0)
> ...
> We need to avoid hitting the CreateSamplingRestricedDrawable. I believe it
> has to do with the way the images are styled to avoid repeating.
> ...

So, I just talked to roc on IRC, and he says this code is triggered when we're clipping a scaled/resized image (I'm guessing that's the round corners for the tiles). Clipping a non-scales image should not hit this code.

Also:
<roc> we should not be using [hitting - avih] it for background-size:contain/cover
<roc> if that's causing it, it's a bug

This combines with another issue I thought of today - we're resizing tons of pixels for the newtab page, even regardless of any clipping.

According to ttaubert, each capture (for the history thumbnails) is originally 1/3 x 1/3 screen size (full screen worth of pixels for 9 thumbnail captures, more than screen and a half for 15 tiles). ON a full HD screen or retina displays that's many many pixels to resize.

On top of that, each resize happens twice (low/high quality), and on top of that, it's not GPU assisted (bug 1027791 comment 3).

I think the newtab page could gain tons of time if it'll use thumbnails at the exact size. In the past (e.g. on Firefox 32), the thumbs were resized according to the window size so there wasn't "exact size" known in advance, but right now it doesn't happen anymore, and they're always displayed at a fixed size, and instead their number/layout change according to the window size.

We should probably file a bug for that.

Comment 10

[Benoit Girard (:BenWa)]

(In reply to Avi Halachmi (:avih) from comment #8)
> Does the browser hang for 1 second? or maybe the tab open smoothly, the
> browser is responsive, but the newtab's page is white for 1s and then the
> content appears? or does it appear gradually over 1 second (i.e. you see the
> tiles render one after the other over 1s)? or some other behavior?
> 
> IOW, what does "take 1s to load" mean in terms of user experience?

When I hit the '+' icon. The browser locks up shortly and then the animation is skipped and the new tab page appears.

Comment 11

[Ed Lee :Mardak]

dcrobot, it seems that there's some pretty bad performance issues with rounded corners for history tiles. It causes the tab opening animations to be choppy or nonexistant as the browser tries to hide the edges of the tiles.

How important are the rounded corners for history tiles?

BenWa, can you confirm that this performance impact does not happen for a new profile (which would be showing directory tiles that are correctly sized, so it should not be hitting the clipping-resized codepath?)

Comment 12

[Benoit Girard (:BenWa)]

It's fine with a new profile. Feels responsive and I only see the directory tiles.

Comment 13

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Could we actually cache website thumbnails at the correct size, now that newtab tiles don't scale anymore?

Comment 14

[Ed Lee :Mardak]

(In reply to Philipp Sackl [:phlsa] from comment #13)
> Could we actually cache website thumbnails at the correct size, now that
> newtab tiles don't scale anymore?
That's an option. Bug 1059559 and elsewhere have made the suggestion to capture at the appropriate size. I'm not sure what else is using these thumbnails (but a quick guess is tab drag/drop and tab groups?)

Potentially we would want a way to specify the size in the uri to support these different uses?

moz-page-thumb://thumbnail?url=..&height=..&width=..

Or if new tab is the highest resolution used and it's okay to resize for the other uses, we could just capture at 290x180.

Comment 15

[Aaron]

If I understand this thread correctly, the issue isn't the rounded corners (which are pretty important) - but rather re-sizing the images.

I originally specified that all original image files (provided as a PNG) should be 2x larger than necessary for a 72 dpi resolution screen (i.e. desktop). My intent was to use those originals images at 100% for hi-res mobile displays. This way we can avoid having to produce and store 2 sets of images for the same tile.

If this assumption is incorrect, then I think we should ask that all tile images going forward are sized exactly as they should be rendered. Would this fix this issue? And if so, how would this affect any inherent mobile strategy?

Comment 16

[Benoit Girard (:BenWa)]

We can resize images fast enough. We just need to do so without hitting CreateSamplingRestricedDrawable.

Comment 17

[Aaron]

So is there another way to render the corners as specified without choking the Firefox?

Comment 18

[Benoit Girard (:BenWa)]

Reduce the test case, find what is causing CreateSamplingRestricedDrawable. AFAIK we don't know that yet. Once you find that we can discuss how to get the desired effect without using it.

Comment 19

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Since tiles are already on Beta, we need an emergency plan for this that we can theoretically uplift.

(In reply to Ed Lee :Mardak from comment #14)
> Or if new tab is the highest resolution used and it's okay to resize for the
> other uses, we could just capture at 290x180.

We should probably capture in double resolution for HiDPI displays.
From the top of my head, I can't think of anything that uses larger thumbnails (except for some configurations of panorama, but that's quite the edge case).

(In reply to Benoit Girard (:BenWa) from comment #18)
> Reduce the test case, find what is causing CreateSamplingRestricedDrawable.
> AFAIK we don't know that yet. Once you find that we can discuss how to get
> the desired effect without using it.

Is anyone assigned to do that yet?

Comment 20

[Avi Halachmi (:avih)]

FWIW, personally I don't think I ever noticed newtab open which takes more than ~250 ms or so (on few different windows systems and non-retina OS X).

I think vladan saw a similar symptom with his low-end T100 Windows 8 laptop, though I don't think it was traced to CreateSamplingRestricedDrawable, so it's not necessarily this bug.

- Is this issue (~1000ms newtab load time) 100% reproducible?
- If yes, what's the exact STR?
- Is it limited to OS X with retina display?

(In reply to Philipp Sackl [:phlsa] from comment #19)
> ...
> We should probably capture in double resolution for HiDPI displays.
> From the top of my head, I can't think of anything that uses larger
> thumbnails (except for some configurations of panorama, but that's quite the
> edge case).

Assuming the main reason for this bug always include resizing (possibly combined with clipping) I think the goal is to avoid resizing at the newtab page. E.g. to have a pre-made image at the resolution at which we display it later at the newtab page. It doesn't necessarily mean we should capture it at the target resolution, and yes, the target resolution could change between normal/retina displays.

According to ttaubert, beyond the ones you mentioned, another use of the captures which needs relatively high resolution is for ctrl-TAB (currently off by default behind the pref browser.ctrlTab.previews).

Comment 21

[Benoit Girard (:BenWa)]

(In reply to Avi Halachmi (:avih) from comment #20)
> Assuming the main reason for this bug always include resizing (possibly
> combined with clipping) I think the goal is to avoid resizing at the newtab
> page.

No, don't assume that. Prove it. Reduce the testcase and confirm. We're hitting a slow path. Let's find out why and not hit the path. This should be an easy bug.

We don't block for a solid second switching to gmail which is an order of magnitude more complicated then a new tab page with 9 images and a few round corner. There's no reason it has to be this slow.

Comment 22

[Avi Halachmi (:avih)]

(In reply to Benoit Girard (:BenWa) from comment #21)
> (In reply to Avi Halachmi (:avih) from comment #20)
> > Assuming the main reason for this bug always include resizing (possibly
> > combined with clipping) I think the goal is to avoid resizing at the newtab
> > page.
> 
> No, don't assume that. ...

I wasn't assuming, which is why I started the sentence as I did. I can't reproduce it, and I don't even know on which systems it happens. I know it doesn't happen on my systems.

> We don't block for a solid second switching to gmail which is an order of
> magnitude more complicated then a new tab page with 9 images and a few round
> corner. There's no reason it has to be this slow.

I couldn't agree more.

Comment 23

[Benoit Girard (:BenWa)]

Well I get it 100% of the time on retina. I have 3 by 5 tiles, none of which are directory tiles.

Comment 24

[Bryan Clark (DevTools PM) [:clarkbw]]

is bug 1063804 a dupe of this?

Comment 26

[Tim Taubert [:ttaubert] (inactive)]

Turning this into a meta bug. We'll have more dependencies soon.

Comment 27

[Ed Lee :Mardak]

Bug 1059558 was originally filed for TART regression because it started running tests with images instead of blank tiles. The suggestion there was to delay image loading somehow so the opening new tab animation could finish before needing to show images. Perhaps if we can load images in a way that doesn't cause flicker (i.e., first appearing blank then momentarily appearing)..... unless we want to make it progressively show tiles after a short delay of each other (similar to the ui tour word by word display)..?

Comment 28

[Ed Lee :Mardak]

(In reply to Ed Lee :Mardak from comment #27)
> Bug 1059558 was originally filed for TART regression
Sorry, I meant bug 1004911.

Comment 29

[Ed Lee :Mardak]

ttaubert, did the investigations lead to any low hanging fruit that we could fix for 34? Are there some ideas that just need someone to implement, and I can help out?

Comment 30

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Ed Lee :Mardak from comment #29)
> ttaubert, did the investigations lead to any low hanging fruit that we could
> fix for 34? Are there some ideas that just need someone to implement, and I
> can help out?

We didn't find too many low-hanging fruits unfortunately. Bug 1077652 isn't too hard to implement but currently blocked by a performance issue I'm still investigating. We're meeting with UX about bug 1073997 on Monday and will see whether that's another acceptable solution.

One thing left is making thumbnails have the correct image size, we deferred that decision to see whether we could get away with not implementing that. It's probably going to be quite involved as other features use thumbnails too.

The other thing left is getting rid of _resizeGrid() by using media queries and a static page layout. We could in one take convert about:newtab to html and let it have a static layout without flexboxes etc. that we don't need anymore. Converting to html would be nice for e10s too but we haven't thought too deeply about that yet...

Comment 31

[Lawrence Mandel [:lmandel] (use needinfo)]

We're planning to ship 34 beta5 tomorrow. That leaves ~2 weeks to get any fix in for 34. If there is something that we can do in 34 can you please identify it so that we can find an owner and get it onto beta soon?

Comment 32

[Tim Taubert [:ttaubert] (inactive)]

There unfortunately are no quick fixes that we would feel comfortable uplifting to 34. The two bugs we're looking at currently should have at least some baking time on Nightly.

Comment 33

[Lawrence Mandel [:lmandel] (use needinfo)]

Given comment 32, I'm going to mark 34 as wontfix and track this for 35. I'll note that this is a meta bug and it would be better to track the individual bugs that we expect to make a significant dent in this regression instead of the meta bug.

Comment 34

[Lukas Blakk [:lsblakk] use ?needinfo]

I'll go one step further than Lawrence and untrack this meta bug since we can't add much value here in terms of driving a fix into a specific release.  We can track individual efforts to impact this overall issue separately.

Comment 35

[Ed Lee :Mardak]

Looks like the rounded corner clipping of resized images usage of Tiles is gone now.