Closed
Bug 835584
Opened 11 years ago
Closed 11 years ago
Precompile and preload some more scripts that are on the critical startup path
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: cjones, Assigned: cjones)
References
Details
Attachments
(1 file, 1 obsolete file)
7.46 KB,
patch
|
fabrice
:
review+
lsblakk
:
approval-mozilla-b2g18+
|
Details | Diff | Splinter Review |
forms.js and UserAgentOverrides.jsm overhead was significant on startup.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #707338 -
Flags: review?(fabrice)
Comment 2•11 years ago
|
||
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+
Assignee | ||
Comment 3•11 years ago
|
||
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.
Comment 4•11 years ago
|
||
(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?
Assignee | ||
Comment 5•11 years ago
|
||
(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.
Assignee | ||
Comment 6•11 years ago
|
||
(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.
Assignee | ||
Comment 7•11 years ago
|
||
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().
Assignee | ||
Comment 8•11 years ago
|
||
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 :|.
Assignee | ||
Comment 9•11 years ago
|
||
Any suggestion for how we can do this without resorting to something "unclean" like a feature-pref for overrides?
Flags: needinfo?(21)
Assignee | ||
Comment 10•11 years ago
|
||
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)
Assignee | ||
Comment 11•11 years ago
|
||
Feedback ping. This is a very nice measurable win.
Assignee | ||
Comment 12•11 years ago
|
||
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?
Comment 13•11 years ago
|
||
I would be very happy to see UAO_child.js disappear.
Flags: needinfo?(fabrice)
Assignee | ||
Comment 14•11 years ago
|
||
Attachment #707338 -
Attachment is obsolete: true
Attachment #710432 -
Flags: review?(fabrice)
Flags: needinfo?(21)
Comment 15•11 years ago
|
||
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+
Assignee | ||
Comment 16•11 years ago
|
||
Pushed with more verbose comment https://hg.mozilla.org/integration/mozilla-inbound/rev/2716ae058067
Comment 17•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2716ae058067
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Assignee | ||
Comment 18•11 years ago
|
||
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?
Updated•11 years ago
|
Comment 19•11 years ago
|
||
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+
Assignee | ||
Comment 20•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18/rev/e4ffa0660d88 https://hg.mozilla.org/releases/mozilla-b2g18/rev/d3401943481d
Assignee | ||
Updated•11 years ago
|
Updated•11 years ago
|
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.
Description
•