Precompile and preload some more scripts that are on the critical startup path

RESOLVED FIXED in Firefox 21

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: cjones, Assigned: cjones)

Tracking

unspecified
mozilla21
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g1819+ fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

forms.js and UserAgentOverrides.jsm overhead was significant on startup.
Created attachment 707338 [details] [diff] [review]
Precompile forms.js and preload UserAgenOverrides.jsm
Attachment #707338 - Flags: review?(fabrice)
Comment on attachment 707338 [details] [diff] [review]
Precompile forms.js and preload UserAgenOverrides.jsm

Review of attachment 707338 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/ipc/preload.js
@@ +71,5 @@
>    Cc["@mozilla.org/thread-manager;1"].getService(Ci["nsIThreadManager"]);
>    Cc["@mozilla.org/toolkit/app-startup;1"].getService(Ci["nsIAppStartup"]);
>    Cc["@mozilla.org/uriloader;1"].getService(Ci["nsIURILoader"]);
>  
> +  Services.io.getProtocolHandler("app");

I'm not sure this is useful, since the AppsProtocolHandler constructor doesn't do much.
Attachment #707338 - Flags: review?(fabrice) → review+
I added that so we execute AppProtocolHandler.js code, but I didn't test it in isolation so you're right that it may not be a win.
(In reply to Chris Jones [:cjones] [:warhammer] from comment #3)
> I added that so we execute AppProtocolHandler.js code, but I didn't test it
> in isolation so you're right that it may not be a win.

It worth it imho. All the JS API / Modules instantiated on the main loading path seems to hurt.

Also are you not going to call UserAgentOverrides.init(); twice? Once in the preload process and once in uao_child.js when it will be inserted via loadFrameScript?
(In reply to Chris Jones [:cjones] [:warhammer] from comment #3)
> I added that so we execute AppProtocolHandler.js code, but I didn't test it
> in isolation so you're right that it may not be a win.

This gets 5-6 samples out of a profile in AppProtocolHandler.js:1.  I think this is a valid win.
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #4)
> Also are you not going to call UserAgentOverrides.init(); twice? Once in the
> preload process and once in uao_child.js when it will be inserted via
> loadFrameScript?

The code has an "already-initialized" check.  So we call it twice but only run it once.
Oh, I see what you mean.  We only TryCacheAndCompile("UAO_child.js"), we don't run it.  But since it doesn't depend on content, we could go ahead and run that and remove the explicit call to UAO.init().
Actually, with the way UAO_child.js is set up, we're technically breaking the rules here and always loading the overrides, regardless of product.  The way it's set up right now we can't legally preload it :|.
Any suggestion for how we can do this without resorting to something "unclean" like a feature-pref for overrides?
Flags: needinfo?(21)
For example, I might write code to check if there are any override prefs set and then load the UAO code if so, but that might horrify chrome hackers.
Flags: needinfo?(fabrice)
Feedback ping.  This is a very nice measurable win.
I'm going to add a feature pref for UA overrides and check it in preload.js to see whether to load and init() UserAgentOverrides, and remove UAO_child.js entirely.

Anyone object?
I would be very happy to see UAO_child.js disappear.
Flags: needinfo?(fabrice)
Created attachment 710432 [details] [diff] [review]
Precompile forms.js and preload UserAgenOverrides.jsm, v2
Attachment #707338 - Attachment is obsolete: true
Attachment #710432 - Flags: review?(fabrice)
Flags: needinfo?(21)
Comment on attachment 710432 [details] [diff] [review]
Precompile forms.js and preload UserAgenOverrides.jsm, v2

Review of attachment 710432 [details] [diff] [review]:
-----------------------------------------------------------------

::: modules/libpref/src/init/all.js
@@ +27,5 @@
>  // This pref exists only for testing purposes. In order to disable all
>  // overrides by default, don't initialize UserAgentOverrides.jsm.
>  pref("general.useragent.site_specific_overrides", true);
>  
> +// Enable UA overrides for shipping product code.

I know what the pref does, but I fail to understand the English sentence. This may be just me, however ;)
Attachment #710432 - Flags: review?(fabrice) → review+
https://hg.mozilla.org/mozilla-central/rev/2716ae058067
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Comment on attachment 710432 [details] [diff] [review]
Precompile forms.js and preload UserAgenOverrides.jsm, v2

Low-risk patch that significantly reduces time spent on critical startup path in app processes.
Attachment #710432 - Flags: approval-mozilla-b2g18?
status-b2g18: --- → affected
status-b2g18-v1.0.0: --- → wontfix
tracking-b2g18: --- → 19+
Comment on attachment 710432 [details] [diff] [review]
Precompile forms.js and preload UserAgenOverrides.jsm, v2

Please go ahead with uplift to the tip of mozilla-b2g18 branch for v1.0.1
Attachment #710432 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
status-b2g18: affected → fixed
status-b2g18-v1.0.1: --- → fixed
status-firefox19: --- → wontfix
status-firefox20: --- → wontfix
status-firefox21: --- → fixed
You need to log in before you can comment on or make changes to this bug.