evict larger entries first

net/disk_cache/simple/simple_experiment.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/disk_cache/simple/simple_experiment.h"
 
 #include <map>
 #include <string>
 
 #include "base/metrics/field_trial.h"
 #include "base/metrics/field_trial_param_associator.h"
 #include "base/strings/string_number_conversions.h"
 
 namespace disk_cache {
 
 const base::Feature kSimpleSizeExperiment = {"SimpleSizeExperiment",
                                              base::FEATURE_DISABLED_BY_DEFAULT};
+const base::Feature kSimpleCacheEvictionWithSizeExperiment = {
+    "SimpleCacheEvictionWithSizeExperiment", base::FEATURE_DISABLED_BY_DEFAULT};
+
 const char kSizeMultiplierParam[] = "SizeMultiplier";
+const char kSizeEvictionParam[] = "SizeEviction";
 
 namespace {
 
+// Updates experiment if the experiment is active.
+bool CheckForEvictionWithSizeExperiment(
+    disk_cache::SimpleExperiment* experiment) {
+  if (disk_cache::SimpleExperimentType::NONE != experiment->type)

[jkarlin, 2017/07/12 15:51:40]

After returning early in GetSimpleExperiment you can turn this into a DCHECK

[hubbe, 2017/07/12 18:22:50]

Done.

+    return false;
+
+  if (!base::FeatureList::IsEnabled(kSimpleCacheEvictionWithSizeExperiment))
+    return false;
+
+  base::FieldTrial* trial =
+      base::FeatureList::GetFieldTrial(kSimpleCacheEvictionWithSizeExperiment);
+  if (!trial)
+    return false;
+
+  std::map<std::string, std::string> params;
+  base::FieldTrialParamAssociator::GetInstance()->GetFieldTrialParams(
+      trial->trial_name(), &params);
+  auto iter = params.find(kSizeEvictionParam);
+  if (iter == params.end())
+    return false;
+
+  uint32_t param;
+  if (!base::StringToUint(iter->second, &param))
+    return false;
+
+  experiment->type = disk_cache::SimpleExperimentType::SIZE;

[jkarlin, 2017/07/12 15:51:41]

I believe you want EVICT_WITH_SIZE here ;) That would have been caught with a
unittest. Please add relevant tests to simple_experiment_unittest.cc

[hubbe, 2017/07/12 18:22:50]

Done.

+  experiment->param = param;
+  return true;
+}
+
 // Returns true if the experiment is found and properly defined.
 bool CheckForSimpleSizeExperiment(disk_cache::SimpleExperiment* experiment) {
   DCHECK_EQ(disk_cache::SimpleExperimentType::NONE, experiment->type);
   DCHECK_EQ(0u, experiment->param);
 
   if (!base::FeatureList::IsEnabled(kSimpleSizeExperiment))
     return false;
 
   base::FieldTrial* trial =
       base::FeatureList::GetFieldTrial(kSimpleSizeExperiment);
@@ skipping 18 matching lines @@
 
 }  // namespace
 
 // Returns the experiment for the given |cache_type|.
 SimpleExperiment GetSimpleExperiment(net::CacheType cache_type) {
   SimpleExperiment experiment;
 
   if (cache_type != net::DISK_CACHE)
     return experiment;
 
   CheckForSimpleSizeExperiment(&experiment);

[jkarlin, 2017/07/12 15:51:41]

Can you have each check return early if they return true to give us clear
semantics that the first matching experiment wins.

[hubbe, 2017/07/12 18:22:50]

Done.

+  CheckForEvictionWithSizeExperiment(&experiment);
   return experiment;
 }
 
 }  // namespace disk_cache