XPIProvider.jsm is a rather big file. So big that simply *loading* it is the most time-consuming operation during Add-ons Manager startup (assuming checkForChanges doesn't get called). XPIDatabase is a bug chunk of that file - and usually it's not needed on startup. So a good first step in trimming down XPIProvider.jsm is to lazy load XPIDatabase. Other good candidates for the future are: * RDF functions * AddonInstall / AddonInstallWrapper * UpdateChecker

I had numbers for different parts of cold/warm startup on Android (Galaxy Nexus), before and after applying this patch and bug 746909... which I put somewhere safe, and now can't find. Will attach when I find them, but the punchline was a ~30% startup time improvement for the Add-ons Manager. Bug 696141 has initial measurements before any changes.

The import code seems verbose, but it'll make it easy to lazy-load other objects too.

Forgot to mention: there *are* changes to XPIDatabase itself, including but not limited to dbFileExists, removing usage of Prefs object, and access to DBAddonInternal.

One more thing: As an added bonus, this also removes a potentially unnecessary DB transaction on startup (see bug 729330), where a transaction was started before it knew it was going to need one. There's a couple of other cases where that happens too, which will be easy wins.

(In reply to Blair McBride (:Unfocused) from comment #3) > Forgot to mention: there *are* changes to XPIDatabase itself, including but > not limited to dbFileExists, removing usage of Prefs object, and access to > DBAddonInternal. Mm, any chance you could split these changes into separate patches?

I wonder. XPIDatabase should only be used by one thing, XPIProvider.jsm so it seems that there isn't a need to have it in a shared JS module. If instead it just got loaded by a plain subscript loader when first needed then it'd get access to the full scope of XPIProvider.jsm possibly making some of this easier. Thoughts?

Comment on attachment 616499 [details] [diff] [review] Patch v1 Waiting on a response to comment 6

(In reply to Dave Townsend (:Mossop) from comment #6) > I wonder. XPIDatabase should only be used by one thing, XPIProvider.jsm so > it seems that there isn't a need to have it in a shared JS module. If > instead it just got loaded by a plain subscript loader when first needed > then it'd get access to the full scope of XPIProvider.jsm possibly making > some of this easier. Thoughts? I considered that, but it's nice to have more control. I also want to move a bunch of other stuff out of XPIProvider.jsm too - that includes a lot of the utility functions that would only be used by the code that's factored out of XPIProvider.jsm. So a JSM is a nice way to control what goes into the global scope. Also means we end up being a lot more deliberate over what we move out.

This is only the changes made to XPIDatabase to allow it to be lazy-loaded, so that Part 2 only moves that code without making any additional changes. Now with less bitrot.

Comment on attachment 628307 [details] [diff] [review] Part 2, v1.1 Bah, ran the tests but somehow didn't see that there were a couple of failures. Am investigating.

Comment on attachment 628306 [details] [diff] [review] Part 1, v1.1 Review of attachment 628306 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/mozapps/extensions/XPIProvider.jsm @@ +5693,5 @@ > // The selected skin may come from an inactive theme (the default theme > // when a lightweight theme is applied for example) > text += "\r\n[ThemeDirs]\r\n"; > > + let ddsEnabled = false; dssEnabled please.

(In reply to Blair McBride (:Unfocused) from comment #11) > Bah, ran the tests but somehow didn't see that there were a couple of > failures. Am investigating. AFAICT, that was compartment-per-global's fault. Switched to using subscript loader, but still preserving separation of scopes. Also means less compartment/wrapper overhead, but then we don't get memory reporting for just the lazy-loaded code.

s/ddsEnabled/dssEnabled/

Comment on attachment 629049 [details] [diff] [review] Part 2, v2 Review of attachment 629049 [details] [diff] [review]: ----------------------------------------------------------------- I'm trying (and failing) to get diff to tell me whether the stuff in XPIProviderUtils.js is just straight moves at this point. If so this is an r+, otherwise is there an easy way to see what code changes happened?

Yea, any non-obvious changes should be in the first patch.

https://hg.mozilla.org/integration/fx-team/rev/6475b0ff65c1 https://hg.mozilla.org/integration/fx-team/rev/83f101d3c372

https://hg.mozilla.org/mozilla-central/rev/6475b0ff65c1 https://hg.mozilla.org/mozilla-central/rev/83f101d3c372