Currently the Prelaunch app launches 1000 msec after launching an app. If the app being launched takes longer than 1 second to load, then the prelaunch app slows it down. I think that the ideal time to launch would be when the launched app goes idle for the first time. With the prelaunch delayMs set to 1000, the settings app takes about 3.3 seconds from the Launching as OOP message until the end of the apps/js/settings.js showBody method (measured on otoro). With the prelaunch delayMs set to 5000, the settings app only takes about 2.8 seconds.

Adding parts 1 and 3 from bug 780692 drops this to around 2.6 seconds (3 runs), although I had run which was only 2.4 seconds.

Defers the very first prelaunch in the parent until the parent goes idle. Defers subsequent prelaunches until the child process goes idle.

Comment on attachment 668688 [details] [diff] [review] Fix message loop so that idle PostIdleTask works in content children Review of attachment 668688 [details] [diff] [review]: ----------------------------------------------------------------- ::: ipc/chromium/src/base/message_loop.cc @@ +31,5 @@ > #include "base/message_pump_android.h" > #endif > > #include "MessagePump.h" > +#include "nsThreadUtils.h" Bah - I missed this earlier. Will remove.

I also left some very low frequency prints (one per process launch), as they will be useful for determining when the idle actually happens (it isn't always obvious). I have also seen a single case where the idle happened earlier than we wanted, but this was with a debug build. The idle events should also happen earlier on an SMP machine, but we're not as concerned about the startup times in that case.

I also submitted to try since the message loop stuff could impact other platforms. https://tbpl.mozilla.org/?tree=Try&rev=683b57aaa6b6

Comment on attachment 668688 [details] [diff] [review] Fix message loop so that idle PostIdleTask works in content children >diff --git a/ipc/chromium/src/base/message_loop.cc b/ipc/chromium/src/base/message_loop.cc > #include "MessagePump.h" >+#include "nsThreadUtils.h" Looks unused. Gross but no worse than any of the rest of this code ;).

Comment on attachment 668689 [details] [diff] [review] Defer prelaunch >diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp >+static void ChildFirstIdle(void) >+{ >+ printf_stderr("***** ChildFirstIdle *****\n"); >+ ContentChild *child = ContentChild::GetSingleton(); >+ if (!child) { >+ return; >+ } This can never fail, no need to null check. > PBrowserChild* > ContentChild::AllocPBrowser(const uint32_t& aChromeFlags, > const bool& aIsBrowserElement, const AppId& aApp) > { >+ MessageLoop::current()->PostIdleTask(FROM_HERE, NewRunnableFunction(ChildFirstIdle)); >+ AllocPBrowser() is called for every "tab" we create in the process. So this code will run for each tab, and it's not really a "first-idle" notification. That's mostly OK and we'll get away with it in the current setup, but we should maintain semantics. Just adding a |static bool sentFirstIdle = false;| variable to here or something is fine by me. >diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl >+ // Tell the parent that the child has gone idle for the first time >+ async ChildFirstIdle(); >+ The "Child" is implicit from the direction of the message, child -> parent, so we can drop that. Otherwise looks great! The logging is a little gross, so I'd prefer we removed that. Would like to see the next version. Really excited to get this big win in :).

Removed log statements, other minor cleanup

Comment on attachment 668934 [details] [diff] [review] Defer prelaunch v2 \o/

Pushed to inbound: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=2c831793f1a6

https://hg.mozilla.org/mozilla-central/rev/858a75189c01 https://hg.mozilla.org/mozilla-central/rev/2c831793f1a6

Let's let these patches bake for a bit and request aurora approval.

These are very safe patches that are required to meet app startup targets for v1. There's zero risk for other products.

Comment on attachment 668933 [details] [diff] [review] Fix message loop fo that PostIdleTask works in content children v2 [Approval Request Comment] Bug caused by (feature/regressing bug #): 797239 User impact if declined: 0.5 sec slower launch time for apps Testing completed (on m-c, etc.): on m-c Risk to taking this patch (and alternatives if risky): very safe String or UUID changes made by this patch: None

Comment on attachment 668934 [details] [diff] [review] Defer prelaunch v2 [Approval Request Comment] Bug caused by (feature/regressing bug #): 797239 User impact if declined: 0.5 sec slower launch time for apps Testing completed (on m-c, etc.): on m-c Risk to taking this patch (and alternatives if risky): very safe String or UUID changes made by this patch: None

Sorry, I see that if the bug is marked blocking-basecamp then I don't need to request approval.

Comment on attachment 668933 [details] [diff] [review] Fix message loop fo that PostIdleTask works in content children v2 Review of attachment 668933 [details] [diff] [review]: ----------------------------------------------------------------- ::: ipc/chromium/src/base/message_loop.cc @@ +336,5 @@ > nestable_tasks_allowed_ = true; > } > > bool MessageLoop::DeferOrRunPendingTask(const PendingTask& pending_task) { > + if (pending_task.nestable || state_->run_depth >= run_depth_base_) { I'm looking at this again, and I think that the >= should be <=

https://hg.mozilla.org/releases/mozilla-aurora/rev/85aa1352aba7 https://hg.mozilla.org/releases/mozilla-aurora/rev/90a5685dc1be