Skip to content

Commit

Permalink
Add threshold parameters for blink memory cache strong reference
Browse files Browse the repository at this point in the history
This CL introduces two thresholds:
* The total decoded size that the memory cache can keep strong
  references.
* The maximum decoded size that the memory cache can keep a strong
  reference to a resource.

Bug: 1409349
Change-Id: Ib94b96d4bd5733489408734fbe856cc380e0c97c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4736941
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1188242}
  • Loading branch information
bashi authored and Chromium LUCI CQ committed Aug 25, 2023
1 parent bec8a26 commit 0d63ed5
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 7 deletions.
8 changes: 8 additions & 0 deletions third_party/blink/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,14 @@ BASE_FEATURE(kMemoryCacheStrongReferenceFilterScripts,
BASE_FEATURE(kMemoryCacheStrongReference,
"MemoryCacheStrongReference",
base::FEATURE_DISABLED_BY_DEFAULT);
const base::FeatureParam<int>
kMemoryCacheStrongReferenceTotalSizeThresholdParam{
&kMemoryCacheStrongReference,
"memory_cache_strong_ref_total_size_threshold", 10 * 1024 * 1024};
const base::FeatureParam<int>
kMemoryCacheStrongReferenceResourceSizeThresholdParam{
&kMemoryCacheStrongReference,
"memory_cache_strong_ref_resource_size_threshold", 3 * 1024 * 1024};

BASE_FEATURE(kMemoryCacheStrongReferenceSingleUnload,
"MemoryCacheStrongReferenceSingleUnload",
Expand Down
8 changes: 8 additions & 0 deletions third_party/blink/public/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,14 @@ BLINK_COMMON_EXPORT extern const base::FeatureParam<int>
// Keep strong references in the blink memory cache to maximize resource reuse.
// See https://crbug.com/1409349.
BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kMemoryCacheStrongReference);
// The threshold for the total decoded size of resources that keep strong
// references.
BLINK_COMMON_EXPORT extern const base::FeatureParam<int>
kMemoryCacheStrongReferenceTotalSizeThresholdParam;
// The threshold for the decoded size of a resource that can keep a strong
// reference.
BLINK_COMMON_EXPORT extern const base::FeatureParam<int>
kMemoryCacheStrongReferenceResourceSizeThresholdParam;
// Exclude images from the saved strong references for resources.
// See https://crbug.com/1409349.
BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ bool ShouldResourceBeKeptStrongReferenceByType(Resource* resource) {
!base::FeatureList::IsEnabled(
features::kMemoryCacheStrongReferenceFilterScripts)) ||
resource->GetType() == ResourceType::kFont ||
resource->GetType() == ResourceType::kCSSStyleSheet;
resource->GetType() == ResourceType::kCSSStyleSheet ||
resource->GetType() == ResourceType::kMock; // For tests.
}

bool ShouldResourceBeKeptStrongReference(Resource* resource) {
Expand Down Expand Up @@ -741,6 +742,12 @@ Resource* ResourceFetcher::CachedResource(const KURL& resource_url) const {
return it->value.Get();
}

const HeapHashSet<Member<Resource>>
ResourceFetcher::MoveResourceStrongReferences() {
document_resource_strong_refs_total_size_ = 0;
return std::move(document_resource_strong_refs_);
}

mojom::ControllerServiceWorkerMode
ResourceFetcher::IsControlledByServiceWorker() const {
return properties_->GetControllerServiceWorkerMode();
Expand Down Expand Up @@ -1463,11 +1470,19 @@ Resource* ResourceFetcher::RequestResource(FetchParameters& params,
}

void ResourceFetcher::RemoveResourceStrongReference(Resource* resource) {
if (resource) {
if (resource && document_resource_strong_refs_.Contains(resource)) {
const size_t resource_size =
static_cast<size_t>(resource->GetResponse().DecodedBodyLength());
document_resource_strong_refs_.erase(resource);
CHECK_GE(document_resource_strong_refs_total_size_, resource_size);
document_resource_strong_refs_total_size_ -= resource_size;
}
}

bool ResourceFetcher::HasStrongReferenceForTesting(Resource* resource) {
return document_resource_strong_refs_.Contains(resource);
}

void ResourceFetcher::ResourceTimingReportTimerFired(TimerBase* timer) {
DCHECK_EQ(timer, &resource_timing_report_timer_);
Vector<ScheduledResourceTimingInfo> timing_reports;
Expand Down Expand Up @@ -2806,9 +2821,20 @@ void ResourceFetcher::CancelWebBundleSubresourceLoadersFor(
}

void ResourceFetcher::MaybeSaveResourceToStrongReference(Resource* resource) {
static const size_t total_size_threshold = static_cast<size_t>(
features::kMemoryCacheStrongReferenceTotalSizeThresholdParam.Get());
static const size_t resource_size_threshold = static_cast<size_t>(
features::kMemoryCacheStrongReferenceResourceSizeThresholdParam.Get());
const size_t resource_size =
static_cast<size_t>(resource->GetResponse().DecodedBodyLength());
const bool size_is_small_enough = resource_size <= resource_size_threshold &&
resource_size <= total_size_threshold &&
document_resource_strong_refs_total_size_ <=
total_size_threshold - resource_size;
if (base::FeatureList::IsEnabled(features::kMemoryCacheStrongReference) &&
ShouldResourceBeKeptStrongReference(resource)) {
ShouldResourceBeKeptStrongReference(resource) && size_is_small_enough) {
document_resource_strong_refs_.insert(resource);
document_resource_strong_refs_total_size_ += resource_size;
freezable_task_runner_->PostDelayedTask(
FROM_HERE,
WTF::BindOnce(&ResourceFetcher::RemoveResourceStrongReference,
Expand All @@ -2820,6 +2846,7 @@ void ResourceFetcher::MaybeSaveResourceToStrongReference(Resource* resource) {
void ResourceFetcher::OnMemoryPressure(
base::MemoryPressureListener::MemoryPressureLevel level) {
document_resource_strong_refs_.clear();
document_resource_strong_refs_total_size_ = 0;
}

void ResourceFetcher::SetResourceCache(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,8 @@ class PLATFORM_EXPORT ResourceFetcher
return cached_resources_map_;
}

const HeapHashSet<Member<Resource>> MoveResourceStrongReferences() {
return std::move(document_resource_strong_refs_);
}
const HeapHashSet<Member<Resource>> MoveResourceStrongReferences();
bool HasStrongReferenceForTesting(Resource* resource);

enum class ImageLoadBlockingPolicy {
kDefault,
Expand Down Expand Up @@ -519,7 +518,7 @@ class PLATFORM_EXPORT ResourceFetcher

void WarnUnusedPreloads();

void RemoveResourceStrongReference(Resource* image_resource);
void RemoveResourceStrongReference(Resource* resource);

// Information about a resource fetch that had started but not completed yet.
// Would be added to the response data when the response arrives.
Expand Down Expand Up @@ -571,6 +570,7 @@ class PLATFORM_EXPORT ResourceFetcher
// document_resource_strong_refs_ keeps strong references for fonts, images,
// scripts and stylesheets within their freshness lifetime.
HeapHashSet<Member<Resource>> document_resource_strong_refs_;
size_t document_resource_strong_refs_total_size_ = 0;

// |image_resources_| is the subset of all image resources for the document.
HeapHashSet<WeakMember<Resource>> image_resources_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@

#include <memory>

#include "base/strings/string_number_conversions.h"
#include "base/task/single_thread_task_runner.h"
#include "base/test/bind.h"
#include "base/test/scoped_feature_list.h"
#include "build/build_config.h"
#include "services/network/public/mojom/ip_address_space.mojom-blink.h"
Expand Down Expand Up @@ -1608,4 +1610,45 @@ TEST_F(ResourceFetcherTest, DuplicatePreloadAllowsPriorityChange) {
EXPECT_FALSE(resource1->IsUnusedPreload());
}

TEST_F(ResourceFetcherTest, StrongReferenceThreshold) {
// `kTestResourceFilename` has 103 bytes.
const int64_t kMockResourceSize = 103;

// Set up the strong reference feature so that the memory cache can keep
// strong references to `kTestResourcefilename` up to two resources.
const int64_t kTotalSizeThreshold = kMockResourceSize * 2;
const int64_t kResourceSizeThreshold = kMockResourceSize;
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeaturesAndParameters(
/*enabled_features=*/
{
{features::kMemoryCacheStrongReference,
{{"memory_cache_strong_ref_total_size_threshold",
base::NumberToString(kTotalSizeThreshold)},
{"memory_cache_strong_ref_resource_size_threshold",
base::NumberToString(kResourceSizeThreshold)}}},
},
/*disabled_features=*/{});

ResourceFetcher* fetcher = CreateFetcher();

// A closure that fetches the given URL with `kTestResourceFilename` and
// returns whether the memory cache has a strong reference to the resource.
auto perform_fetch = base::BindLambdaForTesting([&](const KURL& url) {
ResourceResponse response(url);
platform_->GetURLLoaderMockFactory()->RegisterURL(
url, WrappedResourceResponse(response),
test::PlatformTestDataPath(kTestResourceFilename));
FetchParameters fetch_params =
FetchParameters::CreateForTest(ResourceRequest(url));
Resource* resource = MockResource::Fetch(fetch_params, fetcher, nullptr);
platform_->GetURLLoaderMockFactory()->ServeAsynchronousRequests();
return fetcher->HasStrongReferenceForTesting(resource);
});

ASSERT_TRUE(perform_fetch.Run(KURL("http://127.0.0.1:8000/foo.png")));
ASSERT_TRUE(perform_fetch.Run(KURL("http://127.0.0.1:8000/bar.png")));
ASSERT_FALSE(perform_fetch.Run(KURL("http://127.0.0.1:8000/baz.png")));
}

} // namespace blink

0 comments on commit 0d63ed5

Please sign in to comment.