1031890 - Vertical homescreen takes a long time to load

Status: RESOLVED FIXED

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

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

The new homescreen takes a long time to reload when it has been killed by an OOM.

This is a regression from the previous version, as it happens a lot of low memory devices.

I think this is mostly due to the fact that the homescreen waits for mozApps to be ready before restarting completely, while the previous one was keeping a local copy of mozApps in order to start faster.

Comment 1

[Jason Smith [:jsmith]]

What's the length of the time to reload post the OOM?

Comment 2

[Kevin Grandon :kgrandon]

We are currently storing the bare minimum to match our local indexedDB object to a Mozapp object[1], and the icon data (in case the user is offline). I was hesitant to add additional data to this, but because we are already storing the icon data, it may not be too bad to store the other required fields to render the icon pre-mozApps results, mainly name I think.

One tricky problem to solve would be the state handling. We need the mozApps API to determine the current state, and display the proper icon state (paused, unrecoverable error, etc). Additionally, if an app is deleted by another homescreen, we currently remove that before rendering. If we do this after rendering we might have to remove the icon later and shift the rendered icons.

It should be possible to do things in the item store in parallel, but this is going to be much harder than in 1.4 due to our additional mozapp features and states that we now support.

Let's get a profile here, and take some measurements of the various startup times. My preference would be to investigate if we can make Mozapps faster, it is notoriously slow and a speedup there would help many parts of the OS.

[1] https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/items/mozapp.js#L31

Comment 3

[Gregor Wagner [:gwagner]]

What is the delta to the old homescreen?

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Gregor Wagner [:gwagner] from comment #3)
> What is the delta to the old homescreen?

I don't have numbers handy, but it is likely huge... Just tried both, kill them and see.

Comment 5

[Cristian Rodriguez (:crdlc)]

Attachment: Pull Request - Cache decorated icons

Basically it holds decorated blobs in indexedDB and they are displayed when once icons starts rendering. Furthermore I moved the mozApps.getAll() to the beginning of the document in order to have the mozApps asap. We can see the icons a couple of seconds before. This patch does not except to be the definitive patch, just an idea to explore and analyze thought

Comment 6

[Kevin Grandon :kgrandon]

Comment on attachment 8448608 [details]
Pull Request - Cache decorated icons

I haven't tested it yet, but I think I am pretty happy with it. I do think that some of the caching/re-rendering logic could be simplified, but I need to think about it a bit more. E.g., I'm not sure we need to cache the icon if it's not decorated, so it seems like if we have the decorated cache, we can simply just use it? Assuming we clean/fix the cache on the proper events.

Comment 7

[Kevin Grandon :kgrandon]

Cristian - assigning this to you since you have a patch underway that will mostly solve this one. Thanks!

Comment 8

[Kevin Grandon :kgrandon]

Comment on attachment 8448608 [details]
Pull Request - Cache decorated icons

Clearing review for now. It appears we have come to the difficult part.

We spent a lot of time ensuring that we had marionette tests for homescreen related icon things, and some of these tests are broken. I'm not sure if they're surfacing actual bugs, or we need to just update the tests, but we need to fix this.

This is probably going to be a long, difficult battle, and I will try to pitch in a bit this week to help. One thing that might help us is to split up the patch a bit - perhaps landing the lazy loading confirm dialog changes first for example.

Comment 9

[Cristian Rodriguez (:crdlc)]

Attachment: Pull Request - Lazy load for libraries

This patch loads lazily several files which are not needed while the home is rendering

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 8449997 [details]
Pull Request - Lazy load for libraries

I tested this and couldn't find any problems. Let's land this after getting a green build on either gaia-try or travis. Thanks!

Comment 11

[Cristian Rodriguez (:crdlc)]

Merged in master the first patch for lazy load:

https://github.com/crdlc/gaia/commit/6ad82297d636daa0c716cdb35fb939222486635e

Comment 12

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8448608 [details]
Pull Request - Cache decorated icons

The code is done and working fine although some marionette tests are broken related to icons :( I will investigate tomorrow a bit about this issue

Comment 13

[Kevin Grandon :kgrandon]

Comment on attachment 8448608 [details]
Pull Request - Cache decorated icons

Clearing review until we have the tests passing.

(In reply to Cristian Rodriguez (:crdlc) from comment #12)
> Comment on attachment 8448608 [details]
> Cache decorated icons
> 
> The code is done and working fine although some marionette tests are broken
> related to icons :( I will investigate tomorrow a bit about this issue

I might try to stick around tonight to help debug this for a bit. We have a few strategies that we use for this. The CLI server can help quite a bit to debug the integration tests here: https://github.com/mozilla-b2g/gaia/blob/master/apps/verticalhome/test/marionette/server/cli.js

Comment 14

[Kevin Grandon :kgrandon]

We also might consider splitting out your mozApps.mgmt startup patch - it may make our lives easier. If you have time it might be worth trying to measure:

1 - Current homescreen startup.
2 - Homescreen startup /w faster mozApps.
3 - Homescreen startup /w faster mozApps and cached decorated icons.

Comment 15

[Cristian Rodriguez (:crdlc)]

Attachment: Video - Master after killing home

Comment 16

[Cristian Rodriguez (:crdlc)]

Attachment: Video - Master + my patch after killing home

Comment 17

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8448608 [details]
Pull Request - Cache decorated icons

Travis is happy now. I fixed a problem thanks to marionette tests :)

Comment 18

[Kevin Grandon :kgrandon]

Comment on attachment 8448608 [details]
Pull Request - Cache decorated icons

Hey.. Sorry, while reviewing I found a change that should've broken a test, and led me to file bug 1034729. We introduced this before, but the test was not named properly, causing it not to run on TBPL/travis. Going to leave a comment on github.

Really sorry about that - it's something we missed earlier =/

Comment 19

[Kevin Grandon :kgrandon]

Also I noticed that this was still causing my homescreen icons to flash like crazy on a nexus4. (Works fine on a flame). As a lot of people actively use the nexus4 to develop or dogfood I think it's important to understand why. I'm going to try to profile this in the next day or two to see if we can get people looking at it.

Comment 20

[Kevin Grandon :kgrandon]

Attachment: Video of icon flashing

Here is a video of the flashing I am seeing on a nexus4 device. I will also try to grab a profile soon.

Comment 21

[Kevin Grandon :kgrandon]

Attachment: Profile - Constant repaints

So it appears something is causing constant repainting on my device, not sure what yet.

Comment 22

[Kevin Grandon :kgrandon]

I'm seeing this in the logs: 

E/GeckoConsole( 1396): [JavaScript Error: "Image corrupt or truncated: data:image/png;base64,<<snip, long, truncated image string>>
E/GeckoConsole( 1396): [JavaScript Error: "NS_ERROR_NOT_AVAILABLE: " {file: "app://verticalhome.gaiamobile.org/shared/elements/gaia_grid/js/grid_icon_renderer.js" line: 89}]

Comment 23

[[:fabrice] Fabrice Desré]

why are we still using base64 images!!

Comment 24

[Kevin Grandon :kgrandon]

(In reply to Fabrice Desré [:fabrice] from comment #23)
> why are we still using base64 images!!

I think it was just copypasta'd from homescreen1. Should we be using a blob URL here instead? It seems like we don't need this image data for more than the lifecycle of this function.

https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/grid_icon_renderer.js#L94

Comment 25

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8448608 [details]
Pull Request - Cache decorated icons

Tests have been fixed and comment 24 was addressed. Please check the Nexus device

Comment 26

[Kevin Grandon :kgrandon]

Comment on attachment 8448608 [details]
Pull Request - Cache decorated icons

(In reply to Cristian Rodriguez (:crdlc) from comment #25)
> Comment on attachment 8448608 [details]
> Pull Request - Cache decorated icons
> 
> Tests have been fixed and comment 24 was addressed. Please check the Nexus
> device

I am unfortunately still seeing icon flashing on this running the latest master and gecko builds. Let me try to check with graphics/layout guys to see if they can help us debug this.

Comment 27

[Kevin Grandon :kgrandon]

Comment on attachment 8451402 [details]
Profile - Constant repaints

Benoit - if you have a minute, could you help us here? When we apply this gaia patch here we see constant icon flashing on the vertical homescreen. This is a profile of when the flashing is occurring, but I don't see any javascript that would be triggering it in the profile.

I was wondering if you could take a quick peek at the profile to see if I am capturing it wrongly, or if you might be familiar with some similar platform bug. Thanks!

Comment 28

[Benoit Girard (:BenWa)]

You have no guarantee to see the Javascript that is causing it in the profile since we have a sampling profiler.

You could dump the display list to see if the display items themselves are changing or you could break internally on the invalidation function and see what the stack is when we invalidate the page. It difficult to shift through all the data these methods report so I suggest you rule out a few things first before trying these approach.

Comment 29

[Cristian Rodriguez (:crdlc)]

Hi Kevin, as you know, I don't have a Nexus device so it is very frustrating for me because I cannot fix the problem :( Please if you detect some problem in my patch let me know to fix it immediately. Thanks a lot

Comment 30

[Kevin Grandon :kgrandon]

(In reply to Cristian Rodriguez (:crdlc) from comment #29)
> Hi Kevin, as you know, I don't have a Nexus device so it is very frustrating
> for me because I cannot fix the problem :( Please if you detect some problem
> in my patch let me know to fix it immediately. Thanks a lot

Yes, I will try to take a look this week. I probably need to get some more platform folks looking at the profile, I couldn't see what could be causing this from JS.

Comment 31

[Benoit Girard (:BenWa)]

I wonder if bug 1033618 may sometimes affect the performance sometimes (instead of preventing the homescreen from starting).

Comment 32

[Kevin Grandon :kgrandon]

(In reply to Benoit Girard (:BenWa) from comment #31)
> I wonder if bug 1033618 may sometimes affect the performance sometimes
> (instead of preventing the homescreen from starting).

The main problem we're seeing now is the icon flashing issue - we mostly have the performance problems solved. Just need to figure out why these icons flash.

Comment 33

[Kevin Grandon :kgrandon]

Comment on attachment 8448608 [details]
Pull Request - Cache decorated icons

Ok, it took all day, but I finally found the problem. I think we were surfacing some platform bug where it caused images to reload over and over again, possibly because we were trying to draw huge icons everywhere. I'm guessing we were throwing away layers and constantly reconstructing them, but not sure.

I left a comment on github at the location where I think we're doing a wrong calculation. If you can address that I think we should be good to go here. Thanks!

Comment 34

[Cristian Rodriguez (:crdlc)]

Merged in master:

1) Lazy load for libraries:

https://github.com/crdlc/gaia/commit/6ad82297d636daa0c716cdb35fb939222486635e

2) Cache decorated icons

https://github.com/crdlc/gaia/commit/0365d7bad8d38ba9c5fce9705f5ef44635bf2fdb

Comment 35

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/9957263271d7217b8cdb19d21f17e1d19fa2a820
v2.0: https://github.com/mozilla-b2g/gaia/commit/d422c2f353816b0781fca219d13f7789205d589c