Bug 98539 – Refactor resource loading to allow for out-of-process loading and memory caching

• Status: ASSIGNED
• Component: Page Loading

[Brady Eidson]

Refactor resource loading to allow for out-of-process loading and memory caching.

This is to support https://bugs.webkit.org/show_bug.cgi?id=98537

[Dimitri Glazkov (Google)]

Is this in WebCore?

[Adam Barth]

When we experimented with moving the MemoryCache out of process, it looked like a net loss.  I'm curious what experiments you've run that show that having the MemoryCache out of process is a win.

[Brady Eidson]

(In reply to comment #1)
> Is this in WebCore?

Yes.

[Brady Eidson]

(In reply to comment #2)
> When we experimented with moving the MemoryCache out of process, it looked like a net loss.  I'm curious what experiments you've run that show that having the MemoryCache out of process is a win.

Net loss in what regard?  Memory use?  Performance?  Etc?  Please share data if you have it.

Experimentally we know that running multiple WebProcesses with individual caches uses unacceptably more memory and - without disk caching - leads to slower page load speeds.

[Brady Eidson]

(In reply to comment #3)
> (In reply to comment #1)
> > Is this in WebCore?
> 
> Yes.

Let me elaborate on this - Refactoring will have to occur in WebCore to support this.

But the feature itself will be in WebKit2.

[Adam Barth]

> Net loss in what regard?  Memory use?  Performance?  Etc?  Please share data if you have it.

We conducted the experiments in 2007 and then again in 2009.  I don't think we have the data around anymore.

> Experimentally we know that running multiple WebProcesses with individual caches uses unacceptably more memory and - without disk caching - leads to slower page load speeds.

Yes, that's correct.  The approach we use in Chromium is to have a global budget for memory cache.  We then apportion the budget to the various render processes based on a number of heuristics.  For example, foreground render processes get a bigger budget than background processes.  When a process is moved to the background, we ask it to reduce its memory cache usage if the budget is tight.  See

http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/web_cache_manager.h
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/web_cache_manager.cc

for details.

As I recall, the experiments we ran showed that there wasn't much duplication of cached items across render processes, so it wasn't worth using shared memory to unify the caches.

[Brady Eidson]

Created attachment 167558 [details]
Patch v1 (for EWS)

Not marking for review just yet, as I expect to get EWS fallout that I'll need to attend to.

[Brady Eidson]

(In reply to comment #6)
> > Net loss in what regard?  Memory use?  Performance?  Etc?  Please share data if you have it.
> 
> We conducted the experiments in 2007 and then again in 2009.  I don't think we have the data around anymore.
> ...
> As I recall, the experiments we ran showed that there wasn't much duplication of cached items across render processes, so it wasn't worth using shared memory to unify the caches.

We have data that shows it is worth it for our expectations in WebKit 2.

I've attached a patch here to get the EWS ball rolling, but am also sending out an email to webkit-dev since you've requested it.

[WebKit Review Bot]

Attachment 167558 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebCore/loader/cache/CachedImage.cpp:39:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/loader/ResourceBuffer.h:49:  More than one command on the same line in if  [whitespace/parens] [4]
Total errors found: 2 in 28 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 167558 [details]
Patch v1 (for EWS)

Attachment 167558 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14207560

[Brady Eidson]

Created attachment 167569 [details]
Patch v2 - Quell the obvious EWS warnings

[Brady Eidson]

Meant to put that patch on 98541, and to actually submit to EWS.  *sigh*

[Antti Koivisto]

Created attachment 167753 [details]
instrumentation patch for duplicate cache resources

Related to this work, here is a patch that can be used to instrument the cache for duplicate resources in different URLs.