New issue
Advanced search Search tips

Issue 905351 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 1
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 2
Type: Bug

Blocking:
issue 853518



Sign in to add a comment

consider adding a simple disk_cache mode that disables access time tracking

Project Member Reported by wanderview@chromium.org, Nov 14

Issue description

When cache_storage uses simple disk_cache it effectively disables the automatic eviction.  The simple disk_cache code, though, still tracks when each entry is accessed in order to feed the eviction code.  This includes writing out the index file periodically to save these access times.

Many uses of cache_storage will be purely read-only.  The call cache.match(), but don't put any new entries into the disk_cache.  In these cases we should be able to avoid all disk writes.

To implement this we could set the timestamp when the entry is created, but perhaps not update it when accessing the entry.  We would also then need to track if the index is dirty or not to avoid spurious writes when the disk_cache is closed.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 1

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

commit 8a7e96202600b55809e87a1611fb905468b80a85
Author: Ben Kelly <wanderview@chromium.org>
Date: Sat Dec 01 18:39:15 2018

SimpleCache: Avoid unnecessary index writes in APP_CACHE mode.

When a simple disk_cache is in APP_CACHE mode eviction is disabled and
access times do not need to be tracked.  This means that it should be
possible to perform read-only operations without triggering any index
disk writes.

This CL implements this optimization by making the following changes:

* Avoid updating access times in APP_CACHE mode.
* Avoid writing a clean index to disk when closing the backend.
* Avoid dirtying the index when a size update does not make a change.
* Avoid dirtying the index when opening an entry that is in the index.
* Avoid dirtying the index when dooming an entry not in the index.

With these changes its possible to use simple disk_cache to perform
read-only operations in cache_storage without any index writes.

Bug:  905351 
Change-Id: I8eed0a72ed166e7b1b3623a7f4c17f7cec7f96d4
Reviewed-on: https://chromium-review.googlesource.com/c/1351539
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612934}
[modify] https://crrev.com/8a7e96202600b55809e87a1611fb905468b80a85/net/disk_cache/simple/simple_backend_impl.cc
[modify] https://crrev.com/8a7e96202600b55809e87a1611fb905468b80a85/net/disk_cache/simple/simple_entry_impl.cc
[modify] https://crrev.com/8a7e96202600b55809e87a1611fb905468b80a85/net/disk_cache/simple/simple_entry_impl.h
[modify] https://crrev.com/8a7e96202600b55809e87a1611fb905468b80a85/net/disk_cache/simple/simple_index.cc
[modify] https://crrev.com/8a7e96202600b55809e87a1611fb905468b80a85/net/disk_cache/simple/simple_index.h
[modify] https://crrev.com/8a7e96202600b55809e87a1611fb905468b80a85/net/disk_cache/simple/simple_index_unittest.cc

Blocking: 853518
Status: Fixed (was: Assigned)
Labels: Merge-Request-72 OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
I'd like to merge this to M72 to support an impacted partner site ASAP.  The CL includes unit tests.
Project Member

Comment 5 by sheriffbot@chromium.org, Dec 4

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 4

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2155b16fe4124e40e1b43eb0710574f624cb4b9d

commit 2155b16fe4124e40e1b43eb0710574f624cb4b9d
Author: Ben Kelly <wanderview@chromium.org>
Date: Tue Dec 04 15:37:57 2018

SimpleCache: Avoid unnecessary index writes in APP_CACHE mode.

When a simple disk_cache is in APP_CACHE mode eviction is disabled and
access times do not need to be tracked.  This means that it should be
possible to perform read-only operations without triggering any index
disk writes.

This CL implements this optimization by making the following changes:

* Avoid updating access times in APP_CACHE mode.
* Avoid writing a clean index to disk when closing the backend.
* Avoid dirtying the index when a size update does not make a change.
* Avoid dirtying the index when opening an entry that is in the index.
* Avoid dirtying the index when dooming an entry not in the index.

With these changes its possible to use simple disk_cache to perform
read-only operations in cache_storage without any index writes.

Bug:  905351 
Change-Id: I8eed0a72ed166e7b1b3623a7f4c17f7cec7f96d4
Reviewed-on: https://chromium-review.googlesource.com/c/1351539
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612934}(cherry picked from commit 8a7e96202600b55809e87a1611fb905468b80a85)
Reviewed-on: https://chromium-review.googlesource.com/c/1361322
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#27}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/2155b16fe4124e40e1b43eb0710574f624cb4b9d/net/disk_cache/simple/simple_backend_impl.cc
[modify] https://crrev.com/2155b16fe4124e40e1b43eb0710574f624cb4b9d/net/disk_cache/simple/simple_entry_impl.cc
[modify] https://crrev.com/2155b16fe4124e40e1b43eb0710574f624cb4b9d/net/disk_cache/simple/simple_entry_impl.h
[modify] https://crrev.com/2155b16fe4124e40e1b43eb0710574f624cb4b9d/net/disk_cache/simple/simple_index.cc
[modify] https://crrev.com/2155b16fe4124e40e1b43eb0710574f624cb4b9d/net/disk_cache/simple/simple_index.h
[modify] https://crrev.com/2155b16fe4124e40e1b43eb0710574f624cb4b9d/net/disk_cache/simple/simple_index_unittest.cc

📍 Couldn't reproduce a difference.
https://pinpoint-dot-chromeperf.appspot.com/job/134f86c2140000
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/2155b16fe4124e40e1b43eb0710574f624cb4b9d

Commit: 2155b16fe4124e40e1b43eb0710574f624cb4b9d
Author: wanderview@chromium.org
Commiter: wanderview@chromium.org
Date: 2018-12-04 15:37:57 +0000 UTC

SimpleCache: Avoid unnecessary index writes in APP_CACHE mode.

When a simple disk_cache is in APP_CACHE mode eviction is disabled and
access times do not need to be tracked.  This means that it should be
possible to perform read-only operations without triggering any index
disk writes.

This CL implements this optimization by making the following changes:

* Avoid updating access times in APP_CACHE mode.
* Avoid writing a clean index to disk when closing the backend.
* Avoid dirtying the index when a size update does not make a change.
* Avoid dirtying the index when opening an entry that is in the index.
* Avoid dirtying the index when dooming an entry not in the index.

With these changes its possible to use simple disk_cache to perform
read-only operations in cache_storage without any index writes.

Bug:  905351 
Change-Id: I8eed0a72ed166e7b1b3623a7f4c17f7cec7f96d4
Reviewed-on: https://chromium-review.googlesource.com/c/1351539
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612934}(cherry picked from commit 8a7e96202600b55809e87a1611fb905468b80a85)
Reviewed-on: https://chromium-review.googlesource.com/c/1361322
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#27}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment