Skip to content

Commit

Permalink
Reland^2: [PA] Enable PA-E on Linux component build.
Browse files Browse the repository at this point in the history
PS1 = the reverted CL: https://chromium-review.googlesource.com/c/chromium/src/+/4357306

The difference between the reverted one and this CL is:
- disable PartitionAlloc-As-Malloc when is_component_build=true and
  is_debug=true.

PartitionAlloc-As-Malloc made browser_tests, interactive_ui_tests,...
running on Linux Dbg(1) bot slower, and caused lots of timeout failures.

Since the debug bots(*)' gn args have both is_debug=true and
is_component_build=true, PartitionAlloc-As-Malloc didn't affect the
debug bots. But the reverted CL enabled PartitionAlloc-As-Malloc on
the bots.
(*) Linux Builder (dbg), cast Linux Debug, and Linux Tests (dbg)(1).

Change-Id: I0d37f66f7bcac01a209531305df883821f79b354

Cq-Include-Trybots: luci.chromium.try:linux_chromium_dbg_ng
Change-Id: I0d37f66f7bcac01a209531305df883821f79b354
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4378189
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Reviewed-by: Bartek Nowierski <bartekn@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1124633}
  • Loading branch information
tasak authored and Chromium LUCI CQ committed Mar 31, 2023
1 parent 840299c commit 42cd794
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 9 deletions.
5 changes: 5 additions & 0 deletions base/allocator/partition_alloc_support.cc
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,11 @@ void ReconfigurePartitionForKnownProcess(const std::string& process_type) {
// experiments.
}

PartitionAllocSupport* PartitionAllocSupport::Get() {
static auto* singleton = new PartitionAllocSupport();
return singleton;
}

PartitionAllocSupport::PartitionAllocSupport() = default;

void PartitionAllocSupport::ReconfigureForTests() {
Expand Down
5 changes: 1 addition & 4 deletions base/allocator/partition_alloc_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,7 @@ class BASE_EXPORT PartitionAllocSupport {
std::string stacktrace);
#endif

static PartitionAllocSupport* Get() {
static auto* singleton = new PartitionAllocSupport();
return singleton;
}
static PartitionAllocSupport* Get();

static BrpConfiguration GetBrpConfiguration(const std::string& process_type);

Expand Down
1 change: 1 addition & 0 deletions base/allocator/partition_allocator/partition_root.h
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,7 @@ enum class PtrPosWithinAlloc {
//
// This isn't a general purpose function. The caller is responsible for ensuring
// that the ref-count is in place for this allocation.
PA_COMPONENT_EXPORT(PARTITION_ALLOC)
PtrPosWithinAlloc IsPtrWithinSameAlloc(uintptr_t orig_address,
uintptr_t test_address,
size_t type_size);
Expand Down
14 changes: 10 additions & 4 deletions build_overrides/partition_alloc.gni
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,17 @@ if (is_ios) {
# Sanitizers replace the allocator, don't use our own.
_is_using_sanitizers = is_asan || is_hwasan || is_lsan || is_tsan || is_msan

# - Component build support is disabled on all platforms. It is known to cause
# issues on some (e.g. Windows with shims, Android with non-universal symbol
# wrapping), and has not been validated on others.
# - Component build support is disabled on all platforms except Linux. It is
# known to cause issues on some (e.g. Windows with shims, Android with
# non-universal symbol wrapping), and has not been validated on others.
# - Disable on Linux when component and debug build are used together, because
# PA-E caused lots of timeouts on Linux Tests(dbg).
# TODO(crbug.com/1429450): fix the timeout issue and enable on linux debug
# and component build.
# - Windows: debug CRT is not compatible, see below.
_disable_partition_alloc_everywhere = is_component_build || (is_win && is_debug)
_disable_partition_alloc_everywhere =
(is_linux && is_component_build && is_debug) ||
(!is_linux && is_component_build) || (is_win && is_debug)

# - NaCl: No plans to support it.
# - iOS: Depends on ios_partition_alloc_enabled.
Expand Down
20 changes: 19 additions & 1 deletion content/browser/web_contents/web_contents_impl_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <utility>
#include <vector>

#include "base/allocator/partition_alloc_features.h"
#include "base/allocator/partition_allocator/partition_alloc_buildflags.h"
#include "base/command_line.h"
#include "base/containers/contains.h"
Expand Down Expand Up @@ -5926,7 +5927,24 @@ class PCScanFeature {
// Initialize PCScanFeature before WebContentsImplBrowserTest to make sure that
// the feature is enabled within the entire lifetime of the test.
class WebContentsImplStarScanBrowserTest : private PCScanFeature,
public WebContentsImplBrowserTest {};
public WebContentsImplBrowserTest {
public:
void SetUp() override {
// Since ReconfigureAfterFeatureListInit() has been already invoked at
// FeatureListScopedToEachTest::OnTestStart(), we cannot enable PCScan
// and cannot re-reconfigure partition roots here. This causes DCHECK()
// failure at PerfromcScan().
if (!base::FeatureList::IsEnabled(base::features::kPartitionAllocPCScan) &&
!base::FeatureList::IsEnabled(
base::features::kPartitionAllocPCScanBrowserOnly) &&
!base::FeatureList::IsEnabled(
base::features::kPartitionAllocPCScanRendererOnly)) {
GTEST_SKIP() << "PCScanFeature is not enabled. Need --enable-features"
<< "=PartitionAllocPCScan";
}
WebContentsImplBrowserTest::SetUp();
}
};

} // namespace

Expand Down

0 comments on commit 42cd794

Please sign in to comment.