Bug 191966 – [PSON] process pre-warming should not be on for everyone

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

Process pre-warming should not be on for everyone as some clients would not benefit from it and it would be an unnecessary memory cost.

[Chris Dumez]

<rdar://problem/46138499>

[Alex Christensen]

How would we be able to tell which clients would benefit from it?

[Chris Dumez]

(In reply to Alex Christensen from comment #2)
> How would we be able to tell which clients would benefit from it?

Detailed on the radar but basically, we turn on process prewarming after the first process swap.

[Chris Dumez]

Created attachment 355671 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 355671 [details]
Patch

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

> Source/WebKit/UIProcess/WebProcessPool.cpp:1288
> -    if (!configuration().isAutomaticProcessWarmingEnabled())
> +    if (!configuration().isAutomaticProcessWarmingEnabled() || !configuration().processSwapsOnNavigation())

But aren't there clients which would benefit from pre-warming without enabling PSON?
e.g. for opening multiple WKWebViews?

[Chris Dumez]

(In reply to Ryosuke Niwa from comment #5)
> Comment on attachment 355671 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=355671&action=review
> 
> > Source/WebKit/UIProcess/WebProcessPool.cpp:1288
> > -    if (!configuration().isAutomaticProcessWarmingEnabled())
> > +    if (!configuration().isAutomaticProcessWarmingEnabled() || !configuration().processSwapsOnNavigation())
> 
> But aren't there clients which would benefit from pre-warming without
> enabling PSON?
> e.g. for opening multiple WKWebViews?

Automatic process prewarming is not currently API, which is why I am adding the heuristic to turn it on automatically when we determine it would be beneficial.

Process-Swap on Navigation is not not currently API and *is* enabled by default so clients do get PSON no matter what. They will also get process prewarming if our heuristic determines it would be beneficial.

As the change log explains, the reason I made this particular change is because Safari will turn automatic process prewarming on explicitly and rely on the experimental feature flag for PSON support. If PSON is disabled in Safari via experimental features, I think we'd also want to disable automatic process prewarming.

[Chris Dumez]

(In reply to Chris Dumez from comment #6)
> (In reply to Ryosuke Niwa from comment #5)
> > Comment on attachment 355671 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=355671&action=review
> > 
> > > Source/WebKit/UIProcess/WebProcessPool.cpp:1288
> > > -    if (!configuration().isAutomaticProcessWarmingEnabled())
> > > +    if (!configuration().isAutomaticProcessWarmingEnabled() || !configuration().processSwapsOnNavigation())
> > 
> > But aren't there clients which would benefit from pre-warming without
> > enabling PSON?
> > e.g. for opening multiple WKWebViews?
> 
> Automatic process prewarming is not currently API, which is why I am adding
> the heuristic to turn it on automatically when we determine it would be
> beneficial.
> 
> Process-Swap on Navigation is not not currently API and *is* enabled by
> default so clients do get PSON no matter what. They will also get process
> prewarming if our heuristic determines it would be beneficial.
> 
> As the change log explains, the reason I made this particular change is
> because Safari will turn automatic process prewarming on explicitly and rely
> on the experimental feature flag for PSON support. If PSON is disabled in
> Safari via experimental features, I think we'd also want to disable
> automatic process prewarming.

Note that the reason we support disabling PSON via experimental features is to easily identify if bugs are caused by PSON or not. I think we should include process-prewarming bugs in the same category, thus my proposal to disable process prewarming if PSON is disabled in experimental features.

[Ryosuke Niwa]

Comment on attachment 355671 [details]
Patch

Okay. Sounds sane to me.

[WebKit Commit Bot]

Comment on attachment 355671 [details]
Patch

Clearing flags on attachment: 355671

Committed r238525: <https://trac.webkit.org/changeset/238525>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.