Closed Bug 1568201 Opened 5 years ago Closed 5 years ago

Try to launch WebExtension process asynchronously

Categories

(WebExtensions :: General, task, P2)

task

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: mconley, Assigned: aswan)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fxperf:p2])

Here's a profile where we block the main thread for something like 90ms in the parent process while we wait for the WebExtension process to init:

https://perfht.ml/2YiODcJ

There's infrastructure that jld added to launch processes asynchronously - we made the Preallocated Process Manager do this, for example, in bug 1446161.

We should look to see what it would take to make launching the WebExtension process asynchronously as well.

Type: enhancement → task
Priority: -- → P2

I'm not comfortable setting the fxperf priority here, Andrew, can you decide for me? Thanks!

Flags: needinfo?(aswan)

In https://perfht.ml/3135Cxp there is a ~240 ms Jank event in the parent process main thread at 7.008 seconds. During that time the browser is starting extension background pages and most of that time is spent blocked deep inside GeckoChildProcessHost::LaunchAndWaitForProcessHandle()

Note that this all happens after sessionstore-windows-restored so the browser UI has been painted and about:home is busy loading in the privileged content process so this isn't directly on the critical path to getting about:home painted, but we shouldn't be janking the parent process like that. I'm going to try to take a stab at this after I clear a few out a few other things.

Assignee: nobody → aswan
Flags: needinfo?(aswan)
Whiteboard: [fxperf] → [fxperf:p2]

(In reply to Andrew Swan [:aswan] from comment #2)

In https://perfht.ml/3135Cxp there is a ~240 ms Jank event in the parent process main thread at 7.008 seconds. During that time the browser is starting extension background pages and most of that time is spent blocked deep inside GeckoChildProcessHost::LaunchAndWaitForProcessHandle()

Is this different from the jank happening whenever we create a content process? That said, it's possible I haven't looked at this closely, a quick read of bug 1446161 seems to indicate this has been significantly improved.

If I understand this all correctly, regular web content processes take a very different code path -- we try to keep a pre-allocated content process available all the time and it was code to create a new one that was causing jank that got fixed in bug 1446161. We only have a single extension process so it doesn't use the PreallocatedProcessManager. The code path it uses blocks the main thread, I'm going to look at how feasible it is to fix that.

From what I can gather, this process is launched synchronously when we insert a browser element here:
https://searchfox.org/mozilla-central/rev/0ffa9e372df56c95547fed9c3433ddec4fbf6f11/toolkit/components/extensions/ExtensionParent.jsm#1382

Before diving too deeply into this, I noticed that the commit message for
https://hg.mozilla.org/mozilla-central/rev/3cbd2cf23c86
mentions: "Project Fission's upcoming rewrite of how the DOM requests new content processes."

Jed, is there work planned for Fission that would make this bug irrelevant? If not, I'm assuming that trying to async-ify all the FrameLoader/ipc stuff that runs from the code linked at the top of this commit is either impractical or impossible. Would it be feasible to add a chrome-only webidl method that we could use to launch the extension process async? The idea would be to wait on that method before inserting the browser to avoid entering a synchronous LaunchAndWaitForProcessHandle(). I realize there are some tricky details to get right, but does the general idea seem feasible?

Flags: needinfo?(jld)

(In reply to Andrew Swan [:aswan] from comment #5)

Before diving too deeply into this, I noticed that the commit message for
https://hg.mozilla.org/mozilla-central/rev/3cbd2cf23c86
mentions: "Project Fission's upcoming rewrite of how the DOM requests new content processes."

Jed, is there work planned for Fission that would make this bug irrelevant?

That comment didn't age well. What I'm currently hoping to accomplish is more like what I removed in that commit than what I added: rework how IPC manages OS resources so that we don't need the process's id/handle just to serialize and enqueue a message. That would remove the distinction between sync and async launch: launching will have a sync API but won't block. But that's not a small project, and it's still in the process of being planned.

An alternative that we'd considered was to use the current promise-based “async launch” for subframes, and rely on more or less the current pre-launching for top-level documents. That's simpler, but it doesn't help anything that's launched at startup, like WebExtensions (or non-content processes like the GPU, of course).

Flags: needinfo?(jld)

You might also want to profile with IPC Launch added to the thread filter. This is on Windows, where the kernel's process creation call takes a nontrivial amount of time, but the last time I looked at this I found we were also spending a lot of time in sandbox policy construction that could be optimized (or at least cached, so that might not help the first child process). I filed bug 1476101 for that, but the profile in that bug was taken in a VM on a fast machine, so data from the official reference hardware would be interesting.

Here's a profile from the reference device https://perfht.ml/2GR9B8C
It looks like the time spent setting up the sandbox is modest compared to the time waiting for NtCreateProcess

Anyway, based on comment 6 I'm going to close this since I don't think it makes sense to try to do something for the extension process if we'll eventually have something more general as part of Fission.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.