Closed Bug 1210737 Opened 9 years ago Closed 9 years ago

[New-Homescreen] Startup time is noticeably worse when compared to verticalhome

Categories

(Firefox OS Graveyard :: Gaia::Homescreen, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::Homescreen
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: cwiiis, Assigned: cwiiis)

References

Details

(Keywords: perf, Whiteboard: [systemsfe])

Attachments

(2 files, 1 obsolete file)

[gaia] Cwiiis:bug1210737-new-homescreen-startup-opt-sync > mozilla-b2g:master
46 bytes, text/x-github-pull-request
gmarty
: review+
Details | Review
[gaia] Cwiiis:bug1210737-new-homescreen-startup-opt-order > mozilla-b2g:master
46 bytes, text/x-github-pull-request
cwiiis
: review+
Details | Review 
Using raptor and the restart-b2g test to collect results, over 5 runs on a Sony Xperia Z3C (aries);

homescreen:

| Metric                | Mean      | Median | Min    | Max    | StdDev  | p95       |
| --------------------- | --------- | ------ | ------ | ------ | ------- | --------- |
| navigationLoaded      | 12449.800 | 12489  | 12132  | 12718  | 195.677 | 12449.800 |
| navigationInteractive | 12510.400 | 12534  | 12213  | 12763  | 187.354 | 12510.400 |
| visuallyLoaded        | 15389.600 | 15447  | 14675  | 15875  | 424.519 | 15389.600 |
| contentInteractive    | 15394.600 | 15452  | 14680  | 15880  | 424.519 | 15394.600 |
| fullyLoaded           | 18510     | 18368  | 17831  | 19067  | 444.180 | 18510     |
| uss                   | 41.041    | 39.570 | 36.594 | 45.848 | 3.636   | 41.041    |
| pss                   | 47.353    | 45.887 | 42.909 | 52.163 | 3.640   | 47.353    |
| rss                   | 63.224    | 61.754 | 58.773 | 68.031 | 3.639   | 63.224    |

verticalhome:

| Metric                | Mean      | Median | Min    | Max    | StdDev  | p95       |
| --------------------- | --------- | ------ | ------ | ------ | ------- | --------- |
| navigationLoaded      | 12679.800 | 12786  | 12376  | 12847  | 181.245 | 12679.800 |
| navigationInteractive | 12684.800 | 12791  | 12381  | 12852  | 181.245 | 12684.800 |
| visuallyLoaded        | 14923     | 14912  | 14449  | 15209  | 270.717 | 14923     |
| contentInteractive    | 14927.800 | 14917  | 14454  | 15215  | 270.992 | 14927.800 |
| fullyLoaded           | 15309.800 | 15329  | 14845  | 15647  | 273.929 | 15309.800 |
| uss                   | 31.411    | 31.836 | 30.441 | 32.148 | 0.718   | 31.411    |
| rss                   | 53.547    | 53.961 | 52.578 | 54.285 | 0.715   | 53.547    |
| pss                   | 37.688    | 38.119 | 36.715 | 38.422 | 0.718   | 37.688    |

There is a slight regression for visuallyLoaded and an ~3 second regression to fullyLoaded - these numbers are likely exaggerated on slower devices. We can probably do better, opening this to track improvements.

We may have to accept some regression here (verticalhome has its own mirror of the applications and bookmarks datastores that allows it faster startup, but has its own issues - it also doesn't support pinned pages, which may impact startup performance negatively), but I'm aware of areas that are sub-optimal in homescreen's startup path that can be improved.
Comment on attachment 8668881 [details] [review]
[gaia] Cwiiis:bug1210737-new-homescreen-startup-opt-sync > mozilla-b2g:master

This was one of the obvious optimisations I've been meaning to make. Numbers with this patch applied:

| Metric                | Mean      | Median | Min    | Max    | StdDev  | p95       |
| --------------------- | --------- | ------ | ------ | ------ | ------- | --------- |
| navigationLoaded      | 12562.200 | 12576  | 11693  | 13212  | 548.036 | 12562.200 |
| navigationInteractive | 12612.400 | 12623  | 11731  | 13267  | 553.583 | 12612.400 |
| visuallyLoaded        | 14547.400 | 14897  | 13733  | 15218  | 602.841 | 14547.400 |
| contentInteractive    | 14552.800 | 14903  | 13739  | 15223  | 602.687 | 14552.800 |
| fullyLoaded           | 17828.600 | 18055  | 16966  | 18628  | 635.969 | 17828.600 |
| uss                   | 46.207    | 48.168 | 41.918 | 49.770 | 3.135   | 46.207    |
| pss                   | 52.526    | 54.473 | 48.212 | 56.062 | 3.152   | 52.526    |
| rss                   | 68.467    | 70.352 | 64.098 | 71.945 | 3.190   | 68.467    |

Almost a second improvement in visuallyLoaded, which of course cascades to fullyLoaded. I expect this to have a larger impact on slower devices (it massively cuts down on relayouting/painting during startup).
Attachment #8668881 - Flags: review?(gmarty)
Keywords: leave-open, perf
Whiteboard: [systemsfe]
Comment on attachment 8668881 [details] [review]
[gaia] Cwiiis:bug1210737-new-homescreen-startup-opt-sync > mozilla-b2g:master

The patch looks good to me. Don't we need at least a simple test for app.js?
Attachment #8668881 - Flags: review?(gmarty) → review+
(In reply to Guillaume Marty [:gmarty] from comment #3)
> Comment on attachment 8668881 [details] [review]
> [gaia] Cwiiis:bug1210737-new-homescreen-startup-opt-sync > mozilla-b2g:master
> 
> The patch looks good to me. Don't we need at least a simple test for app.js?

you're right, I'll add tests to make sure freeze/thaw are called at the right time. If they aren't, marionette tests would totally break, but nice to have it covered in unit tests too.
Test added and merged: https://github.com/mozilla-b2g/gaia/commit/f3d9981dccfa4dfdfcb865d95fdcfb85e4077e1e

The last obvious, but tedious thing to improve startup performance is not to block app and bookmarks loading on metadata loading.

Currently, the startup flow is like this:

Load settings from local storage -> Load icon metadata (order and cached, processed icons) -> Load apps and bookmarks

That third step doesn't strictly depend on the 2nd, only adding them to the grid depends on the 2nd. It's much clearer and easier to do it this way, but we could 1) Load apps and bookmarks *while* we load the icon metadata, and 2) Load cached icons separately from icon order.

I expect we could shave off a little bit from visuallyComplete by doing (1) and a fair amount by doing (2). Doing 2 is a bit sneaky though, in my opinion - are we really visually complete without icons? When I get to it, I'll start off doing (1) and see where that gets us.
Was talking to gmarty about this, we think rather than (1) or (2) that I mention above, instead of blocking on metadata access, we should load apps/bookmarks async and display them as we get results from metadata continuously (and ordered by icon order if possible).

We could think about loading icons async from order after that too, but I think that might get us where we need to be closer and makes a lot more sense (it should reduce layout cost to add icons in the right order too). If we can query results in the right order, we should be able to legitimately get to a visuallyLoaded state a fair bit quicker.

This will need a fair amount of refactoring though...
Comment on attachment 8669761 [details] [review]
[gaia] Cwiiis:bug1210737-new-homescreen-startup-opt-order > mozilla-b2g:master

This patch makes the metadata, app and bookmark loading fully asynchronous - All three happen concurrently and metadata results are loaded in grid order and processed asynchronously as they come in, allowing us to much faster display the first screen of apps. In addition, the grid is hidden and placed in a mode that allows faster operation while we're populating the first screen of icons.

clean raptor build, master gecko/gaia, verticalhome:

| Metric                | Mean      | Median | Min    | Max    | StdDev  | p95       |
| --------------------- | --------- | ------ | ------ | ------ | ------- | --------- |
| navigationLoaded      | 10756.600 | 10849  | 10164  | 11035  | 304.217 | 10756.600 |
| navigationInteractive | 10761.600 | 10854  | 10169  | 11040  | 304.217 | 10761.600 |
| visuallyLoaded        | 12714.400 | 12776  | 12166  | 13061  | 317.982 | 12714.400 |
| contentInteractive    | 12719.200 | 12780  | 12171  | 13066  | 317.943 | 12719.200 |
| fullyLoaded           | 12971.800 | 13163  | 12404  | 13273  | 330.534 | 12971.800 |
| rss                   | 36.760    | 36.621 | 35.578 | 38.309 | 0.877   | 36.760    |
| pss                   | 24.226    | 24.104 | 23.052 | 25.726 | 0.856   | 24.226    |
| uss                   | 17.650    | 17.574 | 16.367 | 19.137 | 0.880   | 17.650    |


homescreen with this patch:

| Metric                | Mean      | Median | Min    | Max    | StdDev  | p95       |
| --------------------- | --------- | ------ | ------ | ------ | ------- | --------- |
| navigationLoaded      | 10643.200 | 10697  | 10384  | 10920  | 196.046 | 10643.200 |
| navigationInteractive | 10734.800 | 10810  | 10464  | 11002  | 194.075 | 10734.800 |
| visuallyLoaded        | 11417     | 11463  | 11210  | 11650  | 156.885 | 11417     |
| contentInteractive    | 11421.600 | 11468  | 11215  | 11654  | 156.503 | 11421.600 |
| fullyLoaded           | 14057.200 | 14107  | 13820  | 14352  | 207.433 | 14057.200 |
| rss                   | 36.418    | 36.465 | 36.148 | 36.668 | 0.187   | 36.418    |
| pss                   | 23.761    | 23.804 | 23.480 | 24.062 | 0.204   | 23.761    |
| uss                   | 17.101    | 17.137 | 16.809 | 17.445 | 0.220   | 17.101    |

So this beats verticalhome considerably for visuallyLoaded and contentInteractive (and there's no cheating, the screen is painted and you can interact with it at this point), but we're still trailing behind on fullyLoaded. Only by a second, but I'll have a think about what can be done. I still think we can get closer.

We may possibly need to take a hit here... verticalhome's method of caching apps can be faster, but it's (at least imho) at the large expense of code complexity and robustness.

This isn't ready for review yet, I still need to amend/fix tests.
Comment on attachment 8669761 [details] [review]
[gaia] Cwiiis:bug1210737-new-homescreen-startup-opt-order > mozilla-b2g:master

Tests should be passing now.
Attachment #8669761 - Flags: review?(gmarty)
Comment on attachment 8669761 [details] [review]
[gaia] Cwiiis:bug1210737-new-homescreen-startup-opt-order > mozilla-b2g:master

Wow. It feels really fast now!
I'm r+ing but some comments left on Github need to be addressed before landing.
Attachment #8669761 - Flags: review?(gmarty) → review+
Comments addressed (I hope) and merged: https://github.com/mozilla-b2g/gaia/commit/882697838fa66da51c0a90a4f658f95b578d788b

Follow-up issue, platform's odd asynchronous image loading is now more evident (previously, all icons would appear at once, mostly blank (so you just see the subtitle), and icons would pop in semi-randomly - now, you tend to get almost a whole page of icons and images simultaneously, then the rest of the images pop in one-by-one). I'll see if I can find someone on platform who knows what we can do here (currently it appears that Image.onload and the actual display of the image are not linked).

Leaving open to see if we can reach parity on fullyLoaded. I'd say it's looking pretty good on the whole now though.
Attachment #8669761 - Attachment is obsolete: true
Flags: needinfo?(chrislord.net)
Comment on attachment 8670777 [details] [review]
[gaia] Cwiiis:bug1210737-new-homescreen-startup-opt-order > mozilla-b2g:master

Carrying r=gmarty, changes are trivial.
Attachment #8670777 - Flags: review+
ftr, the failure was legit - good thing we have decent marionette test coverage :)
Merged: https://github.com/mozilla-b2g/gaia/commit/5ecc0634430e60f730d09b59822b9959c16d8d97
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
This was apparently broken by bug 1196809. De-reverted in https://github.com/mozilla-b2g/gaia/commit/dd9fe1df3f15991e5f7400b8a72f5b7bd2ee695a
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
I actually didn't mean to close this before, but this will get complicated fast, so I'll open new bugs for further optimisations. I'd say it's in a reasonable state now.
Flags: needinfo?(chrislord.net)
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: