Skip to content

Commit

Permalink
Enable experimentation with different disk cache sizes
Browse files Browse the repository at this point in the history
This CL adds the code to enable experimentation of different cache sizes.
Now that cache will be partitioned, it makes sense to see if increasing
the cache size helps offset some performance impact by lowering the
eviction rate.

The CL brings back similar logic as was removed using
https://chromium-review.googlesource.com/c/chromium/src/+/1944285

Bug: 1160690
Change-Id: Ia774a8788caeb693a36b49050289161cd6843b93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2589117
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839682}
  • Loading branch information
shivanigithub authored and Chromium LUCI CQ committed Dec 30, 2020
1 parent 07e3277 commit 19b2f34
Show file tree
Hide file tree
Showing 9 changed files with 273 additions and 18 deletions.
50 changes: 50 additions & 0 deletions chrome/browser/net/profile_network_context_service_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include "mojo/public/cpp/system/data_pipe_utils.h"
#include "net/base/features.h"
#include "net/base/load_flags.h"
#include "net/disk_cache/cache_util.h"
#include "net/http/http_auth_preferences.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/http_request.h"
Expand Down Expand Up @@ -92,6 +93,19 @@ class ProfileNetworkContextServiceBrowsertest : public InProcessBrowserTest {
return loader_factory_;
}

void CheckDiskCacheSizeHistogramRecorded() {
std::string all_metrics;
do {
content::FetchHistogramsFromChildProcesses();
metrics::SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(5));
all_metrics = histograms_.GetAllHistogramsRecorded();
} while (std::string::npos ==
all_metrics.find("HttpCache.MaxFileSizeOnInit"));
}

base::HistogramTester histograms_;

protected:
// The HttpCache is only created when a request is issued, thus we perform a
// navigation to ensure that the http cache is initialized.
Expand Down Expand Up @@ -143,6 +157,42 @@ IN_PROC_BROWSER_TEST_F(ProfileNetworkContextServiceBrowsertest,
/*in_memory=*/false, empty_relative_partition_path,
&network_context_params, &cert_verifier_creation_params);
EXPECT_EQ(0, network_context_params.http_cache_max_size);

CheckDiskCacheSizeHistogramRecorded();
}

class DiskCachesizeExperiment : public ProfileNetworkContextServiceBrowsertest {
public:
DiskCachesizeExperiment() = default;
~DiskCachesizeExperiment() override = default;

void SetUp() override {
std::map<std::string, std::string> field_trial_params;
field_trial_params["percent_relative_size"] = "200";
feature_list_.InitAndEnableFeatureWithParameters(
disk_cache::kChangeDiskCacheSizeExperiment, field_trial_params);
ProfileNetworkContextServiceBrowsertest::SetUp();
}

private:
base::test::ScopedFeatureList feature_list_;
};

IN_PROC_BROWSER_TEST_F(DiskCachesizeExperiment, ScaledCacheSize) {
// We don't have a great way of directly checking that the disk cache has the
// correct max size, but we can make sure that we set up our network context
// params correctly and that the histogram is recorded.
ProfileNetworkContextService* profile_network_context_service =
ProfileNetworkContextServiceFactory::GetForContext(browser()->profile());
base::FilePath empty_relative_partition_path;
network::mojom::NetworkContextParams network_context_params;
network::mojom::CertVerifierCreationParams cert_verifier_creation_params;
profile_network_context_service->ConfigureNetworkContextParams(
/*in_memory=*/false, empty_relative_partition_path,
&network_context_params, &cert_verifier_creation_params);
EXPECT_EQ(0, network_context_params.http_cache_max_size);

CheckDiskCacheSizeHistogramRecorded();
}

IN_PROC_BROWSER_TEST_F(ProfileNetworkContextServiceBrowsertest, BrotliEnabled) {
Expand Down
22 changes: 21 additions & 1 deletion net/disk_cache/backend_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4570,7 +4570,7 @@ TEST_F(DiskCacheBackendTest, SimpleCacheNegMaxSize) {
InitCache();
// We don't know what it will pick, but it's limited to what
// disk_cache::PreferredCacheSize would return, scaled by the size experiment,
// which only goes as much as 2x. It definitely should not be MAX_UINT64.
// which only goes as much as 4x. It definitely should not be MAX_UINT64.
EXPECT_NE(simple_cache_impl_->index()->max_size(),
std::numeric_limits<uint64_t>::max());

Expand All @@ -4580,6 +4580,26 @@ TEST_F(DiskCacheBackendTest, SimpleCacheNegMaxSize) {
ASSERT_GE(max_default_size, 0);
EXPECT_LT(simple_cache_impl_->index()->max_size(),
static_cast<unsigned>(max_default_size));

uint64_t max_size_without_scaling = simple_cache_impl_->index()->max_size();

// Scale to 200%. The size should be twice of |max_size_without_scaling| but
// since that's capped on 20% of available size, checking for the size to be
// between max_size_without_scaling and max_size_without_scaling*2.
{
base::test::ScopedFeatureList scoped_feature_list;
std::map<std::string, std::string> field_trial_params;
field_trial_params["percent_relative_size"] = "200";
scoped_feature_list.InitAndEnableFeatureWithParameters(
disk_cache::kChangeDiskCacheSizeExperiment, field_trial_params);

InitCache();

uint64_t max_size_scaled = simple_cache_impl_->index()->max_size();

EXPECT_GE(max_size_scaled, max_size_without_scaling);
EXPECT_LE(max_size_scaled, 2 * max_size_without_scaling);
}
}

TEST_F(DiskCacheBackendTest, SimpleLastModified) {
Expand Down
39 changes: 37 additions & 2 deletions net/disk_cache/cache_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/files/file_enumerator.h"
#include "base/files/file_util.h"
#include "base/location.h"
#include "base/metrics/field_trial_params.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
Expand Down Expand Up @@ -85,6 +86,9 @@ namespace disk_cache {

const int kDefaultCacheSize = 80 * 1024 * 1024;

const base::Feature kChangeDiskCacheSizeExperiment{
"ChangeDiskCacheSize", base::FEATURE_DISABLED_BY_DEFAULT};

void DeleteCache(const base::FilePath& path, bool remove_folder) {
if (remove_folder) {
if (!base::DeletePathRecursively(path))
Expand Down Expand Up @@ -152,19 +156,50 @@ bool DelayedCacheCleanup(const base::FilePath& full_path) {
// Returns the preferred maximum number of bytes for the cache given the
// number of available bytes.
int PreferredCacheSize(int64_t available, net::CacheType type) {
// Percent of cache size to use, relative to the default size. "100" means to
// use 100% of the default size.
int percent_relative_size = 100;

if (base::FeatureList::IsEnabled(
disk_cache::kChangeDiskCacheSizeExperiment) &&
type == net::DISK_CACHE) {
percent_relative_size = base::GetFieldTrialParamByFeatureAsInt(
disk_cache::kChangeDiskCacheSizeExperiment, "percent_relative_size",
100 /* default value */);
}

// Cap scaling, as a safety check, to avoid overflow.
if (percent_relative_size > 400)
percent_relative_size = 400;
else if (percent_relative_size < 100)
percent_relative_size = 100;

int64_t scaled_default_disk_cache_size =
(static_cast<int64_t>(disk_cache::kDefaultCacheSize) *
percent_relative_size) /
100;

if (available < 0)
return kDefaultCacheSize;
return static_cast<int32_t>(scaled_default_disk_cache_size);

int64_t preferred_cache_size = PreferredCacheSizeInternal(available);

// If the preferred cache size is less than 20% of the available space, scale
// for the field trial, capping the scaled value at 20% of the available
// space.
if (preferred_cache_size < available / 5) {
preferred_cache_size = std::min(
(preferred_cache_size * percent_relative_size) / 100, available / 5);
}

// Limit cache size to somewhat less than kint32max to avoid potential
// integer overflows in cache backend implementations.
//
// Note: the 4x limit is of course far below that; historically it came
// from the blockfile backend with the following explanation:
// "Let's not use more than the default size while we tune-up the performance
// of bigger caches. "
int64_t size_limit = static_cast<int64_t>(kDefaultCacheSize) * 4;
int64_t size_limit = scaled_default_disk_cache_size * 4;
// Native code entries can be large, so we would like a larger cache.
// Make the size limit 50% larger in that case.
if (type == net::GENERATED_NATIVE_CODE_CACHE) {
Expand Down
4 changes: 4 additions & 0 deletions net/disk_cache/cache_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ class FilePath;

namespace disk_cache {

// Experiment to increase the cache size to see the impact on various
// performance metrics.
NET_EXPORT_PRIVATE extern const base::Feature kChangeDiskCacheSizeExperiment;

// Moves the cache files from the given path to another location.
// Fails if the destination exists already, or if it doesn't have
// permission for the operation. This is basically a rename operation
Expand Down
100 changes: 86 additions & 14 deletions net/disk_cache/cache_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

#include "base/files/file_util.h"
#include "base/files/scoped_temp_dir.h"
#include "base/strings/string_number_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "build/chromeos_buildflags.h"
#include "net/disk_cache/cache_util.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -101,45 +103,115 @@ TEST_F(CacheUtilTest, DeleteCacheFile) {
TEST_F(CacheUtilTest, PreferredCacheSize) {
const struct TestCase {
int64_t available;
int expected;
int expected_without_trial;
int expected_with_200_trial;
int expected_with_250_trial;
int expected_with_300_trial;
} kTestCases[] = {
// Weird negative value for available --- return the "default"
{-1000LL, 80 * 1024 * 1024},
{-1LL, 80 * 1024 * 1024},
{-1000LL, 80 * 1024 * 1024, 160 * 1024 * 1024, 200 * 1024 * 1024,
240 * 1024 * 1024},
{-1LL, 80 * 1024 * 1024, 160 * 1024 * 1024, 200 * 1024 * 1024,
240 * 1024 * 1024},

// 0 produces 0.
{0LL, 0},
{0LL, 0, 0, 0, 0},

// Cache is 80% of available space, when default cache size is larger than
// 80% of available space..
{50 * 1024 * 1024LL, 40 * 1024 * 1024},
{50 * 1024 * 1024LL, 40 * 1024 * 1024, 40 * 1024 * 1024, 40 * 1024 * 1024,
40 * 1024 * 1024},
// Cache is default size, when default size is 10% to 80% of available
// space.
{100 * 1024 * 1024LL, 80 * 1024 * 1024},
{200 * 1024 * 1024LL, 80 * 1024 * 1024},
{100 * 1024 * 1024LL, 80 * 1024 * 1024, 80 * 1024 * 1024,
80 * 1024 * 1024, 80 * 1024 * 1024},
{200 * 1024 * 1024LL, 80 * 1024 * 1024, 80 * 1024 * 1024,
80 * 1024 * 1024, 80 * 1024 * 1024},
// Cache is 10% of available space if 2.5 * default size is more than 10%
// of available space.
{1000 * 1024 * 1024LL, 100 * 1024 * 1024},
{2000 * 1024 * 1024LL, 200 * 1024 * 1024},
{1000 * 1024 * 1024LL, 100 * 1024 * 1024, 200 * 1024 * 1024,
200 * 1024 * 1024, 200 * 1024 * 1024},
{2000 * 1024 * 1024LL, 200 * 1024 * 1024, 400 * 1024 * 1024,
400 * 1024 * 1024, 400 * 1024 * 1024},
// Cache is 2.5 * kDefaultCacheSize if 2.5 * kDefaultCacheSize uses from
// 1% to 10% of available space.
{10000 * 1024 * 1024LL, 200 * 1024 * 1024},
{10000 * 1024 * 1024LL, 200 * 1024 * 1024, 400 * 1024 * 1024,
500 * 1024 * 1024, 600 * 1024 * 1024},
// Otherwise, cache is 1% of available space.
{20000 * 1024 * 1024LL, 200 * 1024 * 1024},
{20000 * 1024 * 1024LL, 200 * 1024 * 1024, 400 * 1024 * 1024,
500 * 1024 * 1024, 600 * 1024 * 1024},
// Until it runs into the cache size cap.
{32000 * 1024 * 1024LL, 320 * 1024 * 1024},
{50000 * 1024 * 1024LL, 320 * 1024 * 1024},
{32000 * 1024 * 1024LL, 320 * 1024 * 1024, 640 * 1024 * 1024,
800 * 1024 * 1024, 960 * 1024 * 1024},
{50000 * 1024 * 1024LL, 320 * 1024 * 1024, 640 * 1024 * 1024,
800 * 1024 * 1024, 960 * 1024 * 1024},
};

for (const auto& test_case : kTestCases) {
EXPECT_EQ(test_case.expected, PreferredCacheSize(test_case.available))
EXPECT_EQ(test_case.expected_without_trial,
PreferredCacheSize(test_case.available))
<< test_case.available;
}

// Check that the cache size cap is 50% higher for native code caches.
EXPECT_EQ(((320 * 1024 * 1024) / 2) * 3,
PreferredCacheSize(50000 * 1024 * 1024LL,
net::GENERATED_NATIVE_CODE_CACHE));

for (int cache_size_exeriment : {100, 200, 250, 300}) {
base::test::ScopedFeatureList scoped_feature_list;
std::map<std::string, std::string> field_trial_params;
field_trial_params["percent_relative_size"] =
base::NumberToString(cache_size_exeriment);
scoped_feature_list.InitAndEnableFeatureWithParameters(
disk_cache::kChangeDiskCacheSizeExperiment, field_trial_params);

for (const auto& test_case : kTestCases) {
int expected = 0;
switch (cache_size_exeriment) {
case 100:
expected = test_case.expected_without_trial;
break;
case 200:
expected = test_case.expected_with_200_trial;
break;
case 250:
expected = test_case.expected_with_250_trial;
break;
case 300:
expected = test_case.expected_with_300_trial;
break;
}

EXPECT_EQ(expected, PreferredCacheSize(test_case.available));

// For caches other than disk cache, the size is not scaled.
EXPECT_EQ(test_case.expected_without_trial,
PreferredCacheSize(test_case.available,
net::GENERATED_BYTE_CODE_CACHE));
}

// Check that the cache size cap is 50% higher for native code caches but is
// not scaled for the experiment.
EXPECT_EQ(((320 * 1024 * 1024) / 2) * 3,
PreferredCacheSize(50000 * 1024 * 1024LL,
net::GENERATED_NATIVE_CODE_CACHE));
}

// Check no "percent_relative_size" matches default behavior.
{
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(
disk_cache::kChangeDiskCacheSizeExperiment);
for (const auto& test_case : kTestCases) {
EXPECT_EQ(test_case.expected_without_trial,
PreferredCacheSize(test_case.available));
}
// Check that the cache size cap is 50% higher for native code caches.
EXPECT_EQ(((320 * 1024 * 1024) / 2) * 3,
PreferredCacheSize(50000 * 1024 * 1024LL,
net::GENERATED_NATIVE_CODE_CACHE));
}
}

} // namespace disk_cache
2 changes: 2 additions & 0 deletions net/http/http_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1499,6 +1499,8 @@ void HttpCache::OnBackendCreated(int result, PendingOp* pending_op) {
backend_factory_.reset(); // Reclaim memory.
if (result == OK) {
disk_cache_ = std::move(pending_op->backend);
UMA_HISTOGRAM_MEMORY_KB("HttpCache.MaxFileSizeOnInit",
disk_cache_->MaxFileSize() / 1024);
}
}

Expand Down
Loading

0 comments on commit 19b2f34

Please sign in to comment.