Closed Bug 1128505 Opened 6 years ago Closed 5 years ago

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

Categories

(Firefox OS Graveyard :: Performance, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.2+, firefox37 wontfix, firefox38 wontfix, firefox39 fixed, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S8 (20mar)
blocking-b2g 2.2+
Tracking Status
firefox37 --- wontfix
firefox38 --- wontfix
firefox39 --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: julienw, Assigned: ting)

References

Details

(Keywords: perf, regression)

Attachments

(1 file, 2 obsolete files)

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.
blocking-b2g: 3.0? → 2.2?
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.
Attached patch mozapps-timing.patch (obsolete) — Splinter Review
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.
Summary: Preload mozApps for a 30m launch time boost → Preload mozApps for a 30ms launch time boost
Triage is blocking based on the fact that this is a perf regression. Keyword added.
blocking-b2g: 2.2? → 2.2+
Keywords: regression
Component: General → Performance
Hi Fabrice, will you continue to track this issue? Thanks.
Flags: needinfo?(fabrice)
Summary: Preload mozApps for a 30ms launch time boost → [mozApps] Preload mozApps for a 30ms launch time boost
(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.
Flags: needinfo?(fabrice)
Tingyu, could you share what you think?
Blocks: AppStartup
Flags: needinfo?(janus926)
I don't have enough context here, no ideas what to comment. Is there a profile I can reference to?
Flags: needinfo?(janus926)
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?
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.
Better get a profile to confirm the issue before proceeding.
Flags: needinfo?(felash)
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.
Flags: needinfo?(felash)
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.
StartupCache is disabled on B2G, are there any other means we can preload/precompile Webapps.js?
(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).
Attached patch patch v1 (obsolete) — Splinter Review
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
Assignee: nobody → janus926
Status: NEW → ASSIGNED
Attachment #8579104 - Flags: review?(fabrice)
Attachment #8579104 - Flags: review?(fabrice) → review+
Attached patch patchSplinter Review
Added "r=fabrice" to commit message.
Attachment #8558231 - Attachment is obsolete: true
Attachment #8579104 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/72a1ee1bc952
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S8 (20mar)
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
Attachment #8579112 - Flags: approval-mozilla-b2g37?
Attachment #8579112 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
You need to log in before you can comment on or make changes to this bug.