Summary: | [PSON] process pre-warming should not be on for everyone | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||
Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | achristensen, beidson, commit-queue, ggaren, koivisto, rniwa | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Chris Dumez
2018-11-26 09:07:24 PST
How would we be able to tell which clients would benefit from it? (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. Created attachment 355671 [details]
Patch
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? (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. (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. Comment on attachment 355671 [details]
Patch
Okay. Sounds sane to me.
Comment on attachment 355671 [details] Patch Clearing flags on attachment: 355671 Committed r238525: <https://trac.webkit.org/changeset/238525> All reviewed patches have been landed. Closing bug. |