browser/base/content/test/performance/browser_startup.js indicates that the following scripts are loaded before opening the first window: resource://shield-recipe-client/lib/LogManager.jsm resource://shield-recipe-client/lib/ShieldRecipeClient.jsm resource://shield-recipe-client/lib/CleanupManager.jsm

Three questions: 1. What's an acceptable timeline for addressing this? Is waiting a few weeks okay? Or maybe tie it to a version (57?)? 2. Are there guidelines on what is allowed to be loaded before opening the first window? In particular, I think Shield will want to set the default values for it's own preferences, and set the values for in-progress preference experiments, before the first window is opened, since those experiments may have an effect on the first window's behavior. 3. Is there an event or something we can listen to to know when it's safe to start initializing our stuff?

(In reply to Michael Kelly [:mkelly,:Osmose] from comment #1) > Three questions: > > 1. What's an acceptable timeline for addressing this? Is waiting a few weeks > okay? Or maybe tie it to a version (57?)? I'm not sure. Our ultimate short term goals are focused on 57 and AFAIK this isn't blocking anything so I think waiting a few weeks is fine. > 2. Are there guidelines on what is allowed to be loaded before opening the > first window? As little code as possible. :-) More specifically, only code that is needed to open and display the first window needs to run before we open and display the first window as a matter of principle. > In particular, I think Shield will want to set the default > values for it's own preferences That needs to be done in all.js/firefox.js not programmatically. >, and set the values for in-progress > preference experiments, before the first window is opened, since those > experiments may have an effect on the first window's behavior. Shield needs to set the pref at runtime when it detects the change in the pref and rely on the prefs backend to persist the pref and make it available at startup. It should certainly not roll up its own prefs infrastructure on top of the prefs service. :-) The prefs service is capable of doing all of these things, you should just tell it what to do and it will do it for you. For example if you want a pref to be available at startup next time you restart, it's enough to just set it normally, no extra code needs to run at startup. > 3. Is there an event or something we can listen to to know when it's safe to > start initializing our stuff? It depends on what you want to initialize, sorry I'm not familiar with the shield code so I can't answer this in a useful way. :-)

(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog) from comment #2) > I'm not sure. Our ultimate short term goals are focused on 57 and AFAIK > this isn't blocking anything so I think waiting a few weeks is fine. Rad, thanks. > That needs to be done in all.js/firefox.js not programmatically. Even if Shield is a system add-on? This would mean we can't set defaults for new preferences we add without uplifting just the pref defaults to release. > Shield needs to set the pref at runtime when it detects the change in the > pref and rely on the prefs backend to persist the pref and make it available > at startup. It should certainly not roll up its own prefs infrastructure on > top of the prefs service. :-) The prefs service is capable of doing all of > these things, you should just tell it what to do and it will do it for you. > For example if you want a pref to be available at startup next time you > restart, it's enough to just set it normally, no extra code needs to run at > startup. Preference experiments generally alter the default value of the pref instead of the user-set value after bsmedberg advised that user-set pref values are notoriously difficult to rollback. And runtime changes to the default pref values don't persist between sessions, so we have to set them to the experiment value at startup. The alternative would be to mandate that preferences that need to be set before the first window paint _must_ use user-set preference experiments. I'm not sure what the tradeoffs of that are, and how many preference experiments it would affect. I'll defer to bsmedberg as to what's preferable. :D > It depends on what you want to initialize, sorry I'm not familiar with the > shield code so I can't answer this in a useful way. :-) 302 Gijs: Any tips on how to detect when it's safe for Shield to start doing stuff once the first window paint happens?

(In reply to Michael Kelly [:mkelly,:Osmose] from comment #1) > 1. What's an acceptable timeline for addressing this? Is waiting a few weeks > okay? Or maybe tie it to a version (57?)? Waiting a few weeks is okay for stuff that requires a significant effort to fix. If we are talking about something trivial like making a few module imports lazy, we shouldn't wait, as each bug we keep increases the chance we'll miss something in profiles while attempting to ignore already known bugs. > 2. Are there guidelines on what is allowed to be loaded before opening the > first window? The general guideline is that code that doesn't change what's displayed at first paint shouldn't run before first paint. > In particular, I think Shield will want to set the default > values for it's own preferences, and set the values for in-progress > preference experiments, before the first window is opened, since those > experiments may have an effect on the first window's behavior. Setting a few default preference values from the bootstrap.js file that gets loaded by the add-on manager to start your system add-on is fine. Doing it with plenty of overhead (like loading several JS modules) is not. > 3. Is there an event or something we can listen to to know when it's safe to > start initializing our stuff? The answer will be different depending on whether your stuff needs to be initialized before the first window is created, before the first window is painted, after first paint but before the user has a change to cause any event, or soon after startup without getting in the user's way. (In reply to Michael Kelly [:mkelly,:Osmose] from comment #3) > > It depends on what you want to initialize, sorry I'm not familiar with the > > shield code so I can't answer this in a useful way. :-) > > 302 Gijs: Any tips on how to detect when it's safe for Shield to start doing > stuff once the first window paint happens? You'll need to be a lot more specific about what "doing stuff" means before we can give a useful answer to this question.

(In reply to Florian Quèze [:florian] [:flo] from comment #4) > Setting a few default preference values from the bootstrap.js file that gets > loaded by the add-on manager to start your system add-on is fine. Doing it > with plenty of overhead (like loading several JS modules) is not. To set the default preference values for preference experiments, we'd need to load one JS module and read a JSON file stored in the profile directory that stores what experiments are active. I imagine the file read might be a problem; it's possible we could move that data to somewhere else, like IndexedDB, if that's preferable. > The answer will be different depending on whether your stuff needs to be > initialized before the first window is created, before the first window is > painted, after first paint but before the user has a change to cause any > event, or soon after startup without getting in the user's way. I think we have two groups of initialization tasks: 1. Setting default preference values for active experiments, which is ideally done as early as possible to reduce the amount of non-default preference experiments we have to run. I think this means it falls into the "before the first window is created" bucket? 2. Setting up our logger, setting our own preference defaults and observers, and then checking to see if we should fetch recipes immediately or if we should set a timer to wait for a bit before fetching recipes. This can fall into the "soon after startup without getting in the user's way" bucket; my only concern would be QA testers for Shield, who rely on recipes being fetched very soon (under 5-10 seconds) in order to test efficiently. If this happens quickly, or if there was a way for us to pref-on running immediately, that would help a lot.

I'm not aware of a better way of doing the default preferences stuff than what SHIELD is already doing, as Michael has already explained (ie we need to set them at runtime with the prefs service). We could potentially decouple all of that (as concerning extant recipes) from the rest of SHIELD, but that would be a significant amount of effort and, to some extent, would run counter to desires for modularity and code organization (where you'd want all the pref setting code to live in the same jsm). As a strawman, we could make the pref setting recipe code, for each preference name/value pair (foo.bar : baz), *also* set ( extensions.shield.foo.bar : baz ) as a user pref, and on startup, do nothing initially except set the default prefs based off of the user prefs (maybe straight from bootstrap.js) and then delay everything else until the first window has opened.

(In reply to Michael Kelly [:mkelly,:Osmose] from comment #5) > (In reply to Florian Quèze [:florian] [:flo] from comment #4) > > Setting a few default preference values from the bootstrap.js file that gets > > loaded by the add-on manager to start your system add-on is fine. Doing it > > with plenty of overhead (like loading several JS modules) is not. > > To set the default preference values for preference experiments, we'd need > to load one JS module and read a JSON file stored in the profile directory > that stores what experiments are active. I imagine the file read might be a > problem; it's possible we could move that data to somewhere else, like > IndexedDB, if that's preferable. I don't think indexeddb would be better. If you have few enough preferences that they can all fit in a small piece of JSON, you could store that JSON in a user set preference. (In reply to :Gijs from comment #6) > on startup, do nothing > initially except set the default prefs based off of the user prefs (maybe > straight from bootstrap.js) and then delay everything else until the first > window has opened. Seems like a good plan to me :-).

Be aware that there really needs to be logic running here: for shield pref rollouts and pref experiments, we need to validate that the recipe still applies before applying the prefs: that way e.g. if the user upgrades and the pref rollout is no longer relevant, we don't set incorrect prefs. I'm not sure whether there are cases where we could short-circuit this, such as if we're confident that the buildid hasn't changed. That might help the common case.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #8) > Be aware that there really needs to be logic running here: for shield pref > rollouts and pref experiments, we need to validate that the recipe still > applies before applying the prefs: that way e.g. if the user upgrades and > the pref rollout is no longer relevant, we don't set incorrect prefs. > > I'm not sure whether there are cases where we could short-circuit this, such > as if we're confident that the buildid hasn't changed. That might help the > common case. Currently the only thing that blocks our initial setting of the default pref values is if the pref value changed while Firefox was closed, otherwise we just set them on startup and defer checking if the recipe still applies until we hit the service roughly 5 minutes after startup. Changing that behavior is probably a separate bug, although it has implications for this one. (In reply to :Gijs from comment #6) > As a strawman, we could make the pref setting recipe code, for each > preference name/value pair (foo.bar : baz), *also* set ( > extensions.shield.foo.bar : baz ) as a user pref, and on startup, do nothing > initially except set the default prefs based off of the user prefs (maybe > straight from bootstrap.js) and then delay everything else until the first > window has opened. I like this. We'd probably store JSON rather than just the value so that we can store more data if we need to support the extra startup logic bsmedberg mentioned, but otherwise this sounds reasonable to me.

> until we hit the service roughly 5 minutes after startup. Changing that > behavior is probably a separate bug, although it has implications for this > one. Based on our email conversations, I believe we decided that the current behavior is ok for pref experiments, but is *not* ok for pref rollouts and we need the more strict checking for those.

I've got a WIP patch that implements Gijs' suggestion. The one remaining question I have is when to trigger the rest of our delayed initialization. florian: rhelmer suggested we wait for the "sessionstore-windows-restored" event and finish initialization then. Does that sound like a good time to finish loading our other JSMs and initialization work?

(In reply to Michael Kelly [:mkelly,:Osmose] from comment #11) > I've got a WIP patch that implements Gijs' suggestion. The one remaining > question I have is when to trigger the rest of our delayed initialization. > > florian: rhelmer suggested we wait for the "sessionstore-windows-restored" > event and finish initialization then. Does that sound like a good time to > finish loading our other JSMs and initialization work? Yes, if you want to be in the "soon after startup without getting in the user's way" bucket, you want to observe the "sessionstore-windows-restored" notification, and then use requestIdleCallback (or Services.tm.idleDispatchToMainThread) from there.

Here's the diff of my branch that implements the fix discussed above: https://github.com/mozilla/normandy/compare/master...Osmose:startup-performance While working on bug 1390325, I pushed a try build with the fix to see if it had any effect: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4aa444b45bdeb67082b6c0bc76442e59b257f982 Here's where it gets weird: comparing that push to a baseline push without the patch applied showed no difference in the sudden 9% fileio from bug 1390325, and also shows a 29% _increase_ in main thread fileio. https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=34da49f1ec8c2ee5773462fd8f67d3c5bec801ba&newProject=try&newRevision=4aa444b45bdeb67082b6c0bc76442e59b257f982&framework=1 As far as I understand, the only new IO I'm adding is to the preferences, and I'd expect that to not immediately result in file IO events. jmaher: Do you happen to know: - When does "startup" end for the xperf tests? Would it occur after sessionstore-windows-restored? - What's are "normal" xperf tests vs startup?

we have a few xperf probes inside of firefox: http://searchfox.org/mozilla-central/source/toolkit/components/startup/mozprofilerprobe.mof we measure startup until we have reached MozillaEventSessionStoreWindowRestored: http://searchfox.org/mozilla-central/source/toolkit/components/startup/nsAppStartup.cpp#81 this seems to occur in the code here: http://searchfox.org/mozilla-central/source/toolkit/components/startup/nsAppStartup.cpp#201 so all data we collect that has *_startup_* happens prior to the sessionstore event, and the *_normal_* data is after the sessionstore event.

PR for this is up: https://github.com/mozilla/normandy/pull/1024

Commits pushed to master at https://github.com/mozilla/normandy https://github.com/mozilla/normandy/commit/a462febeccc48009cfbeac9650650b100a6863cd Fix bug 1371350: Delay almost all initialization until after first paint. https://github.com/mozilla/normandy/commit/e96eea7e254e32998d62b4dc45bf249fa3620a4d Merge pull request #1024 from Osmose/startup-performance Bug 1371350: Delay almost all startup tasks until after sessionstore-windows-restored