908225 - Eideticker regression in startup -> about:home with new UI

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect, P5)

Description

[William Lachance (:wlach)]

After the new UI landed (looks great btw), it looks like there's been a small regression (just under a second) in average load times (from app not loaded to about:home fully visible) according to eideticker:

http://eideticker.wrla.ch/#/samsung-gn/startup-abouthome-dirty/timetostableframe

The new eideticker "frame difference view" may illuminate some more details about what's going on.

Old: http://eideticker.wrla.ch/framediff-view.html?title=Frame%20Difference%20Startup%20to%20about:home%20%28dirty%20profile%29%20%282013-08-20%29&video=videos/video-1377070732.28.webm&framediff=framediffs/framediff-1377070742.01.json

New: http://eideticker.wrla.ch/framediff-view.html?title=Frame%20Difference%20Startup%20to%20about:home%20%28dirty%20profile%29%20%282013-08-21%29&video=videos/video-1377157380.32.webm&framediff=framediffs/framediff-1377157389.72.json

Perhaps this regression is considered acceptable, but I thought I'd file the bug so we were aware of this issue.

Comment 1

[Sriram Ramasubramanian [:sriram]]

This is expected. Earlier, there was a query to find the top bookmarks alone. Now we load "all" the bookmarks as a list. Hence two queries, re-layouts once cursor loads, and so on.

Comment 2

[Nathan Froyd [:froydnj]]

(In reply to Sriram Ramasubramanian [:sriram] from comment #1)
> This is expected. Earlier, there was a query to find the top bookmarks
> alone. Now we load "all" the bookmarks as a list. Hence two queries,
> re-layouts once cursor loads, and so on.

That seems suboptimal, especially if it's showing up on eideticker, where profiles hardly have any bookmarks...

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Sriram Ramasubramanian [:sriram] from comment #1)
> This is expected. Earlier, there was a query to find the top bookmarks
> alone. Now we load "all" the bookmarks as a list. Hence two queries,
> re-layouts once cursor loads, and so on.

It might be expected, but I think we need to optimize it now that the new code is mostly functional.

Comment 4

[Lucas Rocha (:lucasr)]

(In reply to Mark Finkle (:mfinkle) from comment #3)
> (In reply to Sriram Ramasubramanian [:sriram] from comment #1)
> > This is expected. Earlier, there was a query to find the top bookmarks
> > alone. Now we load "all" the bookmarks as a list. Hence two queries,
> > re-layouts once cursor loads, and so on.
> 
> It might be expected, but I think we need to optimize it now that the new
> code is mostly functional.

+1

Comment 5

[Sriram Ramasubramanian [:sriram]]

I somehow feel the patch landed in Bug 904172 would help reduce 500ms. I'll wait for a day before debugging further.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Sriram Ramasubramanian [:sriram] from comment #5)
> I somehow feel the patch landed in Bug 904172 would help reduce 500ms. I'll
> wait for a day before debugging further.

Looks like your patch brought the time down ~0.3-0.5 secs, as you predicted.

The eideticker result are noisy, but noisy for a reason. There is a low and high swing to this test. The low end is the "normal" situation. The high end is a case where one of the thumbnail backgrounds takes a long time to show up. Eideticker correctly catches this, but it's intermittent.We need to find out why that happens. Example frame difference traces:

Low trace:
http://eideticker.mozilla.org/framediff-view.html?title=Frame%20Difference%20Startup%20to%20about:home%20%28fresh%20profile%29%20%282013-09-10%29&video=videos/video-1378884833.72.webm&framediff=framediffs/framediff-1378884843.28.json&actionlog=

High trace:
http://eideticker.mozilla.org/framediff-view.html?title=Frame%20Difference%20Startup%20to%20about:home%20%28fresh%20profile%29%20%282013-09-10%29&video=videos/video-1378884986.35.webm&framediff=framediffs/framediff-1378884996.02.json&actionlog=

Comment 7

[Sriram Ramasubramanian [:sriram]]

So, is this bug still valid? Do we want to do something about this?

Comment 8

[William Lachance (:wlach)]

(In reply to Sriram Ramasubramanian [:sriram] from comment #7)
> So, is this bug still valid? Do we want to do something about this?

Even on the low-end, there is about a half second regression in the time that it takes for everything to be completely rendered and usable. It's easy to see this if you put two framediffs side-by-side and flip between them. Here's two (on the lower band of how long it takes for things to load i.e. "best case").

Before the change (August 20): http://eideticker.wrla.ch/framediff-view.html?title=Frame%20Difference%20Startup%20to%20about:home%20%28dirty%20profile%29%20%282013-08-20%29&video=videos/video-1377083200.93.webm&framediff=framediffs/framediff-1377083210.64.json&actionlog=
After the change (September 11): http://eideticker.wrla.ch/framediff-view.html?title=Frame%20Difference%20Startup%20to%20about:home%20%28dirty%20profile%29%20%282013-09-11%29&video=videos/video-1378984326.22.webm&framediff=framediffs/framediff-1378984335.82.json&actionlog=

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Right. I think we still need to:
1. Fix the high end noise by removing the late background update. Or at least find out why it is happening.
2. Fix the addition 0.5 sec delay, which could be done in the patch in bug 914872. Or bug 914872 might regress this again.

Comment 10

[Sriram Ramasubramanian [:sriram]]

In the old UI, we showed a default thumbnail if we didnt have the thumbnail for any item in the grid. In the new UI, we show a colored favicon of the url, if there is no thumbnail in the grid. This colored favicon is obtained separately, and cannot be updated in one-go like the older approach. This causes about 130-150ms regression -- which cannot be avoided.

Comment 11

[Sriram Ramasubramanian [:sriram]]

In old UI the graph starts at 0.6s while in the new one it starts at 0.8s. Not sure why this 0.2s regression even before the app starts.
The about:home inflation times that I can see are:
Old: 1.43s to 2.13s (about 0.7s).
New: 1.65s to 2.36s (about 0.7s) -- without the colored favicons.
     1.65s to 2.43s (about 0.8s) -- with the colored favicons.

So the about:home doesn't seem to cause any regression now (other than the 150ms for the colored favicons).

Comment 12

[Lucas Rocha (:lucasr)]

This doesn't seem to be critical for 26. Turning it into a +.

Comment 13

[Brad Lassey [:blassey] (use needinfo?)]

filter on [mass-p5]

Comment 14

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.