Changeset 238525 in webkit

Timestamp:

Nov 26, 2018 2:54:48 PM (5 years ago)

Author:

Chris Dumez

Message:

[PSON] process pre-warming should not be on for everyone
​https://bugs.webkit.org/show_bug.cgi?id=191966
<rdar://problem/46138499>

Reviewed by Ryosuke Niwa.

If automatic process-prewarming is not explicitly turned on/off by the client, then we now only
turn it on automatically after the first process swap. Previously, it was always enabled if
process-swap on cross-site navigation was enabled (which is the case by default for all apps).

This is important because some apps do not actually browse (only display static content) or never
browse cross-site, and thus would not benefit from process prewarming and yet pay a memory cost.

• UIProcess/API/APIProcessPoolConfiguration.h:

Add m_clientWouldBenefitFromAutomaticProcessPrewarming flag which is false by default and is used
as a fallback if the client did not call setIsAutomaticProcessWarmingEnabled().

• UIProcess/API/C/WKContextConfigurationRef.cpp:

(WKContextConfigurationPrewarmsProcessesAutomatically):
(WKContextConfigurationSetPrewarmsProcessesAutomatically):
Add C API to toggle automatic process prewarming. It is needed for Safari to turn on this feature
by default for browsing. This way Safari does not have to wait for the first process swap to
start prewarming.

• UIProcess/API/C/WKContextConfigurationRef.h:
• UIProcess/WebProcessPool.cpp:

(WebKit::WebProcessPool::didReachGoodTimeToPrewarm):
Only do automatic process prewarming if process-swap on cross-site navigation is also enabled.
This is needed for clients like Safari that explicitly enable automatic process prewarming but
only want to have it enabled if PSON is also enabled via experimental features.

(WebKit::WebProcessPool::processForNavigation):
On first process swap, if the client did not explicitly turn on or off automatic process prewarming
then we enable it then. This avoids paying the cost of process prewarming in apps that would not
benefit from it.

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/API/APIProcessPoolConfiguration.h (modified) (2 diffs)
• UIProcess/API/C/WKContextConfigurationRef.cpp (modified) (1 diff)
• UIProcess/API/C/WKContextConfigurationRef.h (modified) (1 diff)
• UIProcess/WebProcessPool.cpp (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2018-11-26  Chris Dumez  <cdumez@apple.com>
+
+        [PSON] process pre-warming should not be on for everyone
+        https://bugs.webkit.org/show_bug.cgi?id=191966
+        <rdar://problem/46138499>
+
+        Reviewed by Ryosuke Niwa.
+
+        If automatic process-prewarming is not explicitly turned on/off by the client, then we now only
+        turn it on automatically after the first process swap. Previously, it was always enabled if
+        process-swap on cross-site navigation was enabled (which is the case by default for all apps).
+
+        This is important because some apps do not actually browse (only display static content) or never
+        browse cross-site, and thus would not benefit from process prewarming and yet pay a memory cost.
+
+        * UIProcess/API/APIProcessPoolConfiguration.h:
+        Add m_clientWouldBenefitFromAutomaticProcessPrewarming flag which is false by default and is used
+        as a fallback if the client did not call setIsAutomaticProcessWarmingEnabled().
+
+        * UIProcess/API/C/WKContextConfigurationRef.cpp:
+        (WKContextConfigurationPrewarmsProcessesAutomatically):
+        (WKContextConfigurationSetPrewarmsProcessesAutomatically):
+        Add C API to toggle automatic process prewarming. It is needed for Safari to turn on this feature
+        by default for browsing. This way Safari does not have to wait for the first process swap to
+        start prewarming.
+
+        * UIProcess/API/C/WKContextConfigurationRef.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::didReachGoodTimeToPrewarm):
+        Only do automatic process prewarming if process-swap on cross-site navigation is also enabled.
+        This is needed for clients like Safari that explicitly enable automatic process prewarming but
+        only want to have it enabled if PSON is also enabled via experimental features.
+
+        (WebKit::WebProcessPool::processForNavigation):
+        On first process swap, if the client did not explicitly turn on or off automatic process prewarming
+        then we enable it then. This avoids paying the cost of process prewarming in apps that would not
+        benefit from it.
+
 2018-11-26  Andy Estes  <aestes@apple.com>
 

trunk/Source/WebKit/UIProcess/API/APIProcessPoolConfiguration.h

@@ -62 +62 @@
     bool isAutomaticProcessWarmingEnabled() const
     {
-        // FIXME: For now, turning on PSON from the experimental features menu also turns on
-        // automatic process warming until clients can be updated.
-        return m_isAutomaticProcessWarmingEnabledByClient.value_or(m_processSwapsOnNavigationFromExperimentalFeatures);
+        return m_isAutomaticProcessWarmingEnabledByClient.value_or(m_clientWouldBenefitFromAutomaticProcessPrewarming);
     }
+
+    bool wasAutomaticProcessWarmingSetByClient() const { return !!m_isAutomaticProcessWarmingEnabledByClient; }
     void setIsAutomaticProcessWarmingEnabled(bool value) { m_isAutomaticProcessWarmingEnabledByClient = value; }
+
+    bool clientWouldBenefitFromAutomaticProcessPrewarming() const { return m_clientWouldBenefitFromAutomaticProcessPrewarming; }
+    void setClientWouldBenefitFromAutomaticProcessPrewarming(bool value) { m_clientWouldBenefitFromAutomaticProcessPrewarming = value; }
 
     bool diskCacheSpeculativeValidationEnabled() const { return m_diskCacheSpeculativeValidationEnabled; }
@@ -223 +226 @@
     bool m_processSwapsOnWindowOpenWithOpener { false };
     std::optional<bool> m_isAutomaticProcessWarmingEnabledByClient;
+    bool m_clientWouldBenefitFromAutomaticProcessPrewarming { false };
     WTF::String m_customWebContentServiceBundleIdentifier;
     bool m_isJITEnabled { true };

trunk/Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.cpp

@@ -169 +169 @@
 }
 
+bool WKContextConfigurationPrewarmsProcessesAutomatically(WKContextConfigurationRef configuration)
+{
+    return toImpl(configuration)->isAutomaticProcessWarmingEnabled();
+}
+
+void WKContextConfigurationSetPrewarmsProcessesAutomatically(WKContextConfigurationRef configuration, bool prewarms)
+{
+    toImpl(configuration)->setIsAutomaticProcessWarmingEnabled(prewarms);
+}
+
 bool WKContextConfigurationAlwaysKeepAndReuseSwappedProcesses(WKContextConfigurationRef configuration)
 {

trunk/Source/WebKit/UIProcess/API/C/WKContextConfigurationRef.h

@@ -72 +72 @@
 WK_EXPORT void WKContextConfigurationSetProcessSwapsOnNavigation(WKContextConfigurationRef configuration, bool swaps);
 
+WK_EXPORT bool WKContextConfigurationPrewarmsProcessesAutomatically(WKContextConfigurationRef configuration);
+WK_EXPORT void WKContextConfigurationSetPrewarmsProcessesAutomatically(WKContextConfigurationRef configuration, bool prewarms);
+
 WK_EXPORT bool WKContextConfigurationAlwaysKeepAndReuseSwappedProcesses(WKContextConfigurationRef configuration);
 WK_EXPORT void WKContextConfigurationSetAlwaysKeepAndReuseSwappedProcesses(WKContextConfigurationRef configuration, bool keepAndReuse);

trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

@@ -1286 +1286 @@
 void WebProcessPool::didReachGoodTimeToPrewarm()
 {
-    if (!configuration().isAutomaticProcessWarmingEnabled())
+    if (!configuration().isAutomaticProcessWarmingEnabled() || !configuration().processSwapsOnNavigation())
         return;
 
@@ -2114 +2114 @@
     auto process = processForNavigationInternal(page, navigation, processSwapRequestedByClient, reason);
 
+    // We are process-swapping so automatic process prewarming would be beneficial if the client has not explicitly enabled / disabled it.
+    bool doingAnAutomaticProcessSwap = processSwapRequestedByClient == ProcessSwapRequestedByClient::No && process.ptr() != &page.process();
+    if (doingAnAutomaticProcessSwap && !configuration().wasAutomaticProcessWarmingSetByClient() && !configuration().clientWouldBenefitFromAutomaticProcessPrewarming()) {
+        RELEASE_LOG(PerformanceLogging, "Automatically turning on process prewarming because the client would benefit from it");
+        configuration().setClientWouldBenefitFromAutomaticProcessPrewarming(true);
+    }
+
     if (m_configuration->alwaysKeepAndReuseSwappedProcesses() && process.ptr() != &page.process()) {
         static std::once_flag onceFlag;