New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 8 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocking:
issue 910708
issue 911307



Sign in to add a comment
link

Issue 898855: Experiment with splitting the cache by main frame origin

Reported by jkarlin@chromium.org, Oct 25 Project Member

Issue description

We've toyed with the idea of splitting the cache by the main frame's origin. We don't know what the impact would be on network bytes so let's create a field trial and find out.
 

Comment 1 by rbyers@chromium.org, Oct 25

Cc: kenjibaheux@chromium.org kinuko@chromium.org bmcquade@chromium.org
Labels: -Type-Bug Type-Feature
I agree we should try this out.

/cc Kinuko as loading expert and someone who has been talking with Apple about how signed exchanges should integrate with double-keyed caching.

In addition to network bytes, I think it's also important that we look at page load metrics (FCP and FID for now, though FCP++ once we have it is probably really what we want). We'll also want to break down by geo - eg. this might be OK in the US but terrible in India where network reliability is a bigger issue, or Africa where raw bandwidth is a huge problem.

Comment 2 by kenjibaheux@google.com, Oct 25

I think we'll need to know how much value we would lose:
 - storage savings
 - bandwidth savings
 - performance wins
 - ???

Options: metrics, estimations, lab (dirty proto), experiment (ablation, quick proto), ???

This would allow us to assess how much efforts* we should put into maintaining (if not improving) some or all of the current/perceived upsides.


*: for instance, this exploration with Mozilla, Apple at TPAC: https://docs.google.com/document/d/1lmEpFPqGSDuW6IH5nFwMF8vqjzm0DjHPKynTM_nmOQk/edit#heading=h.z4j7mu29yu2o

Comment 3 by kinuko@chromium.org, Oct 25

+1 to experiment with this, we should at least have better understanding on how this affects, for us to have better design decisions. This could probably give us another angle on how much we should look into ideas like web shared libraries (having a loss in one setting wouldn't immediately mean we should stop pursuing it).

The metrics sliced by device capabilities could be also interesting.

Comment 4 by jkarlin@google.com, Oct 26

Status: Started (was: Assigned)
Alright. I'll begin by plumbing the main frame origin up through resource requests to the network stack. Then I'll add a cache experiment that keys by the main frame origin concatenated with the url. We'll want this to be a persistent experiment and wipe the backend as a user enters the experiment (or control group) for an apples-to-apples comparison.

Comment 5 by morlovich@chromium.org, Oct 26

You may need to be extra careful with the cache-clearing privacy features.

Comment 6 by jkarlin@chromium.org, Oct 26

Maks: Can you elaborate on what you mean by the cache-clearing privacy features?

Comment 7 by morlovich@chromium.org, Oct 26

There are APIs that let one clear a portion of the cache based on a list of hosts or domains. For network service it's NetworkContext::ClearHttpCache, plus there are some layers higher up which might not hit it (to support the media cache in non-network service case). 

I thought settings -> clear browsing data exposed it, but it looks like it might not? 

(clear-site-data uses it, but breaking that would be a whooolllleee lot less
 of a big deal than breaking user-initiated clearance --- it was disabled for caches for a while anyway, and it's not possible for 
 us to implement efficiently...)

Comment 8 by jkarlin@chromium.org, Oct 26

Ah, yes, good to be cautious there. I don't see per-domain exposed in the clear browsing data dialog on desktop. Perhaps it's okay for an experiment to just support deleting the whole cache or cache entries older than X.

Another concern is the non-standard methods of getting to the cache. For instance, what should we do about H2 Push? For the experiment I'm thinking of just not adding an origin to those and they'll be wasted, but that shouldn't be breaking and I imagine they're relatively rare.

Another concern is the metadata writer. If compiled v8 is written to the cache it should get stored with the correct entry.

Comment 9 by morlovich@chromium.org, Oct 26

WRT to the metadata writer, moving away from that to per-domain stuff managed by the browser (in a separate simplecache) is in process, and in experiment:
https://cs.chromium.org/chromium/src/net/base/features.h?rcl=2eb52a2d9763e244047de59257ddd5e483f07ac9&l=18

Comment 10 by morlovich@chromium.org, Oct 26

Side thought: jefftk might be a good person to talk to, him and his teammates seem really good at noticing us doing things that affect cache hit rates for ads javascript

Comment 11 by jkarlin@chromium.org, Oct 26

I'll get in touch with Jeff's team. Good idea there.

My plan is only to add the main-frame origin for subresources fetched for a document. This means that Push, Workers, and ServiceWorker requests will not be double-keyed. We can add that later, but assuming such requests are the minority, I'd rather get something up fast that can give us a reasonable approximation.

Cool about the separate code cache. Any chance that will launch soon? I guess for now I'll also ignore the metadata writes. This will negatively impact performance until your feature launches though.

Comment 12 by morlovich@chromium.org, Oct 26

They seem to be targeting M72 if everything goes well:
https://bugs.chromium.org/p/chromium/issues/detail?id=892652#c13
(unless that means just for stable experiment? at any rate mythria@ is the driving engineer there)

Comment 13 by jkarlin@chromium.org, Nov 19

Cc: kleber@google.com

Comment 14 by jkarlin@chromium.org, Nov 30

Issue 910708 has the meta bug for all of the things that would need to be done if we want to make the split cache real after the experiment.

Comment 15 by horo@chromium.org, Dec 4

Blocking: 911307

Comment 16 by bugdroid1@chromium.org, Dec 7

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/be37f9105bb8760e009c71e61770b514449fe398

commit be37f9105bb8760e009c71e61770b514449fe398
Author: Josh Karlin <jkarlin@chromium.org>
Date: Fri Dec 07 01:05:14 2018

Plumb the top frame origin of resource requests to the net stack

In order to experiment with splitting the disk cache by top frame
origin, we'll need to plumb the top frame origin up to the network stack
for each resource request.

We're focused on just document subresource requests and navigation
requests for now because that's the majority of request types and this
is meant to support an early experiment. We will need to include other
types of requests (e.g., from workers, or signed exchange redirects)
later.

Subresources requests (and subframe navigations) don't change the top
frame origin on redirect. Top frame navigations do however.

Bug:  898855 
Change-Id: I86a1c5a61986d15c445991c4e50482209f051cca
Reviewed-on: https://chromium-review.googlesource.com/c/1335869
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614520}
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/browser/devtools/devtools_url_loader_interceptor.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/browser/frame_host/navigation_request_info.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/browser/frame_host/navigation_request_info.h
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/browser/frame_host/navigator_impl_unittest.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/browser/loader/navigation_url_loader_impl_unittest.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/browser/loader/navigation_url_loader_unittest.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/browser/loader/resource_dispatcher_host_impl.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/browser/loader/resource_dispatcher_host_unittest.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/browser/navigation_browsertest.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/common/service_worker/service_worker_loader_helpers.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/common/throttling_url_loader.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/renderer/loader/web_url_loader_impl.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/renderer/loader/web_url_loader_impl_unittest.cc
[add] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/content/test/data/page_with_iframe_and_image.html
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/headless/test/test_network_interceptor.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/net/url_request/redirect_info.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/net/url_request/redirect_info.h
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/net/url_request/redirect_info_unittest.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/net/url_request/url_request.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/net/url_request/url_request.h
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/net/url_request/url_request_job.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/net/url_request/url_request_unittest.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/services/network/public/cpp/net_ipc_param_traits.h
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/services/network/public/cpp/network_ipc_param_traits.h
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/services/network/public/cpp/resource_request.h
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/services/network/url_loader.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/public/platform/web_url_loader_client.h
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/public/platform/web_url_request.h
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/renderer/core/dom/document.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/renderer/core/dom/document.h
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/renderer/core/exported/web_document_test.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/renderer/core/loader/frame_fetch_context.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/renderer/core/loader/frame_fetch_context.h
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/renderer/platform/exported/web_url_request.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/renderer/platform/loader/fetch/resource_loader.h
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/renderer/platform/loader/fetch/resource_request.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/renderer/platform/loader/fetch/resource_request.h
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/renderer/platform/loader/fetch/resource_request_test.cc
[modify] https://crrev.com/be37f9105bb8760e009c71e61770b514449fe398/third_party/blink/renderer/platform/testing/weburl_loader_mock.cc

Comment 17 by creis@chromium.org, Dec 7

Cc: mythria@chromium.org creis@chromium.org
Components: Internals>Sandbox>SiteIsolation
Nice-- I'm excited to see experiments for this.  CC'ing Site Isolation folks (and mythria@ from issue 892652) just to follow along.  (This is interesting for having a bit more isolation between requests from different origins, though it's worth noting the limitations in comment 11 for the time being.  Those seem fine for an experiment.)

Comment 18 by nasko@chromium.org, Dec 7

Cc: nasko@chromium.org

Comment 19 by mythria@google.com, Dec 10

Cc: rmcilroy@chromium.org
Re: separate code caches: Yes, the separate double-keyed code caches are enabled by default starting 72.0.3622.0: https://chromium-review.googlesource.com/c/chromium/src/+/1337625

Comment 20 by bugdroid1@chromium.org, Dec 10

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f3caee16417dc39e55af87787de4dc6b472d8832

commit f3caee16417dc39e55af87787de4dc6b472d8832
Author: Josh Karlin <jkarlin@chromium.org>
Date: Mon Dec 10 22:54:27 2018

Create experiment to split cache by top frame origin

This CL prefixes the disk cache's key with the top frame origin,
 like this: "[top frame url].[url]". This way the same resource
loaded from two pages with different origins won't use the same
cached entry.

Changes:
* Added a new experiment, SplitCacheByTopFrameOrigin. The
  experiment does *not* clear the caches on entry/exit
  from the experiment. If this proves to be problematic
  we can fix that later.
* Added a top_frame_url member to HttpRequestInfo,
  copied from the URLRequest.
* Changed the cache key generation function.
* Added both unit and browser tests.

Bug:  898855 
Change-Id: I4a693e98af88de71d1ae6c4630c5df9fa042670b
Reviewed-on: https://chromium-review.googlesource.com/c/1352344
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615294}
[modify] https://crrev.com/f3caee16417dc39e55af87787de4dc6b472d8832/content/browser/web_contents/web_contents_impl_browsertest.cc
[modify] https://crrev.com/f3caee16417dc39e55af87787de4dc6b472d8832/net/base/features.cc
[modify] https://crrev.com/f3caee16417dc39e55af87787de4dc6b472d8832/net/base/features.h
[modify] https://crrev.com/f3caee16417dc39e55af87787de4dc6b472d8832/net/http/http_cache.cc
[modify] https://crrev.com/f3caee16417dc39e55af87787de4dc6b472d8832/net/http/http_cache_unittest.cc
[modify] https://crrev.com/f3caee16417dc39e55af87787de4dc6b472d8832/net/http/http_request_info.h
[modify] https://crrev.com/f3caee16417dc39e55af87787de4dc6b472d8832/net/http/mock_http_cache.cc
[modify] https://crrev.com/f3caee16417dc39e55af87787de4dc6b472d8832/net/url_request/url_request_http_job.cc
[modify] https://crrev.com/f3caee16417dc39e55af87787de4dc6b472d8832/net/url_request/url_request_unittest.cc

Comment 21 by bugdroid1@chromium.org, Dec 17

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/158ea09bc8b714af5767114d432457d1c7a3c118

commit 158ea09bc8b714af5767114d432457d1c7a3c118
Author: Maks Orlovich <morlovich@chromium.org>
Date: Mon Dec 17 22:16:12 2018

Wire up top_frame_origin to other things using HttpCache::GenerateKey

This is a follow up https://chromium-review.googlesource.com/c/1352344
(kept separate to avoid merge conflicts with a starter CL)

Bug:  898855 
Change-Id: I1d427220c0de8c8e03c0688336e2b967c60507fa
Reviewed-on: https://chromium-review.googlesource.com/c/1374491
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617254}
[modify] https://crrev.com/158ea09bc8b714af5767114d432457d1c7a3c118/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/158ea09bc8b714af5767114d432457d1c7a3c118/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/158ea09bc8b714af5767114d432457d1c7a3c118/content/common/frame_messages.h
[modify] https://crrev.com/158ea09bc8b714af5767114d432457d1c7a3c118/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/158ea09bc8b714af5767114d432457d1c7a3c118/net/http/http_cache.cc
[modify] https://crrev.com/158ea09bc8b714af5767114d432457d1c7a3c118/net/http/http_cache.h
[modify] https://crrev.com/158ea09bc8b714af5767114d432457d1c7a3c118/net/http/http_cache_transaction.cc
[modify] https://crrev.com/158ea09bc8b714af5767114d432457d1c7a3c118/net/http/http_cache_unittest.cc
[modify] https://crrev.com/158ea09bc8b714af5767114d432457d1c7a3c118/services/network/network_context.cc
[modify] https://crrev.com/158ea09bc8b714af5767114d432457d1c7a3c118/services/network/network_context.h
[modify] https://crrev.com/158ea09bc8b714af5767114d432457d1c7a3c118/services/network/network_context_unittest.cc
[modify] https://crrev.com/158ea09bc8b714af5767114d432457d1c7a3c118/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/158ea09bc8b714af5767114d432457d1c7a3c118/services/network/test/test_network_context.h

Comment 22 by yoavweiss@chromium.org, Dec 24

Cc: yoavweiss@chromium.org

Comment 23 by jkarlin@chromium.org, Jan 2

For those watching, the experiment is humming along nicely. The cache hit rate drops by about 6% but the overall fraction of bytes coming from the cache is only dropping ~1%. FCP looks like it's likely to increase some but nothing terrible.

https://uma.googleplex.com/p/chrome/variations/?sid=f1fd854a2fad888be0fab2844bdad6fa

Comment 24 by kinuko@chromium.org, Jan 17

Blocking: 910708

Comment 25 by jkarlin@chromium.org, Feb 4

Cc: -bmcquade@chromium.org shivanisha@chromium.org
Status: Fixed (was: Started)
The initial experiment seems reasonable, so Shivani (shivanisha@) will take over the launch process.

Sign in to add a comment