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

RESOLVED FIXED in Firefox 39

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: julienw, Assigned: ting)

Tracking

({perf, regression})

unspecified
2.2 S8 (20mar)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

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

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

4 years ago
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.
(Reporter)

Updated

4 years ago
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.
Posted 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

Comment 4

4 years ago
Triage is blocking based on the fact that this is a perf regression. Keyword added.
blocking-b2g: 2.2? → 2.2+
Keywords: regression

Updated

4 years ago
Component: General → Performance

Comment 5

4 years ago
Hi Fabrice, will you continue to track this issue? Thanks.
Flags: needinfo?(fabrice)

Updated

4 years ago
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)

Comment 7

4 years ago
Tingyu, could you share what you think?
Blocks: AppStartup
Flags: needinfo?(janus926)
(Assignee)

Comment 8

4 years ago
I don't have enough context here, no ideas what to comment. Is there a profile I can reference to?
Flags: needinfo?(janus926)
(Assignee)

Comment 9

4 years ago
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.
(Assignee)

Comment 11

4 years ago
Better get a profile to confirm the issue before proceeding.
Flags: needinfo?(felash)
(Reporter)

Comment 12

4 years ago
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)
(Assignee)

Comment 13

4 years ago
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.
(Assignee)

Comment 14

4 years ago
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).
(Assignee)

Comment 16

4 years ago
Posted 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+
(Assignee)

Comment 17

4 years ago
Posted patch patchSplinter Review
Added "r=fabrice" to commit message.
Attachment #8558231 - Attachment is obsolete: true
Attachment #8579104 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/72a1ee1bc952
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S8 (20mar)
(Assignee)

Comment 20

4 years ago
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.