In our Indexed DB implementation we keep backing stores alive for a few seconds (hardcoded, selected arbitrarily) connections are dropped. Could do the same thing here to avoid a long term memory hit.

Yeah, a timeout is possible.  I think that would limit the effectiveness of the optimization for some sites, though.  Also, I think simple disk_cache is less memory hungry than leveldb.  I'm inclined to do the simple thing at first and measure how well it works.

Thanks for raising this. A couple of questions:
1) Would it be possible for the service worker to load the Cache objects into memory pre-emptively, when the service worker starts up, so the resources are quickly accessible by the time the resource fetch event is triggered.
2) What's the risk of always keeping the Cache in memory? Does it prevent modifications or maintenance to happen to the disk version? I'm thinking for cases like Docs, where the service worker is almost always up because of network activity from several tabs.
3) Would it be possible to implement a workaround for this in code, until this is fixed in stable? i.e. If we just opened the correct Cache object, and kept  the handle for it in memory on sw startup, could that give us a perf advantage?

Just to clarify, this will not keep the entire contents of the cache in memory.  It just keeps the simple disk_cache open which has some amount of per-entry memory for fast cache-miss detection.  The simple disk_cache design document says it should be about 100 bytes per entry:

http://www.chromium.org/developers/design-documents/network-stack/disk-cache/very-simple-backend

+morlovich who might know if the current implementation meets that memory goal.

Regarding (2), I don't think there should be any functional downside to keeping the simple disk_cache open longer.  I believe it was designed to function this way.

Regarding (3), yes you could implement code in your service worker script to get this behavior now.  If you do a `caches.open()` and keep the resulting Cache object referenced in your service worker then the disk_cache will remain open.  The current code closes the disk_cache backend when the last reference is dropped.

Oh, I didn't answer the early warming question.  That might be possible as well, but it runs a larger risk of wasting device resources for data that might never be accessed.  It would probably need a more complex heuristic for determining what to pre-warm and how long to keep it around.

Since keeping already accessed caches open longer can be a very simple patch I was hoping to do that first.  It may be possible to merge it to 71.

WRT to memory usage: it stores 8 bytes of data with 8 bytes of key in an unordered_map, but unordered_map can be quite wasteful itself. Should still be way under 100 bytes per entry, though.

(The flipside, however, is that we don't know the keys/URLs of the entries until we go to disk, only the hashes of the keys).

And it is a good idea to keep it open since if it's closed, we need to write out the index to disk, and re-open needs to read, so it's serialized to be after that, which can add up quickly...

If we do keep it in-memory, do you encourage us to use Cache#match() rather than CacheStorage#match(), to ensure that the in-memory Cache is used without needing to wait for all Caches to be opened?

If you already have the correct Cache open and know that the resource should be in that Cache, then using cache.match() should be fastest.

If you have all your Cache objects open and are not sure which one holds the given URL, then its probably still best to use caches.match().

There are some additional issues to deal with here:

1. I'm adding a memory pressure listener to close unreferenced cache objects at the "critical" level.
2. It appears browser-side CacheStorage objects are created immediately for all origins. (Probably by quota system...)  This means warm cache data would be kept alive even after all pages for the origin are closed.  I need to add some mechanism to close cache objects when blink destroys the last CacheStorage object for the origin.

Can you expand on the second point? What exactly is "warm cache data" that is kept alive? Does this change the likelihood of the workaround we discussed above from having an effect?

Currently when a browser process CacheStorage object is created, its never destroyed until either browser shutdown or the origin storage is wiped.  This is fine today because if there are no renderer processes referencing the CacheStorageCache objects then they deallocate.  So having the CacheStorage objects sitting there doesn't really use any resources.

With my proposed change here, though, CacheStorageCache objects would no longer cleanup when de-referenced.  We would keep them open so that we can optimize the next time a renderer needs to reference it.  Given that quota manager touches every origin the user has ever visited this can be a lot of "warmed" CacheStorageCache objects.

My plan is to only keep unreferenced CacheStorageCache objects alive if there is a renderer with a service worker/window/etc of the same origin.  So if there are no contexts of the given origin then the browser-side CacheStorageCache objects would be allowed to close.

Since docs has many long running contexts I believe it would still benefit from the optimization.

Another piece of the puzzle:

There is code here that debounces Cache opens for explicit caches.open() calls:

https://cs.chromium.org/chromium/src/content/browser/cache_storage/cache_storage_dispatcher_host.cc?l=364-368&rcl=85f78938e304061d5dea7a5455ffaab5d7881adb

This was added in  bug 558427 .  As noted in #c0, though, this doesn't help caches.match() where this path is not taken.

I'm still inclined to just keep these objects open until there is no renderer using the origin any more or memory pressure triggers a flush.  So I'm likely to remove this timer code in this bug.

This timer code may also be contributing to bug 782869.  If a caches.delete() is performed while the timer is holding the cache alive then the deletion is unnecessarily delayed.

Thanks for the clarification.

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

commit fdf95b6bd04cda5359df138f8f8012759ce638ad
Author: Ben Kelly <wanderview@chromium.org>
Date: Fri Nov 16 05:04:28 2018

CacheStorage: Refactor handle ref-counting.

This CL factors out the reference counting handle code into a new
CacheStorageHandleType template.  This is a first step towards
referencing counting handles for both CacheStorageCache and
CacheStorage objects.

This CL also has the added benefit of removing a std::map in favor
of storing the count directly on the target object itself.

Bug:  902488 
Change-Id: I75c0d86f57c338d3da1db33da40a893c120a720e
Reviewed-on: https://chromium-review.googlesource.com/c/1331847
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608670}
[modify] https://crrev.com/fdf95b6bd04cda5359df138f8f8012759ce638ad/content/browser/BUILD.gn
[modify] https://crrev.com/fdf95b6bd04cda5359df138f8f8012759ce638ad/content/browser/cache_storage/cache_storage.cc
[modify] https://crrev.com/fdf95b6bd04cda5359df138f8f8012759ce638ad/content/browser/cache_storage/cache_storage.h
[modify] https://crrev.com/fdf95b6bd04cda5359df138f8f8012759ce638ad/content/browser/cache_storage/cache_storage_cache.cc
[modify] https://crrev.com/fdf95b6bd04cda5359df138f8f8012759ce638ad/content/browser/cache_storage/cache_storage_cache.h
[delete] https://crrev.com/39f676e6d2817c5b85d1a8ef9ba0f96cd4a970b1/content/browser/cache_storage/cache_storage_cache_handle.cc
[modify] https://crrev.com/fdf95b6bd04cda5359df138f8f8012759ce638ad/content/browser/cache_storage/cache_storage_cache_handle.h
[modify] https://crrev.com/fdf95b6bd04cda5359df138f8f8012759ce638ad/content/browser/cache_storage/cache_storage_cache_unittest.cc
[add] https://crrev.com/fdf95b6bd04cda5359df138f8f8012759ce638ad/content/browser/cache_storage/cache_storage_ref.h
[modify] https://crrev.com/fdf95b6bd04cda5359df138f8f8012759ce638ad/content/browser/renderer_host/code_cache_host_impl.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d72398adc749d18a5ba21711a53efa7c497e435

commit 5d72398adc749d18a5ba21711a53efa7c497e435
Author: Ben Kelly <wanderview@chromium.org>
Date: Sat Dec 08 00:08:18 2018

CacheStorage: Track explicit handle references to CacheStorage objects.

This CL adds a CacheStorageHandle type that tracks explicit references
to a CacheStorage instance.  This will allow us to determine when an
opened CacheStorage instance is no longer needed and can be cleaned up.
This in turn may allow us to more aggressively keep data in memory
while the instance is actively being used.

This CL also includes some refactoring in order to better associate
the CacheStorageHandle with the CacheStorage mojo interface.  A new
CacheStorageImpl now holds a CacheStorageHandle and directly accesses
the associated CacheStorage object.

Further the CacheStorage operation methods in CacheStorageManager and
CacheStorageDispatcherHost are no longer necessary.  This CL removes
them.

Finally, the background_fetch code has been modified to hold a
CacheStorageHandle for each active registration instead of using the
CacheStorageManager methods.

Bug:  902488 
Change-Id: I22d5932361e078b9e077d28c2260235b87c41249
Reviewed-on: https://chromium-review.googlesource.com/c/1347210
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Mugdha Lakhani <nator@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614876}
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/BUILD.gn
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/background_fetch/background_fetch_data_manager.cc
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/background_fetch/background_fetch_data_manager.h
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/background_fetch/background_fetch_data_manager_unittest.cc
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/background_fetch/storage/create_metadata_task.cc
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/background_fetch/storage/database_task.cc
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/background_fetch/storage/database_task.h
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/background_fetch/storage/delete_registration_task.cc
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/background_fetch/storage/mark_request_complete_task.cc
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/background_fetch/storage/match_requests_task.cc
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/cache_storage/cache_storage.cc
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/cache_storage/cache_storage.h
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/cache_storage/cache_storage_cache.cc
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/cache_storage/cache_storage_cache.h
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/cache_storage/cache_storage_dispatcher_host.cc
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/cache_storage/cache_storage_dispatcher_host.h
[add] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/cache_storage/cache_storage_handle.h
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/cache_storage/cache_storage_histogram_utils.h
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/cache_storage/cache_storage_manager.cc
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/cache_storage/cache_storage_manager.h
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/cache_storage/cache_storage_manager_unittest.cc
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/renderer_host/code_cache_host_impl.cc
[modify] https://crrev.com/5d72398adc749d18a5ba21711a53efa7c497e435/content/browser/service_worker/service_worker_browsertest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8423b03442e09d15cb0148893439fbe7988cefba

commit 8423b03442e09d15cb0148893439fbe7988cefba
Author: Ben Kelly <wanderview@chromium.org>
Date: Sat Dec 08 02:04:03 2018

Hold warmed CacheStorageCache objects open.

Opening files is expensive on some platforms.  To avoid paying the cost
of opening the simple disk_cache backends multiple times we can instead
hold warmed CacheStorageCache object open.  They will eventually be closed
once the origin's CacheStorage is no longer actively referenced or a
critical memory pressure events occurs.

Bug:  902488 
Change-Id: I4567e42d127763cd2ad644d00951e712d7946c18
Reviewed-on: https://chromium-review.googlesource.com/c/1321209
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614913}
[modify] https://crrev.com/8423b03442e09d15cb0148893439fbe7988cefba/content/browser/cache_storage/cache_storage.cc
[modify] https://crrev.com/8423b03442e09d15cb0148893439fbe7988cefba/content/browser/cache_storage/cache_storage.h
[modify] https://crrev.com/8423b03442e09d15cb0148893439fbe7988cefba/content/browser/cache_storage/cache_storage_cache.cc
[modify] https://crrev.com/8423b03442e09d15cb0148893439fbe7988cefba/content/browser/cache_storage/cache_storage_cache.h
[modify] https://crrev.com/8423b03442e09d15cb0148893439fbe7988cefba/content/browser/cache_storage/cache_storage_dispatcher_host.cc
[modify] https://crrev.com/8423b03442e09d15cb0148893439fbe7988cefba/content/browser/cache_storage/cache_storage_manager.cc
[modify] https://crrev.com/8423b03442e09d15cb0148893439fbe7988cefba/content/browser/cache_storage/cache_storage_manager.h
[modify] https://crrev.com/8423b03442e09d15cb0148893439fbe7988cefba/content/browser/cache_storage/cache_storage_manager_unittest.cc