- Revision
- 237735
- Author
- [email protected]
- Date
- 2018-11-02 08:59:23 -0700 (Fri, 02 Nov 2018)
Log Message
[PSON] Reuse SuspendedPages' process when possible, for performance
https://bugs.webkit.org/show_bug.cgi?id=191166
Reviewed by Geoffrey Garen.
Source/WebKit:
When process-swapping check if there is an existing SuspendedPage for the domain we're going to.
If there is, use this SuspendedPage's process for the navigation instead of a fresh new process.
This change should be beneficial for performance as it:
- Avoids spinning up a new process (CPU & memory cost)
- Likely better leverages caches since this process already loaded this domain in the past
Due to current limitations, using a SuspendedPage's proxy may consume the SuspendedPage, which
means that it can no longer be used for PageCache on history navigations. We need to do this when
the SuspendedPageProxy in question was created for the current WebPageProxy because:
- This SuspendedPageProxy's process already has a suspended WebPage with this WebPageProxy's pageID
and
- We do not currently support having more than one WebPage with a given pageID within a single
WebProcess.
* UIProcess/SuspendedPageProxy.cpp:
(WebKit::SuspendedPageProxy::SuspendedPageProxy):
* UIProcess/SuspendedPageProxy.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::processForNavigationInternal):
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (237734 => 237735)
--- trunk/Source/WebKit/ChangeLog 2018-11-02 15:46:57 UTC (rev 237734)
+++ trunk/Source/WebKit/ChangeLog 2018-11-02 15:59:23 UTC (rev 237735)
@@ -1,3 +1,30 @@
+2018-11-02 Chris Dumez <[email protected]>
+
+ [PSON] Reuse SuspendedPages' process when possible, for performance
+ https://bugs.webkit.org/show_bug.cgi?id=191166
+
+ Reviewed by Geoffrey Garen.
+
+ When process-swapping check if there is an existing SuspendedPage for the domain we're going to.
+ If there is, use this SuspendedPage's process for the navigation instead of a fresh new process.
+ This change should be beneficial for performance as it:
+ - Avoids spinning up a new process (CPU & memory cost)
+ - Likely better leverages caches since this process already loaded this domain in the past
+
+ Due to current limitations, using a SuspendedPage's proxy may consume the SuspendedPage, which
+ means that it can no longer be used for PageCache on history navigations. We need to do this when
+ the SuspendedPageProxy in question was created for the current WebPageProxy because:
+ - This SuspendedPageProxy's process already has a suspended WebPage with this WebPageProxy's pageID
+ and
+ - We do not currently support having more than one WebPage with a given pageID within a single
+ WebProcess.
+
+ * UIProcess/SuspendedPageProxy.cpp:
+ (WebKit::SuspendedPageProxy::SuspendedPageProxy):
+ * UIProcess/SuspendedPageProxy.h:
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::processForNavigationInternal):
+
2018-11-01 Fujii Hironori <[email protected]>
Rename <wtf/unicode/UTF8.h> to <wtf/unicode/UTF8Conversion.h> in order to avoid conflicting with ICU's unicode/utf8.h
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp (237734 => 237735)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2018-11-02 15:46:57 UTC (rev 237734)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp 2018-11-02 15:59:23 UTC (rev 237735)
@@ -78,7 +78,7 @@
: m_page(page)
, m_process(WTFMove(process))
, m_mainFrameID(mainFrameID)
- , m_origin(SecurityOriginData::fromURL({ { }, item.url() }))
+ , m_registrableDomain(toRegistrableDomain(URL(URL(), item.url())))
{
item.setSuspendedPage(*this);
m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_page.pageID(), *this);
Modified: trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h (237734 => 237735)
--- trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2018-11-02 15:46:57 UTC (rev 237734)
+++ trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h 2018-11-02 15:59:23 UTC (rev 237735)
@@ -44,7 +44,7 @@
WebPageProxy& page() const { return m_page; }
WebProcessProxy& process() { return m_process.get(); }
uint64_t mainFrameID() const { return m_mainFrameID; }
- const WebCore::SecurityOriginData& origin() const { return m_origin; }
+ const String& registrableDomain() const { return m_registrableDomain; }
void unsuspend();
@@ -62,7 +62,7 @@
WebPageProxy& m_page;
Ref<WebProcessProxy> m_process;
uint64_t m_mainFrameID;
- WebCore::SecurityOriginData m_origin;
+ String m_registrableDomain;
bool m_isSuspended { true };
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (237734 => 237735)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-11-02 15:46:57 UTC (rev 237734)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-11-02 15:59:23 UTC (rev 237735)
@@ -2185,8 +2185,8 @@
} else
reason = "Process swap was requested by the client"_s;
+ auto registrableDomain = toRegistrableDomain(targetURL);
if (m_configuration->alwaysKeepAndReuseSwappedProcesses()) {
- auto registrableDomain = toRegistrableDomain(targetURL);
LOG(ProcessSwapping, "(ProcessSwapping) Considering re-use of a previously cached process for domain %s", registrableDomain.utf8().data());
if (auto* process = m_swappedProcessesPerRegistrableDomain.get(registrableDomain)) {
@@ -2196,8 +2196,10 @@
// FIXME: Architecturally we do not currently support multiple WebPage's with the same ID in a given WebProcess.
// In the case where this WebProcess has a SuspendedPageProxy for this WebPage, we can throw it away to support
// WebProcess re-use.
- // In the future it would be great to refactor-out this limitation.
- page.process().processPool().removeAllSuspendedPageProxiesForPage(page);
+ // In the future it would be great to refactor-out this limitation (https://bugs.webkit.org/show_bug.cgi?id=191166).
+ m_suspendedPages.removeAllMatching([&](auto& suspendedPage) {
+ return &suspendedPage->page() == &page && &suspendedPage->process() == process;
+ });
return makeRef(*process);
}
@@ -2204,6 +2206,24 @@
}
}
+ // Check if we have a suspended page for the given registrable domain and use its process if we do, for performance reasons.
+ auto it = m_suspendedPages.findIf([&](auto& suspendedPage) {
+ return suspendedPage->registrableDomain() == registrableDomain;
+ });
+ if (it != m_suspendedPages.end()) {
+ Ref<WebProcessProxy> process = (*it)->process();
+
+ // FIXME: If the SuspendedPage is for this page, then we need to destroy the suspended page as we do not support having
+ // multiple WebPages with the same ID in a given WebProcess currently (https://bugs.webkit.org/show_bug.cgi?id=191166).
+ if (&(*it)->page() == &page)
+ m_suspendedPages.remove(it);
+
+ if (&process->websiteDataStore() != &page.websiteDataStore())
+ process->send(Messages::WebProcess::AddWebsiteDataStore(page.websiteDataStore().parameters()), 0);
+
+ return process;
+ }
+
if (RefPtr<WebProcessProxy> process = tryTakePrewarmedProcess(page.websiteDataStore())) {
tryPrewarmWithDomainInformation(*process, targetURL);
return process.releaseNonNull();
Modified: trunk/Tools/ChangeLog (237734 => 237735)
--- trunk/Tools/ChangeLog 2018-11-02 15:46:57 UTC (rev 237734)
+++ trunk/Tools/ChangeLog 2018-11-02 15:59:23 UTC (rev 237735)
@@ -1,3 +1,14 @@
+2018-11-02 Chris Dumez <[email protected]>
+
+ [PSON] Reuse SuspendedPages' process when possible, for performance
+ https://bugs.webkit.org/show_bug.cgi?id=191166
+
+ Reviewed by Geoffrey Garen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
2018-11-02 Zalan Bujtas <[email protected]>
[LFC][IFC] Add support for intrinsic width calculation
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (237734 => 237735)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-11-02 15:46:57 UTC (rev 237734)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2018-11-02 15:59:23 UTC (rev 237735)
@@ -1523,7 +1523,7 @@
TestWebKitAPI::Util::run(&done);
done = false;
- auto pid1 = [webView _webProcessIdentifier];
+ auto webkitPID = [webView _webProcessIdentifier];
request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
[webView loadRequest:request];
@@ -1531,8 +1531,11 @@
TestWebKitAPI::Util::run(&done);
done = false;
- auto pid2 = [webView _webProcessIdentifier];
+ auto applePID = [webView _webProcessIdentifier];
+ // Verify the web pages are in different processes
+ EXPECT_NE(webkitPID, applePID);
+
request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
[webView loadRequest:request];
@@ -1541,13 +1544,9 @@
TestWebKitAPI::Util::run(&done);
done = false;
- auto pid3 = [webView _webProcessIdentifier];
+ // We should have gone back to the webkit.org process for this load since we reuse SuspendedPages' process when possible.
+ EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
- // Verify the web pages are in different processes
- EXPECT_NE(pid1, pid2);
- EXPECT_NE(pid1, pid3);
- EXPECT_NE(pid2, pid3);
-
// Verify the sessionStorage values were as expected
EXPECT_EQ([receivedMessages count], 2u);
EXPECT_TRUE([receivedMessages.get()[0] isEqualToString:@""]);
@@ -1554,6 +1553,58 @@
EXPECT_TRUE([receivedMessages.get()[1] isEqualToString:@"I exist!"]);
}
+TEST(ProcessSwap, ReuseSuspendedProcess)
+{
+ auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+ [processPoolConfiguration setProcessSwapsOnNavigation:YES];
+ auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+ auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ [webViewConfiguration setProcessPool:processPool.get()];
+ auto handler = adoptNS([[PSONScheme alloc] init]);
+ [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+ auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+ [webView setNavigationDelegate:delegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main1.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto webkitPID = [webView _webProcessIdentifier];
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main1.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ auto applePID = [webView _webProcessIdentifier];
+
+ EXPECT_NE(webkitPID, applePID);
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main2.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ // We should have gone back to the webkit.org process for this load since we reuse SuspendedPages' process when possible.
+ EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
+
+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main2.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ // We should have gone back to the apple.com process for this load since we reuse SuspendedPages' process when possible.
+ EXPECT_EQ(applePID, [webView _webProcessIdentifier]);
+}
+
static const char* mainFramesOnlyMainFrame = R"PSONRESOURCE(
<script>
function loaded() {
