Skip to content

Commit

Permalink
partitions: Add buffer partition escape-hatch
Browse files Browse the repository at this point in the history
Per https://crbug.com/1444624#c44, we have discovered a quarantine leak
in the Buffer partition causing unacceptable memory overhead when
MiraclePtr is brought live in the renderer process.

We think we've found the leak (https://crbug.com/1444624#c45), but out
of an abundance of caution, we add an escape-hatch to turn off BRP in
the buffer partition. We do not want to use it, but it will be necessary
if we cannot wrestle quarantine bloat under control at 99P (especially
on Android).

Bug: 1444624
Change-Id: Ic3b28353ecdc3bbaa31756eb5e6ba5da7deb784a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4958135
Commit-Queue: Kalvin Lee <kdlee@chromium.org>
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1213204}
  • Loading branch information
Kalvin Lee authored and Chromium LUCI CQ committed Oct 22, 2023
1 parent 93d25e4 commit e933f3a
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 0 deletions.
12 changes: 12 additions & 0 deletions base/allocator/partition_alloc_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -415,5 +415,17 @@ MIRACLE_PARAMETER_FOR_INT(
"ThreadCacheMinCachedMemoryForPurgingBytes",
partition_alloc::kMinCachedMemoryForPurgingBytes)

// An apparent quarantine leak in the buffer partition unacceptably
// bloats memory when MiraclePtr is enabled in the renderer process.
// We believe we have found and patched the leak, but out of an
// abundance of caution, we provide this toggle that allows us to
// wholly disable MiraclePtr in the buffer partition, if necessary.
//
// TODO(crbug.com/1444624): this is unneeded once
// MiraclePtr-for-Renderer launches.
BASE_FEATURE(kPartitionAllocDisableBRPInBufferPartition,
"PartitionAllocDisableBRPInBufferPartition",
FEATURE_DISABLED_BY_DEFAULT);

} // namespace features
} // namespace base
2 changes: 2 additions & 0 deletions base/allocator/partition_alloc_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ BASE_EXPORT BASE_DECLARE_FEATURE(
kEnableConfigurableThreadCacheMinCachedMemoryForPurging);
BASE_EXPORT int GetThreadCacheMinCachedMemoryForPurgingBytes();

BASE_EXPORT BASE_DECLARE_FEATURE(kPartitionAllocDisableBRPInBufferPartition);

} // namespace features
} // namespace base

Expand Down
12 changes: 12 additions & 0 deletions third_party/blink/renderer/platform/wtf/allocator/partitions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,22 @@ bool Partitions::InitializeOnce() {
partition_alloc::PartitionAllocGlobalInit(&Partitions::HandleOutOfMemory);

auto options = PartitionOptionsFromFeatures();

const auto actual_brp_setting = options.backup_ref_ptr;
if (base::FeatureList::IsEnabled(
base::features::kPartitionAllocDisableBRPInBufferPartition)) {
options.backup_ref_ptr = PartitionOptions::kDisabled;
}

static base::NoDestructor<partition_alloc::PartitionAllocator>
buffer_allocator(options);
buffer_root_ = buffer_allocator->root();

if (base::FeatureList::IsEnabled(
base::features::kPartitionAllocDisableBRPInBufferPartition)) {
options.backup_ref_ptr = actual_brp_setting;
}

scan_is_enabled_ =
(options.backup_ref_ptr == PartitionOptions::kDisabled) &&
#if BUILDFLAG(USE_STARSCAN)
Expand Down

0 comments on commit e933f3a

Please sign in to comment.