Bug 219689

Summary: [WPE][GTK] Should enable WebProcessCache
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, berto, bugs-noreply, cdumez, cgarcia, ews-watchlist, gustavo, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
aperez: review+
Patch for landing none

Michael Catanzaro
Reported 2020-12-09 08:34:27 PST
It looks like WPE and GTK do not enable the WebProcessCache feature added in r241556: """ Introduce a WebContent Process cache to reduce the number of process launches when process swap on navigation is enabled, and to reduce the power cost of the feature. If a WebProcess loaded pages from a single registrable domain then it is eligible for the cache. When process-swapping on navigation to a new registrable domain, we now attempt to retrieve a process from the cache for the domain in question instead of always launching a new one. The WebProcess cache currently has the following attributes: It may contains 4 processes per GB of RAM the machine has, up to 30 processes. WebProcesses automatically get evicted from the cache after 30 minutes. If the application is no longer the active app, then the cache will get cleared after 5 minutes. WebProcesses that are in the cache are reported as "(Cached)" in Activity Monitor. The WebProcess cache is currently disabled by default and can by enabled by the client via SPI. """ We probably want to have that enabled, or at least make it possible to enable it?
Attachments
Patch (1.25 KB, patch)
2020-12-10 05:23 PST, Carlos Garcia Campos 		aperez: review+
DetailsFormatted DiffDiff
Patch for landing (1.26 KB, patch)
2020-12-14 03:01 PST, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2020-12-09 08:36:16 PST
If you have Process-Swap-On-Navigation (PSON) enabled, then I think you'd definitely want the process cache too. Note that Apple only enables the process-cache on desktop (not mobile) due to memory constraints.
Michael Catanzaro
Comment 2 2020-12-09 08:39:03 PST
Yeah of course we have PSON. That has been enabled for ages now. I think nobody realized that the process cache had to be enabled separately.
Michael Catanzaro
Comment 3 2020-12-09 08:42:22 PST
(In reply to Chris Dumez from comment #1) > If you have Process-Swap-On-Navigation (PSON) enabled, then I think you'd > definitely want the process cache too. Note that Apple only enables the > process-cache on desktop (not mobile) due to memory constraints. Currently we have a chassis type check that could be used for this, but it doesn't work reliably due to bug #218122, and is currently only used to set a mobile user agent header. It would probably make more sense to enable or disable the cache based on total available RAM, rather than checking form factor, since there's not really any difference between a mobile phone with 3 GB RAM (Librem 5) and a desktop with 3 GB RAM. The performance hit if you run out of RAM is going to be bad either way.
Chris Dumez
Comment 4 2020-12-09 08:44:34 PST
(In reply to Michael Catanzaro from comment #3) > (In reply to Chris Dumez from comment #1) > > If you have Process-Swap-On-Navigation (PSON) enabled, then I think you'd > > definitely want the process cache too. Note that Apple only enables the > > process-cache on desktop (not mobile) due to memory constraints. > > Currently we have a chassis type check that could be used for this, but it > doesn't work reliably due to bug #218122, and is currently only used to set > a mobile user agent header. > > It would probably make more sense to enable or disable the cache based on > total available RAM, rather than checking form factor, since there's not > really any difference between a mobile phone with 3 GB RAM (Librem 5) and a > desktop with 3 GB RAM. The performance hit if you run out of RAM is going to > be bad either way. Yes, Apple port listen to memory pressure warnings and kill those cached processes as necessary.
Carlos Garcia Campos
Comment 5 2020-12-10 05:22:33 PST
I thought it was enabled. I'll submit a patch.
Carlos Garcia Campos
Comment 6 2020-12-10 05:23:25 PST
Created attachment 415860 [details] Patch
EWS Watchlist
Comment 7 2020-12-10 05:24:15 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Adrian Perez
Comment 8 2020-12-10 06:28:46 PST
Comment on attachment 415860 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415860&action=review > Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:394 > + configuration.setUsesWebProcessCache(true); I think it's fine to land this as-is, but depending on things go we might need to either add some smarts to choose a sensible default for this and/or add API to WebKitWebContext to toggle it.
Michael Catanzaro
Comment 9 2020-12-10 06:56:39 PST
The GTK API EWS is red. Score +1 for whoever set up that EWS bot. That is nice to have....
Carlos Garcia Campos
Comment 10 2020-12-14 03:01:00 PST
Created attachment 416146 [details] Patch for landing
Carlos Garcia Campos
Comment 11 2020-12-14 04:28:25 PST
Committed r270767: <https://trac.webkit.org/changeset/270767>
Note You need to log in before you can comment on or make changes to this bug.