Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(361)

Issue 1401053004: Revert of Implement cache counting for the simple and memory backends. (Closed)

Created:
5 years, 2 months ago by Mark P
Modified:
5 years, 2 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, gavinp+disk_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Implement cache counting for the simple and memory backends. (patchset #3 id:40001 of https://codereview.chromium.org/1398053002/ ) Reason for revert: Causes test failures on XP bot: https://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%281%29/builds/40465/steps/net_unittests%20on%20Windows-XP-SP3/logs/DiskCacheBackendTest.SimpleCacheCalculateSizeOfAllEntries DiskCacheBackendTest.SimpleCacheCalculateSizeOfAllEntries (run #1): [ RUN ] DiskCacheBackendTest.SimpleCacheCalculateSizeOfAllEntries c:uild\slave\win_builderuild\src et\disk_cacheackend_unittest.cc(1730): error: Value of: result Actual: 7447 Expected: (count - 1) * count / 2 + total_metadata_size Which is: 7455 c:uild\slave\win_builderuild\src et\disk_cacheackend_unittest.cc(1747): error: Value of: new_result Actual: 8243 Expected: result + last_entry_size + GetEntryMetadataSize(key) Which is: 8235 c:uild\slave\win_builderuild\src et\disk_cacheackend_unittest.cc(1751): error: Value of: new_result Actual: 7455 Expected: result Which is: 7447 [ FAILED ] DiskCacheBackendTest.SimpleCacheCalculateSizeOfAllEntries (171 ms) Original issue's description: > Implement cache counting for the simple and memory backends. > > This is a followup to https://codereview.chromium.org/1304363013/, which implemented it for the blockfile backend. > > BUG=510028 > > Committed: https://crrev.com/a319545886d0e297b88301f01b804432bea602f7 > Cr-Commit-Position: refs/heads/master@{#354037} TBR=gavinp@chromium.org,pasko@chromium.org,ttuttle@chromium.org,msramek@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=510028 Committed: https://crrev.com/81c61dd0811b103aeca08e1a5184db725fdb19fb Cr-Commit-Position: refs/heads/master@{#354118}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+54 lines, -68 lines) Patch
M net/disk_cache/backend_unittest.cc View 6 chunks +37 lines, -31 lines 0 comments Download
M net/disk_cache/memory/mem_backend_impl.cc View 1 chunk +2 lines, -1 line 0 comments Download
M net/disk_cache/simple/simple_backend_impl.h View 1 chunk +0 lines, -4 lines 0 comments Download
M net/disk_cache/simple/simple_backend_impl.cc View 3 chunks +15 lines, -23 lines 0 comments Download
M net/disk_cache/simple/simple_index.h View 1 chunk +0 lines, -4 lines 0 comments Download
M net/disk_cache/simple/simple_index.cc View 1 chunk +0 lines, -5 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Mark P
Created Revert of Implement cache counting for the simple and memory backends.
5 years, 2 months ago (2015-10-14 21:33:55 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1401053004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1401053004/1
5 years, 2 months ago (2015-10-14 21:34:41 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 2 months ago (2015-10-14 21:35:59 UTC) #3
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/81c61dd0811b103aeca08e1a5184db725fdb19fb Cr-Commit-Position: refs/heads/master@{#354118}
5 years, 2 months ago (2015-10-14 21:37:01 UTC) #4
msramek
5 years, 2 months ago (2015-10-15 09:49:00 UTC) #5
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1401923006/ by msramek@chromium.org.

The reason for reverting is: The original CL was reverted because of a failing
unittest. The results in the unittest are off by 8 bytes. 8 bytes is the size of
the second-to-last entry, which is the last one where we wrote to the stream #1.
As the comment in this unittest already mentions, writing to the stream #1
triggers another operation that is asynchronous and can possibly happen after
the call to CalculateSizeOfAllEntries. Apparently, this didn't happen on Linux,
but it did happen on Windows.

The test can be fixed by only writing to the stream #0..

Powered by Google App Engine
This is Rietveld 408576698