Closed Bug 1210691 Opened 9 years ago Closed 9 years ago

[Music][NGA] Optimize script loading

Categories

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

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

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: stas, Assigned: justindarc)

References

Details

Attachments

(2 files)

[gaia] stasm:1210691-music-defer > mozilla-b2g:master
46 bytes, text/x-github-pull-request
justindarc
: feedback-
Details | Review
[gaia] justindarc:bug1210691 > mozilla-b2g:master
46 bytes, text/x-github-pull-request
squib
: review+
Details | Review 
I started experimenting with a bridge-enabled version of l20n in Music and I realized that I'd need bridge.js/client.js to be included in <head> before l20n.js.  I then experimented with moving all scripts to <head> and deferring them.  Raptor perf results look promising on my flame-kk.

master:

[gaia] raptor test coldlaunch --app music --output quiet --runs 10
| Metric                | Mean     | Median   | Min    | Max    | StdDev  | p95    |
| --------------------- | -------- | -------- | ------ | ------ | ------- | ------ |
| navigationLoaded      | 3507     | 3522.500 | 3002   | 3725   | 182.579 | 3725   |
| navigationInteractive | 3526.500 | 3540.500 | 3023   | 3745   | 181.933 | 3745   |
| visuallyLoaded        | 4413.200 | 4387     | 4066   | 4650   | 147.380 | 4650   |
| contentInteractive    | 4414     | 4387     | 4067   | 4650   | 147.673 | 4650   |
| fullyLoaded           | 4414.500 | 4387.500 | 4067   | 4651   | 147.772 | 4651   |
| uss                   | 20.980   | 20.713   | 20.297 | 22.074 | 0.640   | 22.074 |
| pss                   | 25.168   | 24.890   | 24.479 | 26.291 | 0.653   | 26.291 |
| rss                   | 43.452   | 43.156   | 42.758 | 44.617 | 0.671   | 44.617 |


wip with all scripts in <head> and deferred

[gaia] raptor test coldlaunch --app music --output quiet --runs 10
| Metric                | Mean     | Median   | Min    | Max    | StdDev  | p95    |
| --------------------- | -------- | -------- | ------ | ------ | ------- | ------ |
| navigationLoaded      | 3375.500 | 3422.500 | 2865   | 3547   | 182.160 | 3547   |
| navigationInteractive | 3394.800 | 3441     | 2884   | 3564   | 182.205 | 3564   |
| visuallyLoaded        | 4218     | 4272     | 3758   | 4366   | 168.853 | 4366   |
| contentInteractive    | 4218.400 | 4273     | 3759   | 4366   | 168.722 | 4366   |
| fullyLoaded           | 4219.100 | 4273.500 | 3760   | 4366   | 168.554 | 4366   |
| uss                   | 21.086   | 21.014   | 20.195 | 22     | 0.654   | 22     |
| rss                   | 43.574   | 43.494   | 42.656 | 44.523 | 0.683   | 44.523 |
| pss                   | 25.279   | 25.204   | 24.378 | 26.206 | 0.665   | 26.206 |
Comment on attachment 8668811 [details] [review]
[gaia] stasm:1210691-music-defer > mozilla-b2g:master

Here's an early wip in which I only modified index.html and views/home/index.html.  Justin, what are your thoughts on this?  Are there any down-sides to this approach?
Attachment #8668811 - Flags: feedback?(jdarcangelo)
Blocks: 1210720
Comment on attachment 8668811 [details] [review]
[gaia] stasm:1210691-music-defer > mozilla-b2g:master

Ok, so, I believe that deferring everything could potentially expose us to a race condition where web components are not properly initialized before we expect them to be and use them.

I think we can safely move all scripts to the <head> though, but we just need to be somewhat selective about which scripts we defer. All scripts currently in the <head> (with the exception of the l10n scripts) must *not* be deferred since they are for loading and initializing the web components on the page. I also think we may be able to defer all scripts that are currently in the <body> if we move them to the <head>, but I'd need to confirm this.

I know this is a preliminary patch, but also note that in addition do /apps/music/index.html, you will also need to change the similar /apps/music/open.html and /apps/music/pick.html as well. Those two files are the entry points for the "open" and "pick" activities.
Attachment #8668811 - Flags: feedback?(jdarcangelo) → feedback-
Thanks for the explanation, Justin!  If webcomponents must not be deferred, then we also should block on l20n.js (because components use document.1l0n) and, in the future when l20n uses the bridge, bridge.js.

I can work on moving other scripts from <body> to <head>.
Taking this for further investigation into lazy loading whatever scripts we can.
Assignee: nobody → jdarcangelo
Summary: Move all scripts to <head> and defer them → [Music][NGA] Optimize script loading
Attachment #8669219 - Flags: review?(squibblyflabbetydoo)
Comment on attachment 8669219 [details] [review]
[gaia] justindarc:bug1210691 > mozilla-b2g:master

Looks good to me. I think we should backport the database/metadata changes to OGA so that we have a better idea how much NGA-specific stuff is costing us. We can do that in a followup commit, though.
Attachment #8669219 - Flags: review?(squibblyflabbetydoo) → review+
Landed on master:

https://github.com/mozilla-b2g/gaia/commit/25dc0972c221d9facaf0ffe9cec980666166bceb
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: