Commit dac062d8 authored by Zoe Clifford's avatar Zoe Clifford Committed by Commit Bot

Allow string parking to be disabled

String parking can be fairly expensive in terms of CPU, which makes it
undesirable for certain headless use-cases, such as those with many
short-lived pages, or where RAM is more plentiful than CPU.

The option to disable string parking was removed in [1]. This CL re-adds
a similar option so it may be disabled once more.

(Google Internal) screenshot of CPU profiling: https://screenshot.googleplex.com/cFXiZq1ydBN

[1]
https://chromium-review.googlesource.com/c/chromium/src/+/1697649

Change-Id: Ib05e79a009fb70565011bd8aa9ba6238c6a8c617
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1820023
Commit-Queue: Zoe Clifford <zoeclifford@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704882}
parent 9869039f
......@@ -25,6 +25,12 @@
namespace blink {
// Disabling this will cause parkable strings to never be compressed.
// This is useful for headless mode + virtual time. Since virtual time advances
// quickly, strings may be parked too eagerly in that mode.
const base::Feature kCompressParkableStrings{"CompressParkableStrings",
base::FEATURE_ENABLED_BY_DEFAULT};
struct ParkableStringManager::Statistics {
size_t original_size;
size_t uncompressed_size;
......@@ -38,11 +44,18 @@ struct ParkableStringManager::Statistics {
namespace {
bool CompressionEnabled() {
return base::FeatureList::IsEnabled(kCompressParkableStrings);
}
class OnPurgeMemoryListener : public GarbageCollected<OnPurgeMemoryListener>,
public MemoryPressureListener {
USING_GARBAGE_COLLECTED_MIXIN(OnPurgeMemoryListener);
void OnPurgeMemory() override {
if (!CompressionEnabled()) {
return;
}
ParkableStringManager::Instance().PurgeMemory();
}
};
......@@ -147,7 +160,8 @@ bool ParkableStringManager::ShouldPark(const StringImpl& string) {
// Don't attempt to park strings smaller than this size.
static constexpr unsigned int kSizeThreshold = 10000;
// TODO(lizeb): Consider parking non-main thread strings.
return string.length() > kSizeThreshold && IsMainThread();
return string.length() > kSizeThreshold && IsMainThread() &&
CompressionEnabled();
}
scoped_refptr<ParkableStringImpl> ParkableStringManager::Add(
......@@ -254,6 +268,7 @@ void ParkableStringManager::RecordUnparkingTime(
void ParkableStringManager::ParkAll(ParkableStringImpl::ParkingMode mode) {
DCHECK(IsMainThread());
DCHECK(CompressionEnabled());
size_t total_size = 0;
for (ParkableStringImpl* str : parked_strings_)
......@@ -305,6 +320,8 @@ void ParkableStringManager::RecordStatisticsAfter5Minutes() const {
}
void ParkableStringManager::AgeStringsAndPark() {
DCHECK(CompressionEnabled());
TRACE_EVENT0("blink", "ParkableStringManager::AgeStringsAndPark");
has_pending_aging_task_ = false;
......@@ -331,6 +348,9 @@ void ParkableStringManager::AgeStringsAndPark() {
}
void ParkableStringManager::ScheduleAgingTaskIfNeeded() {
if (!CompressionEnabled())
return;
if (has_pending_aging_task_)
return;
......@@ -346,6 +366,7 @@ void ParkableStringManager::ScheduleAgingTaskIfNeeded() {
void ParkableStringManager::PurgeMemory() {
DCHECK(IsMainThread());
DCHECK(CompressionEnabled());
ParkAll(ParkableStringImpl::ParkingMode::kAlways);
// Critical memory pressure: drop compressed data for strings that we cannot
......
......@@ -5,6 +5,7 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_BINDINGS_PARKABLE_STRING_MANAGER_H_
#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_BINDINGS_PARKABLE_STRING_MANAGER_H_
#include "base/feature_list.h"
#include "base/macros.h"
#include "base/memory/scoped_refptr.h"
#include "base/time/time.h"
......@@ -21,6 +22,8 @@ namespace blink {
class ParkableString;
PLATFORM_EXPORT extern const base::Feature kCompressParkableStrings;
class PLATFORM_EXPORT ParkableStringManagerDumpProvider
: public base::trace_event::MemoryDumpProvider {
USING_FAST_MALLOC(ParkableStringManagerDumpProvider);
......
......@@ -8,6 +8,7 @@
#include "base/run_loop.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/timer/elapsed_timer.h"
......@@ -59,7 +60,9 @@ class ParkableStringTest : public ::testing::Test {
}
void WaitForAging() {
EXPECT_GT(task_environment_.GetPendingMainThreadTaskCount(), 0u);
if (base::FeatureList::IsEnabled(kCompressParkableStrings)) {
EXPECT_GT(task_environment_.GetPendingMainThreadTaskCount(), 0u);
}
task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(
ParkableStringManager::kAgingIntervalInSeconds));
}
......@@ -673,6 +676,18 @@ TEST_F(ParkableStringTest, ReportMemoryDump) {
EXPECT_THAT(dump->entries(), Contains(Eq(ByRef(savings))));
}
TEST_F(ParkableStringTest, CompressionDisabled) {
base::test::ScopedFeatureList features;
features.InitAndDisableFeature(kCompressParkableStrings);
ParkableString parkable(MakeLargeString().ReleaseImpl());
WaitForDelayedParking();
EXPECT_FALSE(parkable.Impl()->is_parked());
MemoryPressureListenerRegistry::Instance().OnPurgeMemory();
EXPECT_FALSE(parkable.Impl()->is_parked());
}
TEST_F(ParkableStringTest, Aging) {
ParkableString parkable(MakeLargeString().ReleaseImpl());
EXPECT_TRUE(parkable.Impl()->is_young_for_testing());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment