1128505 - [mozApps] Preload mozApps for a 30ms launch time boost

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Performance, defect)

Description

[Julien Wajsberg [:julienw]]

Since bug 1115796 landed we observe a ~30-40ms launch-time regression (see last comments on bug 1115796).

We should likely preload mozApps to remove this launch-time performance regression.

Comment 1

[[:fabrice] Fabrice Desré]

I'm not sure what's happening, but for sure we're super slow at getting navigator.mozApps:

I added some timing probes to dom/apps/Webapps.js and here's what I get:

I/Gecko   (13899): Webapps.js loaded in 8 ms.
I/Gecko   (13899): mozApps.init: Getting new Preferences() 0.22364599999991697 / 0.22364599999991697
I/Gecko   (13899): mozApps.init: initDOMRequestHelper 6.952656000000104 / 7.176302000000021
I/Gecko   (13899): mozApps.init: RegisterForMessages 5.38489699999991 / 12.561198999999931
I/Gecko   (13899): mozApps.init: appId && app 2.411145000000033 / 14.972343999999964
I/Gecko   (13899): mozApps.init: done 3.3214060000000245 / 18.29374999999999
I/Browser (13899): Content JS LOG: Getting navigator.mozApps took 60.21286500000008 ms. 
I/Browser (13899):     at init (http://people.mozilla.org/~fdesre/:14:0)

So event though loading the js component and running the init() method for navigator.mozApps should be less than 30ms, in the web page we spend ~60ms in this run. I need to dig into the webidl bindings I guess.

Comment 2

[[:fabrice] Fabrice Desré]

Attachment: mozapps-timing.patch

Comment 3

[Boris Zbarsky [:bzbarsky]]

Does this only happen the first time you touch .mozApps in a given process?  Or the first time you touch it in a given page?

Here's what needs to happen for navigator.mozApps:

1)  Land in Navigator::DoResolve with aId == "mozApps".
2)  Hashtable lookup on the name to find the relevant GlobalNameStruct.
3)  Some checks to see whether the property is enabled, etc.
4)  Another hashtable lookup to see whether we have an already-resolved value.
5)  Construction of the actual object.  This involves doing a createInstance for the
    relevant contractid and whatnot.  That probably means going through a bunch of xpcom
    goop, as well as reading the actual JS file off disk and so forth.
6)  Returning the object.

I would not be surprised if the time is mostly in step 5 there, reading from storage and whatever other work createInstance ends up doing....  You can instrument $objdir/dom/binding/AppsBinding.cpp, the ConstructNavigatorObjectHelper bit, to see how long ConstructJSImplementation takes, for example.

Comment 4

[Stephany Wilkes]

Triage is blocking based on the fact that this is a perf regression. Keyword added.

Comment 5

[howie [:howie]]

Hi Fabrice, will you continue to track this issue? Thanks.

Comment 6

[[:fabrice] Fabrice Desré]

(In reply to howie [:howie] from comment #5)
> Hi Fabrice, will you continue to track this issue? Thanks.

I won't have time to work on this bug this week, so if someone else is available, just steal it.

Comment 7

[Bobby Chien]

Tingyu, could you share what you think?

Comment 8

[Ting-Yu Chou [:ting] (away)]

I don't have enough context here, no ideas what to comment. Is there a profile I can reference to?

Comment 9

[Ting-Yu Chou [:ting] (away)]

So the constructor of navigator.mozApps is DOMApplicationRegistry. If the 30ms was observed in child, preload.js does import AppsServiceChild.jsm, is there anything I misunderstood?

Comment 10

[[:fabrice] Fabrice Desré]

The constructor of navigator.mozApps is in Webapps.js, not Webapps.jsm. But indeed we preload AppsServiceChild.jsm so we should not have any heavy lifting to do on first instantiation.

Comment 11

[Ting-Yu Chou [:ting] (away)]

Better get a profile to confirm the issue before proceeding.

Comment 12

[Julien Wajsberg [:julienw]]

I'm bad at profiling using the profiler tool. But if you look at attachment 8554585 [details] [diff] [review] in bug 1115796 you'll see how I see that we lose 30ms because of this. Please also see bug 1115796 comment 26.

Comment 13

[Ting-Yu Chou [:ting] (away)]

I captured a profile on Flame 319MB with master:

http://people.mozilla.org/~bgirard/cleopatra/#report=abebc4a06ba23d4e123993f04c3db09dfdfceec3

Check [2390,2470] for the initResources()@l10n.js. So before instantiating a WebappsRegistry, the script Webapps.js is compiled.

Comment 14

[Ting-Yu Chou [:ting] (away)]

StartupCache is disabled on B2G, are there any other means we can preload/precompile Webapps.js?

Comment 15

[[:fabrice] Fabrice Desré]

(In reply to Ting-Yu Chou [:ting] from comment #14)
> StartupCache is disabled on B2G, are there any other means we can
> preload/precompile Webapps.js?

You can create an instance of the @mozilla.org/webapps;1 component (just QI to nsISupports since you don't really need to use it).

Comment 16

[Ting-Yu Chou [:ting] (away)]

Attachment: patch v1

Profile with the patch:

http://people.mozilla.org/~bgirard/cleopatra/#report=1829204232de9493c9f7453446038c64b572cae4

CompileFunction() is gone and initResources() now takes ~40ms, which is ~35ms improvement compare to the profile at comment 13.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bef7b82ac93f

Comment 17

[Ting-Yu Chou [:ting] (away)]

Attachment: patch

Added "r=fabrice" to commit message.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/72a1ee1bc952

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/72a1ee1bc952

Comment 20

[Ting-Yu Chou [:ting] (away)]

Comment on attachment 8579112 [details] [diff] [review]
patch

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): I don't really think it's a regression from bug 1115796, it's just never been preloaded.
User impact if declined: ~35ms longer application startup time, all the applications using l10n.js are affected
Testing completed: b2g desktop & emulator, linux
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: n/a

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/12139abae350