Bug 196208

Summary: [GTK][WPE] Disable process warming
Product: WebKit Reporter: Patrick Griffis <pgriffis>
Component: WebKitGTKAssignee: Philippe Normand <pnormand>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cdumez, commit-queue, mcatanzaro, pnormand
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=196104
https://bugs.webkit.org/show_bug.cgi?id=198526
Attachments:
Description Flags
Patch
none
Patch none

Description Patrick Griffis 2019-03-25 11:20:29 PDT
[GTK][WPE] Disable process warming
Comment 1 Patrick Griffis 2019-03-25 11:23:11 PDT
Created attachment 365877 [details]
Patch
Comment 2 Michael Catanzaro 2019-03-25 11:40:46 PDT
Comment on attachment 365877 [details]
Patch

Process warming should already be disabled because PSON is disabled. Is there a bug in the logic for that?

This is OK as an emergency measure, but the comment needs a FIXME, because this is very bad. Process warming is not optional for us any more than bubblewrap sandboxing is; we have to make them both work together. Without process warming, PSON will be a severe performance penalty. And we have to make PSON work for Spectre mitigation.

I don't think prewarming is fundamentally incompatible with the bubblewrap sandbox. Prewarmed processes should just use the same WebsiteDataStore as their corresponding web view. If the application creates a view with a different WebsiteDataStore, then of course the prewarming will be defeated and a new process will need to be spun up, but that's an unusual case.
Comment 3 Michael Catanzaro 2019-03-25 11:47:05 PDT
(In reply to Michael Catanzaro from comment #2)
> Prewarmed processes should just use the same WebsiteDataStore as
> their corresponding web view. If the application creates a view with a
> different WebsiteDataStore, then of course the prewarming will be defeated
> and a new process will need to be spun up, but that's an unusual case.

And if this logic has to be platform-specific because it's not needed by Apple, then so be it. We've accepted the constraint that all data directories must be known before launching the process, and it's not practical to revisit that, so the inescapable conclusion is WebsiteDataStore has to be ready when the process is launched.
Comment 4 Patrick Griffis 2019-03-25 11:51:18 PDT
(In reply to Michael Catanzaro from comment #2)
> Comment on attachment 365877 [details]
> Patch
> 
> Process warming should already be disabled because PSON is disabled. Is
> there a bug in the logic for that?

I could not reproduce any issue without forcing PSON on, so I think its fine.

Agreed about this being temporary though.
Comment 5 Chris Dumez 2019-03-25 12:17:01 PDT
Comment on attachment 365877 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=365877&action=review

r=me

> Source/WebKit/UIProcess/glib/WebProcessPoolGLib.cpp:90
> +    // Process warming is incompatible with the fact our WebProcessProxy::platformGetLaunchOptions()

You may want a FIXME comment as the performance of process-swap-on-navigation is not going to be great without process-prewarming.
Comment 6 Philippe Normand 2019-03-26 02:38:34 PDT
Created attachment 365952 [details]
Patch
Comment 7 WebKit Commit Bot 2019-03-26 03:16:52 PDT
Comment on attachment 365952 [details]
Patch

Clearing flags on attachment: 365952

Committed r243490: <https://trac.webkit.org/changeset/243490>
Comment 8 WebKit Commit Bot 2019-03-26 03:16:54 PDT
All reviewed patches have been landed.  Closing bug.