Sorry for the ignorance. What is the network service?

It is true that the code cache relies on the JS source to be already cached by the http cache. The code cache data is stored there as cache metadata.

Drive-by comments:

#2: The network service implements network-related functionality in its own mojo process. It's a major, ongoing refactoring of Chromium's networking functionality. It isn't (and can't be) fully API compatible with the previous, non-mojo-ified code, so this requires work on many neighbouring code bits.

#0: networking and RenderMessageFilter are several layers away from what V8 team is working on. (V8 --> Blink/V8 bindings --> blink::Resource --> ... [Here I get lost myself]) I suspect they're not the right people to address this.

#4: It turns out, I think I'm not the right person to address this either, since these days I'm not working on any of the involved pieces.


Writing a proper integration test is a good idea, but back when I looked at this I didn't find a palatable way of doing that. It would be useful if the senior engs on this issue could outline a good way of testing this.

We have one test for service-workers [1] that checks that we have both produced and consumed the code. If I understand correctly, the service-workers caching would go through DidGenerateCacheableMetadataInCacheStorage. So, if that test is working with network service then I thought normal caches would work as well.

leszeks@ tried adding a similar test (using devtools) for the normal caches but could not get it to working. We could not think of any other alternatives. 

[1] https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-worker-v8-cache.js?dr

I am pretty sure DidGenerateCacheableMetadataInCacheStorage shouldn't have been listed in the original report, and as a flipside, it working doesn't mean DidGenerateCacheableMetadata would.  Basically CacheStorage has its own directories for stuff, that are completely distinct from what HttpCache uses (they are not even format compatible on the platforms where they use the same disk_cache backend, IIRC); so Network Service owning the HTTP cache doesn't change anything for that.

DidGenerateCacheableMetadata does of course need to be changed. (And this may have a lot of overlap with the double-keying work, also when it comes to deciding how it's designed wrt to whether the browser process is involved or not, etc.)

 Issue 841526  has been merged into this issue.

I just stumbled on this bug.  V8 Code Cache has been crucial to service worker performance. It seems important to fix this, if it's not fixed already.

I think this does not impact the service worker cache. They are not owned by the network service if I understand correctly. This is only for the code currently stored in the HttpCache.

BTW the service worker test mentioned in c#5 is failing in NetworkService:
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/FlagExpectations/enable-features%3DNetworkService?l=27&rcl=146eef8c14ff3a313aa7757ba8dd4bfd17563ad2

With the new design of the code caches that are decoupled from the HttpCache, I am not sure this would be needed. The new metadata caches will be owned by the browser and not by network service. Though the decoupled code caches would take at least a couple of quarters to be enabled by default. We may have to fix this if network service is enabled before that time.

+horo@ for the failure on service worker tests. I am not sure the v8 code caches and service worker caches are the same issue though. See c#6.

#12- depending on expected perf regression & how long the new code could take we might some migration there, though that can probably be decided after we start to get more perf data...

And yes the service worker one should be fine, and mythria's new design should solve this for regular loading too eventually.