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

Issue 14129004: Add support for the control group in the Simple Cache Experiment. (Closed)

Created:
7 years, 8 months ago by gavinp
Modified:
7 years, 8 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, gavinp+disk_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Add support for the control group in the Simple Cache Experiment. The simple cache experiment control group will be "version 3" of the cache format, enforced on the index. This will cause users to drop their blockfile backend on startup in the control group, but keep their backend from session to session. R=felipeg@chromium.org,pasko@chromium.org BUG=231054 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=195399

Patch Set 1 #

Patch Set 2 : add tests #

Patch Set 3 : don't change chrome experiment #

Patch Set 4 : fix upgrade #

Patch Set 5 : add test data #

Patch Set 6 : use experiments #

Total comments: 10

Patch Set 7 : remediate & fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -4 lines) Patch
M net/disk_cache/backend_impl.cc View 1 2 3 4 5 6 2 chunks +14 lines, -3 lines 0 comments Download
M net/disk_cache/experiments.h View 1 2 3 4 5 1 chunk +4 lines, -1 line 0 comments Download

Messages

Total messages: 12 (0 generated)
gavinp
Ricardo, I know you're working on version 3 of the blockfile cache, but as you ...
7 years, 8 months ago (2013-04-13 15:27:22 UTC) #1
gavinp
Also, not sure how test data will work with the try job. Here's hoping!
7 years, 8 months ago (2013-04-13 15:33:56 UTC) #2
gavinp
On 2013/04/13 15:33:56, gavinp wrote: > Also, not sure how test data will work with ...
7 years, 8 months ago (2013-04-13 15:34:21 UTC) #3
rvargas (doing something else)
Hold on... what is this? The simple cache experiment should not be touching the regular ...
7 years, 8 months ago (2013-04-16 02:31:02 UTC) #4
rvargas (doing something else)
On 2013/04/16 02:31:02, rvargas wrote: > Hold on... what is this? The simple cache experiment ...
7 years, 8 months ago (2013-04-16 02:37:23 UTC) #5
gavinp
PTAL Ricardo.
7 years, 8 months ago (2013-04-19 14:14:49 UTC) #6
gavinp
Test data doesn't always go to rietveld right. Here it is attached. On Fri, Apr ...
7 years, 8 months ago (2013-04-19 14:16:08 UTC) #7
rvargas (doing something else)
The change itself lgtm. https://codereview.chromium.org/14129004/diff/20001/net/disk_cache/backend_impl.cc File net/disk_cache/backend_impl.cc (right): https://codereview.chromium.org/14129004/diff/20001/net/disk_cache/backend_impl.cc#newcode88 net/disk_cache/backend_impl.cc:88: LOG(INFO) << "hi?"; remove https://codereview.chromium.org/14129004/diff/20001/net/disk_cache/backend_impl.cc#newcode91 ...
7 years, 8 months ago (2013-04-19 17:40:55 UTC) #8
pasko-google - do not use
fly by comments .. https://codereview.chromium.org/14129004/diff/20001/net/disk_cache/backend_unittest.cc File net/disk_cache/backend_unittest.cc (right): https://codereview.chromium.org/14129004/diff/20001/net/disk_cache/backend_unittest.cc#newcode1734 net/disk_cache/backend_unittest.cc:1734: ASSERT_TRUE(CopyTestCache("simple_cache_control")); coud we create this ...
7 years, 8 months ago (2013-04-19 18:00:30 UTC) #9
gavinp
I've remediated to review. Also, some manual testing uncovered I'd use the wrong experiment name ...
7 years, 8 months ago (2013-04-20 07:17:32 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/gavinp@chromium.org/14129004/31001
7 years, 8 months ago (2013-04-20 12:36:47 UTC) #11
commit-bot: I haz the power
7 years, 8 months ago (2013-04-20 17:55:42 UTC) #12
Message was sent while issue was closed.
Change committed as 195399

Powered by Google App Engine
This is Rietveld 408576698