Store zero-freshness + response non-conditionality in in-memory index |
||
Issue descriptionThis is a variant of the ideas in issue 292923: It turns out that the vast majority of the time when we can't conditionalize fetches, it's for resources that have zero freshness. The fact of zero-freshness is storable in only one bit; an another can store some of the common causes of non-conditionability, so it may make sense to store this information in the in-memory index, to make it really cheap to fail those cases, w/o reading in the entry. Sketching this out in https://codereview.chromium.org/2922973003/
,
Aug 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e63bba9024d3397e80e393aed842ba9957385796 commit e63bba9024d3397e80e393aed842ba9957385796 Author: morlovich <morlovich@chromium.org> Date: Wed Aug 30 16:29:02 2017 Interface for associating in-memory hint bytes with disk cache entries. Implementation for it in the mock cache (with off switch) The intent is to implement it in simple cache, and to use it in HttpCache::Transaction to avoid opening entries that would not be usable under present load flags due to caching-hostile headers. BUG= 729679 Review-Url: https://codereview.chromium.org/2958033002 Cr-Commit-Position: refs/heads/master@{#498491} [modify] https://crrev.com/e63bba9024d3397e80e393aed842ba9957385796/net/disk_cache/disk_cache.cc [modify] https://crrev.com/e63bba9024d3397e80e393aed842ba9957385796/net/disk_cache/disk_cache.h [modify] https://crrev.com/e63bba9024d3397e80e393aed842ba9957385796/net/http/mock_http_cache.cc [modify] https://crrev.com/e63bba9024d3397e80e393aed842ba9957385796/net/http/mock_http_cache.h
,
Aug 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0f701ce1c09dd85eb1c175aa953f8078b0ce8349 commit 0f701ce1c09dd85eb1c175aa953f8078b0ce8349 Author: morlovich <morlovich@chromium.org> Date: Wed Aug 30 19:21:25 2017 Implement in-memory byte hints in SimpleCache This is done by stealing a byte off the size field in the in-memory index. Dependent on: https://codereview.chromium.org/2958033002/ (and will be updated if the interface there changes). BUG= 729679 Review-Url: https://codereview.chromium.org/2957133002 Cr-Commit-Position: refs/heads/master@{#498566} [modify] https://crrev.com/0f701ce1c09dd85eb1c175aa953f8078b0ce8349/net/disk_cache/backend_unittest.cc [modify] https://crrev.com/0f701ce1c09dd85eb1c175aa953f8078b0ce8349/net/disk_cache/simple/simple_backend_impl.cc [modify] https://crrev.com/0f701ce1c09dd85eb1c175aa953f8078b0ce8349/net/disk_cache/simple/simple_backend_impl.h [modify] https://crrev.com/0f701ce1c09dd85eb1c175aa953f8078b0ce8349/net/disk_cache/simple/simple_backend_version.h [modify] https://crrev.com/0f701ce1c09dd85eb1c175aa953f8078b0ce8349/net/disk_cache/simple/simple_index.cc [modify] https://crrev.com/0f701ce1c09dd85eb1c175aa953f8078b0ce8349/net/disk_cache/simple/simple_index.h [modify] https://crrev.com/0f701ce1c09dd85eb1c175aa953f8078b0ce8349/net/disk_cache/simple/simple_index_file.cc [modify] https://crrev.com/0f701ce1c09dd85eb1c175aa953f8078b0ce8349/net/disk_cache/simple/simple_index_file.h [modify] https://crrev.com/0f701ce1c09dd85eb1c175aa953f8078b0ce8349/net/disk_cache/simple/simple_index_file_unittest.cc [modify] https://crrev.com/0f701ce1c09dd85eb1c175aa953f8078b0ce8349/net/disk_cache/simple/simple_index_unittest.cc [modify] https://crrev.com/0f701ce1c09dd85eb1c175aa953f8078b0ce8349/net/disk_cache/simple/simple_synchronous_entry.cc [modify] https://crrev.com/0f701ce1c09dd85eb1c175aa953f8078b0ce8349/net/disk_cache/simple/simple_version_upgrade.cc
,
Oct 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b6d2d88636508af23b09cec87bbbd57af4778407 commit b6d2d88636508af23b09cec87bbbd57af4778407 Author: Maks Orlovich <morlovich@chromium.org> Date: Fri Oct 27 12:09:21 2017 SimpleCache: Permit optimistic create while doom is pending. This is desirable if we use in memory hints to avoid opening files in CantConditionalize case --- in that case we still want a cache entry, so the simplest way of doing that is to Doom the old one and Create a new one, which before this change would block until Doom completes. This lets CreateEntry return an Entry object optimistically, as long as the create is the first operation after Doom. This works by creating the entry in STATE_IO_PENDING state, and transitioning away from that and actually starting creation I/O via an entries_pending_doom_ callback. (This may be also be me trying to be too clever, I am worried this may be a bit too cute) Bug: 729679 Change-Id: I210f7ae5686a1a04e0930ea5c13e67c81131bd1a Reviewed-on: https://chromium-review.googlesource.com/684855 Commit-Queue: Maks Orlovich <morlovich@chromium.org> Reviewed-by: Julia Tuttle <juliatuttle@chromium.org> Cr-Commit-Position: refs/heads/master@{#512151} [modify] https://crrev.com/b6d2d88636508af23b09cec87bbbd57af4778407/net/disk_cache/disk_cache_test_base.cc [modify] https://crrev.com/b6d2d88636508af23b09cec87bbbd57af4778407/net/disk_cache/entry_unittest.cc [modify] https://crrev.com/b6d2d88636508af23b09cec87bbbd57af4778407/net/disk_cache/simple/simple_backend_impl.cc [modify] https://crrev.com/b6d2d88636508af23b09cec87bbbd57af4778407/net/disk_cache/simple/simple_entry_impl.cc [modify] https://crrev.com/b6d2d88636508af23b09cec87bbbd57af4778407/net/disk_cache/simple/simple_entry_impl.h
,
Nov 9 2017
I wonder if the HttpCache should just doom unconditionalizable zero-freshness resources and hang onto the entry for a few minutes to allow for short-term back-forward navigation. That way they're not taking up space in the cache longer than necessary.
,
Nov 9 2017
Hmm, would have to be potentially things other than HTML / more than a singular resource, even if those are wayyy less likely to be CantConditionalize, but it's plausible that'd indeed be a better approach, and there is some level of tracking of existing entries in HCT level, after all. How hard would invalidation be, though? (The approach I've had in mind is actually quite straightforward...)
,
Nov 14 2017
Sorry, didn't see your response until now. What I suggested in #5 wouldn't quite work as I'd suggested. Once doomed, the entry can't be used again for back-forward navigation. So we'd have to doom after the timeout. What I'm looking for is to improve cache eviction as well as fast-fail. So, as a follow-up CL to https://chromium-review.googlesource.com/c/chromium/src/+/732841 we could change the eviction algorithm to first evict entries older than X minutes that have the HINT_UNUSABLE_PER_CACHING_HEADERS bit set. I think it'd be simpler (than marking the entries and changing the eviction algorithm) however to keep a set of entries in HttpCache::Transaction that are to be doomed either on ~HttpCache::Transaction or after X time has elapsed.
,
Nov 15 2017
The timeout thing is a nice idea, and one I am probably insufficiently friendly towards due to being nearly-done with the different approach, but I do think it does have downsides that are not just my mind's inertia --- in particular: 1) Would you cancel the timer if circumstances change? On new use? That would require some non-trivial bookkeeping. 2) It seems a little hard to reason about/keep stats on, due to how hands off it is from the use time. As for eviction idea... well, layering-wise a delegate would probably be desirable, though an API for one seems tricky... though we could experiment with a quick hack easily enough.
,
Nov 15 2017
Okay, after F2F discussion we agreed to stick with the bits in https://chromium-review.googlesource.com/c/chromium/src/+/732841 and punt evicting such resources to another day. Some options we have for eviction: 1) Consumer uses a delayed task to doom the entry 2) Backend does the delayed doom (e.g., cache_entry_->DoomLater(base::TimeDelta) 3) Backend takes a hint from the client that the resource is low priority and should be evicted first at next eviction.
,
Nov 15 2017
Side note: we don't seem to have any metrics for memory caches? Is that intentional due to their use in incognito?
,
Nov 15 2017
I don't actually know if we're trying to avoid collection in incognito or if it was too painful to double the UMA metrics.
,
Nov 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d036d702fce56b666ba6f1904721756f19c68b15 commit d036d702fce56b666ba6f1904721756f19c68b15 Author: Maks Orlovich <morlovich@chromium.org> Date: Fri Nov 17 15:51:18 2017 Use in-memory hints to avoid opening already expired non-conditionalizable entries. HttpCache.CantConditionalizeZeroFreshnessFromMemHint is logged along HttpCache.CantConditianlizeCause == ZERO_FRESHNESS to denote this working. Since this is implemented as dooming entries, it messes somewhat with the "open stale" feature on error page. It's my understanding that feature is not turned on, so this just relaxes tests for it to use things not eligible for the behavior change. Bug: 729679 Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: Ie9a489fb6fa53a76691615c789c7373650180de0 Reviewed-on: https://chromium-review.googlesource.com/732841 Commit-Queue: Maks Orlovich <morlovich@chromium.org> Reviewed-by: Mark Pearson <mpearson@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Randy Smith <rdsmith@chromium.org> Cr-Commit-Position: refs/heads/master@{#517405} [modify] https://crrev.com/d036d702fce56b666ba6f1904721756f19c68b15/chrome/browser/net/errorpage_browsertest.cc [add] https://crrev.com/d036d702fce56b666ba6f1904721756f19c68b15/chrome/test/data/nocache-with-etag.html [add] https://crrev.com/d036d702fce56b666ba6f1904721756f19c68b15/chrome/test/data/nocache-with-etag.html.mock-http-headers [modify] https://crrev.com/d036d702fce56b666ba6f1904721756f19c68b15/content/renderer/browser_render_view_browsertest.cc [add] https://crrev.com/d036d702fce56b666ba6f1904721756f19c68b15/content/test/data/nocache-with-etag.html [add] https://crrev.com/d036d702fce56b666ba6f1904721756f19c68b15/content/test/data/nocache-with-etag.html.mock-http-headers [modify] https://crrev.com/d036d702fce56b666ba6f1904721756f19c68b15/net/base/net_error_list.h [modify] https://crrev.com/d036d702fce56b666ba6f1904721756f19c68b15/net/http/http_cache.cc [modify] https://crrev.com/d036d702fce56b666ba6f1904721756f19c68b15/net/http/http_cache.h [modify] https://crrev.com/d036d702fce56b666ba6f1904721756f19c68b15/net/http/http_cache_transaction.cc [modify] https://crrev.com/d036d702fce56b666ba6f1904721756f19c68b15/net/http/http_cache_transaction.h [modify] https://crrev.com/d036d702fce56b666ba6f1904721756f19c68b15/net/http/http_cache_unittest.cc [modify] https://crrev.com/d036d702fce56b666ba6f1904721756f19c68b15/tools/metrics/histograms/histograms.xml
,
Nov 21 2017
Issue 292914 has been merged into this issue.
,
Mar 27 2018
|
||
►
Sign in to add a comment |
||
Comment 1 by morlovich@chromium.org
, Jun 5 2017