863210 - Insert panel specific stylesheets when the app is idle rather than on panel opening

Status: RESOLVED FIXED

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

Description

[Anthony Ricaud (:rik)]

To speed up app opening, we delayed stylesheet insertion in bug 841760. But this is showing up a lot in profiles when opening a panel. So we should probably insert them as soon as possible, probably using IdleObserver.

Comment 1

[Anthony Ricaud (:rik)]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9411

Pointer to Github pull-request

Comment 2

[Evelyn Hung]

Comment on attachment 741856 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9411

redirect review request to Arthur, he is interested in this topic.

Comment 3

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 741856 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9411

Hi Anthony,

I'm not sure about the improvement made by lazy loading CSS files because I have no idea on profiling it (show me if you know the secret! :D). 

The most concern I have about this patch is loading all of the CSS files at a time. It would be better to load (or lazy load) needed CSS files when users enter each subpanel. In addition to that, to avoid loading or applying duplicate CSS files, adding some checks before loading the files might be a good idea.

Comment 4

[Anthony Ricaud (:rik)]

(In reply to Arthur Chen [:arthurcc] from comment #3)
> Hi Anthony,
> 
> I'm not sure about the improvement made by lazy loading CSS files because I
> have no idea on profiling it (show me if you know the secret! :D). 
We had a session about this last week, you need a special build [1]. So we profiled with some Gecko engineers. [2]
Rather than profiling, you can check that the panel opening time is improving (while not regressing app launch time).
For the "Device information" panel, it goes from 2s to 1.3s to load.

> The most concern I have about this patch is loading all of the CSS files at
> a time. It would be better to load (or lazy load) needed CSS files when
> users enter each subpanel.
This is what we're doing right now but this is not performant because Gecko has to recompute a lot of things each time we insert a stylesheet. You can see in [2] that 30% of the time is spent in PresShell::flushPendingNotifications. So in this patch, we're trying to load the stylesheets before opening a panel. (And if the app was not idle enough, we load them for the first panel).

> In addition to that, to avoid loading or applying
> duplicate CSS files, adding some checks before loading the files might be a
> good idea.
LazyLoader maintains a list of already loaded stylesheets so this is already taking care of.

[1] See "Other Ways to Profile B2G" on https://developer.mozilla.org/en-US/docs/Performance/Profiling_with_the_Built-in_Profiler#Profiling_Boot_to_Gecko_%28with_a_real_device%29
[2] http://people.mozilla.com/~bgirard/cleopatra/#report=eb50d591cc3c16c345441734ec28a075257cc0de

Comment 5

[Anthony Ricaud (:rik)]

Attachment: Dirty patch to measure perf improvements

You can use a patch like this to measure the difference before and after.

Comment 6

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 741856 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9411

Thank you for the explanation and work! It sounds a good deal to trade the launch time of the first panel for the others. I just have only one suggestion on the naming of the function. r=me.

Comment 7

[Anthony Ricaud (:rik)]

I landed without the change to the function name. Given that it will only load the stylesheets the first time, I like the name to reflect that.

Landed with https://github.com/mozilla-b2g/gaia/commit/863be27daee9494cf2013d2636ee5da12629627b