WICG / compression-dictionary-transport

Other
96 stars 8 forks source link

Automatic retry fetching on cached dictionary read failure #1

Closed horo-t closed 1 year ago

horo-t commented 1 year ago

Can we make the browser automatically retry the request without the sec-bikeshed-available-dictionary: header when it failed to read the cached dictionary?

The current explainer says:

In case the browser advertized a dictionary but then fails to successfuly fetch it from its cache and the dictionary was used by the server, the resource request should be terminated

So the browser must check the existence of the cached dictionary on the disk before sending the request to reduce the risk of such failure.

If the automatic retry is allowed, the browser can speculatively send the request with sec-bikeshed-available-dictionary: header without checking the cached dictionary. I think this is very important for performance.

yoavweiss commented 1 year ago

How often would we expect such read failures to actually happen?

/cc @pmeenan

pmeenan commented 1 year ago

It shouldn't be meaningfully different from a 304 not-modified and should probably be treated the same way. In both cases, the browser has an index where it believes the cache entry is available, makes the request and depends on the entry actually being there.

Not sure if the fetch specs actually specify the behavior in that case of if it is implementation-specific but the breakage (and overhead for checking the entry) should be the same.

horo-t commented 1 year ago

I was thinking about using two databases, SQLite for keeping the metadata (path, hash, etc), DiskCache for keeping the blob (binary) of the dictionary. (PoC CL: https://chromium-review.googlesource.com/c/chromium/src/+/3872966/37) But unfortunately, DiskCache doesn’t provide a way to detect the cache entry eviction. So Chrome has to open the DiskCache entry before sending the request with the sec-bikeshed-available-dictionary: header. This is not good for performance.

It sounds like using DiskCache as a blob storage is not a good idea. It is impossible to keep the two databases in sync in an efficient way. I will reconsider the design to use raw files.

Re: @pmeenan Chrome opens the disk cache entry before sending the network request, and keeps the entry as active so that the entry would not be evicted. So 304 not-modified responses can be handled correctly even if the eviction logic has been triggered. https://source.chromium.org/chromium/chromium/src/+/main:net/http/http_cache_transaction.cc;l=734;drc=e672a665ffa8fe4901184f03922e2cc548399da5 I think we should not block the browser from sending the request while opening the dictionary file.

yoavweiss commented 1 year ago

Would it help if we added a requirement that dictionaries must be immutable? We could then maybe tweak the eviction logic to make sure they don't get evicted while there's a request that relies on them in flight

horo-t commented 1 year ago

Adding such a requirement sounds good to me.

By the way, @nhiroki told me that using APP_CACHE CacheType can prevent the auto eviction. So I think I can use DiskCache as a storage for the shared dictionary, just like Service Worker scripts and CacheStorage.