Implement notification via dispatcher

Notification of memory allocation and free events previously happened
through hooks installed by PoissonAllocationSampler.

This CL establishes the notification via a dispatcher. The actual
notification path can be selected via gn.args parameter. By default the
hooks that are installed by PoissonAllocationSampler will be used.

Bug: 1137393
Change-Id: I1255294309858d02e871826955cddc9f429ef42e
Validate-Test-Flakiness: skip
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4180726
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Joe Mason <joenotcharles@google.com>
Reviewed-by: Benoit Lize <lizeb@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: Richard Townsend <richard.townsend@arm.com>
Cr-Commit-Position: refs/heads/main@{#1115632}

android_webview/lib/aw_main_delegate.cc

@@ -432,17 +432,7 @@ absl::optional<int> AwMainDelegate::PostEarlyInitialization(
     content::InitializeMojoCore();
   }
 
-  version_info::Channel channel = version_info::android::GetChannel();
-  const bool is_canary_dev = (channel == version_info::Channel::CANARY ||
-                              channel == version_info::Channel::DEV);
-  const std::string process_type =
-      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-          switches::kProcessType);
-  const bool gwp_asan_boost_sampling = is_canary_dev || is_browser_process;
-
-  memory_system::Initializer()
-      .SetGwpAsanParameters(gwp_asan_boost_sampling, process_type)
-      .Initialize(memory_system_);
+  InitializeMemorySystem(is_browser_process);
 
   return absl::nullopt;
 }
@@ -495,4 +485,26 @@ content::ContentRendererClient* AwMainDelegate::CreateContentRendererClient() {
   return content_renderer_client_.get();
 }
 
+void AwMainDelegate::InitializeMemorySystem(const bool is_browser_process) {
+  const version_info::Channel channel = version_info::android::GetChannel();
+  const bool is_canary_dev = (channel == version_info::Channel::CANARY ||
+                              channel == version_info::Channel::DEV);
+  const std::string process_type =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kProcessType);
+  const bool gwp_asan_boost_sampling = is_canary_dev || is_browser_process;
+
+  // Add PoissonAllocationSampler. On Android WebView we do not have obvious
+  // observers of PoissonAllocationSampler. Unfortunately, some potential
+  // candidates are still linked and may sneak in through hidden paths.
+  // Therefore, we include PoissonAllocationSampler unconditionally.
+  // TODO(crbug.com/1411454): Which observers of PoissonAllocationSampler are
+  // really in use on Android WebView? Can we add the sampler conditionally or
+  // remove it completely?
+  memory_system::Initializer()
+      .SetGwpAsanParameters(gwp_asan_boost_sampling, process_type)
+      .SetDispatcherParameters(memory_system::DispatcherParameters::
+                                   PoissonAllocationSamplerInclusion::kEnforce)
+      .Initialize(memory_system_);
+}
 }  // namespace android_webview

android_webview/lib/aw_main_delegate.h

@@ -52,6 +52,8 @@ class AwMainDelegate : public content::ContentMainDelegate {
   content::ContentGpuClient* CreateContentGpuClient() override;
   content::ContentRendererClient* CreateContentRendererClient() override;
 
+  void InitializeMemorySystem(const bool is_browser_process);
+
   // Responsible for creating a feature list from the seed. This object must
   // exist for the lifetime of the process as it contains the FieldTrialList
   // that can be queried for the state of experiments.

base/allocator/BUILD.gn

@@ -18,6 +18,8 @@ buildflag_header("buildflags") {
     "USE_ALLOCATOR_SHIM=$use_allocator_shim",
 
     "USE_PARTITION_ALLOC_AS_GWP_ASAN_STORE=$enable_backup_ref_ptr_support",
+
+    "USE_ALLOCATION_EVENT_DISPATCHER=$use_allocation_event_dispatcher",
   ]
 }
 

base/allocator/allocator.gni

@@ -12,6 +12,11 @@ if (is_ios) {
 declare_args() {
   # Causes all the allocations to be routed via allocator_shim.cc.
   use_allocator_shim = use_allocator_shim_default
+
+  # Use the new allocation event dispatcher to distribute events to event observers.
+  # If set to false, PoissonAllocationSampler will hook into PartitionAllocator and
+  # AllocatorShims directly.
+  use_allocation_event_dispatcher = false
 }
 
 assert(

base/allocator/dispatcher/dispatcher.cc

@@ -18,6 +18,7 @@
 #include <atomic>
 #endif
 
+#if !BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
 #if BUILDFLAG(USE_ALLOCATOR_SHIM)
 namespace base::allocator::dispatcher::allocator_shim_details {
 namespace {
@@ -223,9 +224,11 @@ void PartitionFreeHook(void* address) {
 }  // namespace
 }  // namespace base::allocator::dispatcher::partition_allocator_details
 #endif  // BUILDFLAG(USE_PARTITION_ALLOC)
+#endif  // !BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
 
 namespace base::allocator::dispatcher {
 
+#if !BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
 void InstallStandardAllocatorHooks() {
 #if BUILDFLAG(USE_ALLOCATOR_SHIM)
   allocator_shim::InsertAllocatorDispatch(
@@ -242,10 +245,7 @@ void InstallStandardAllocatorHooks() {
       &partition_allocator_details::PartitionFreeHook);
 #endif  // BUILDFLAG(USE_PARTITION_ALLOC)
 }
-
-}  // namespace base::allocator::dispatcher
-
-namespace base::allocator::dispatcher {
+#endif  // !BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
 
 // The private implementation of Dispatcher.
 struct Dispatcher::Impl {

base/allocator/dispatcher/dispatcher.h

@@ -5,14 +5,17 @@
 #ifndef BASE_ALLOCATOR_DISPATCHER_DISPATCHER_H_
 #define BASE_ALLOCATOR_DISPATCHER_DISPATCHER_H_
 
+#include "base/allocator/buildflags.h"
 #include "base/allocator/dispatcher/internal/dispatcher_internal.h"
 #include "base/base_export.h"
 
 #include <memory>
 
 namespace base::allocator::dispatcher {
 
+#if !BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
 void BASE_EXPORT InstallStandardAllocatorHooks();
+#endif
 
 namespace internal {
 struct DispatchData;

base/sampling_heap_profiler/poisson_allocation_sampler.cc

@@ -171,20 +171,26 @@ PoissonAllocationSampler::ScopedMuteHookedSamplesForTesting::
   ResetProfilingStateFlag(ProfilingStateFlag::kHookedSamplesMutedForTesting);
 }
 
+#if !BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
 // static
 PoissonAllocationSampler* PoissonAllocationSampler::instance_ = nullptr;
+#endif
 
 // static
 ABSL_CONST_INIT std::atomic<PoissonAllocationSampler::ProfilingStateFlagMask>
     PoissonAllocationSampler::profiling_state_{0};
 
 PoissonAllocationSampler::PoissonAllocationSampler() {
+#if !BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
   CHECK_EQ(nullptr, instance_);
   instance_ = this;
+#endif
+
   Init();
   auto* sampled_addresses = new LockFreeAddressHashSet(64);
   g_sampled_addresses_set.store(sampled_addresses, std::memory_order_release);
 
+#if !BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
   // Install the allocator hooks immediately, to better match the behaviour
   // of base::allocator::Initializer.
   //
@@ -193,6 +199,7 @@ PoissonAllocationSampler::PoissonAllocationSampler() {
   // initializer at the same time so this will install the hooks even
   // earlier in process startup.
   allocator::dispatcher::InstallStandardAllocatorHooks();
+#endif
 }
 
 // static

base/sampling_heap_profiler/poisson_allocation_sampler.h

@@ -8,6 +8,7 @@
 #include <atomic>
 #include <vector>
 
+#include "base/allocator/buildflags.h"
 #include "base/allocator/dispatcher/reentry_guard.h"
 #include "base/allocator/dispatcher/subsystem.h"
 #include "base/base_export.h"
@@ -103,12 +104,14 @@ class BASE_EXPORT PoissonAllocationSampler {
   // Returns the current mean sampling interval, in bytes.
   size_t SamplingInterval() const;
 
+#if !BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
   ALWAYS_INLINE static void RecordAlloc(
       void* address,
       size_t,
       base::allocator::dispatcher::AllocationSubsystem,
       const char* context);
   ALWAYS_INLINE static void RecordFree(void* address);
+#endif
 
   void OnAllocation(void* address,
                     size_t,
@@ -211,7 +214,9 @@ class BASE_EXPORT PoissonAllocationSampler {
   // RemoveSamplesObserver().
   std::vector<SamplesObserver*> observers_ GUARDED_BY(mutex_);
 
+#if !BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
   static PoissonAllocationSampler* instance_;
+#endif
 
   // Fast, thread-safe access to the current profiling state.
   static std::atomic<ProfilingStateFlagMask> profiling_state_;
@@ -224,6 +229,7 @@ class BASE_EXPORT PoissonAllocationSampler {
   FRIEND_TEST_ALL_PREFIXES(SamplingHeapProfilerTest, HookedAllocatorMuted);
 };
 
+#if !BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
 // static
 ALWAYS_INLINE void PoissonAllocationSampler::RecordAlloc(
     void* address,
@@ -237,6 +243,7 @@ ALWAYS_INLINE void PoissonAllocationSampler::RecordAlloc(
 ALWAYS_INLINE void PoissonAllocationSampler::RecordFree(void* address) {
   instance_->OnFree(address);
 }
+#endif
 
 inline void PoissonAllocationSampler::OnAllocation(
     void* address,

base/sampling_heap_profiler/sampling_heap_profiler_unittest.cc

@@ -7,6 +7,7 @@
 #include <stdlib.h>
 #include <cinttypes>
 
+#include "base/allocator/buildflags.h"
 #include "base/allocator/partition_allocator/shim/allocator_shim.h"
 #include "base/debug/alias.h"
 #include "base/memory/raw_ptr.h"
@@ -17,6 +18,10 @@
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
+#include "base/allocator/dispatcher/dispatcher.h"
+#endif
+
 namespace base {
 
 using ScopedSuppressRandomnessForTesting =
@@ -35,6 +40,17 @@ class SamplingHeapProfilerTest : public ::testing::Test {
     ASSERT_FALSE(PoissonAllocationSampler::AreHookedSamplesMuted());
     ASSERT_FALSE(PoissonAllocationSampler::ScopedMuteThreadSamples::IsMuted());
     ASSERT_FALSE(ScopedSuppressRandomnessForTesting::IsSuppressed());
+
+#if BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
+    allocator::dispatcher::Dispatcher::GetInstance().InitializeForTesting(
+        PoissonAllocationSampler::Get());
+#endif
+  }
+
+  void TearDown() override {
+#if BUILDFLAG(USE_ALLOCATION_EVENT_DISPATCHER)
+    allocator::dispatcher::Dispatcher::GetInstance().ResetForTesting();
+#endif
   }
 
   size_t GetNextSample(size_t mean_interval) {
@@ -252,13 +268,13 @@ TEST_F(SamplingHeapProfilerTest, MAYBE_MANUAL_SamplerMicroBenchmark) {
 
   base::TimeTicks t0 = base::TimeTicks::Now();
   for (int i = 1; i <= kNumAllocations; ++i) {
-    sampler->RecordAlloc(reinterpret_cast<void*>(static_cast<intptr_t>(i)),
-                         allocation_size, AllocationSubsystem::kAllocatorShim,
-                         nullptr);
+    sampler->OnAllocation(reinterpret_cast<void*>(static_cast<intptr_t>(i)),
+                          allocation_size, AllocationSubsystem::kAllocatorShim,
+                          nullptr);
   }
   base::TimeTicks t1 = base::TimeTicks::Now();
   for (int i = 1; i <= kNumAllocations; ++i)
-    sampler->RecordFree(reinterpret_cast<void*>(static_cast<intptr_t>(i)));
+    sampler->OnFree(reinterpret_cast<void*>(static_cast<intptr_t>(i)));
   base::TimeTicks t2 = base::TimeTicks::Now();
 
   printf(
@@ -342,9 +358,9 @@ TEST_F(SamplingHeapProfilerTest, HookedAllocatorMuted) {
     // Manual allocations should be captured.
     sampler->AddSamplesObserver(&collector);
     void* const kAddress = reinterpret_cast<void*>(0x1234);
-    sampler->RecordAlloc(kAddress, 10000,
-                         AllocationSubsystem::kManualForTesting, nullptr);
-    sampler->RecordFree(kAddress);
+    sampler->OnAllocation(kAddress, 10000,
+                          AllocationSubsystem::kManualForTesting, nullptr);
+    sampler->OnFree(kAddress);
     sampler->RemoveSamplesObserver(&collector);
     EXPECT_TRUE(collector.sample_added);
     EXPECT_TRUE(collector.sample_removed);

chrome/app/chrome_main_delegate.cc

@@ -905,17 +905,9 @@ void ChromeMainDelegate::CommonEarlyInitialization() {
   base::PlatformThread::InitFeaturesPostFieldTrial();
 #endif
 
-  const version_info::Channel channel = chrome::GetChannel();
-  const bool is_canary_dev = (channel == version_info::Channel::CANARY ||
-                              channel == version_info::Channel::DEV);
-
-  const bool gwp_asan_boost_sampling = is_canary_dev || is_browser_process;
-
-  memory_system::Initializer()
-      .SetGwpAsanParameters(gwp_asan_boost_sampling, process_type)
-      .SetProfilingClientParameters(channel,
-                                    GetProfileParamsProcess(*command_line))
-      .Initialize(memory_system_);
+  // Start memory observation as early as possible so it can start recording
+  // memory allocations. This includes heap profiling.
+  InitializeMemorySystem();
 
   if (is_browser_process) {
 #if BUILDFLAG(IS_CHROMEOS_ASH)
@@ -1752,3 +1744,23 @@ absl::optional<int> ChromeMainDelegate::PreBrowserMain() {
   // Do not interrupt startup.
   return absl::nullopt;
 }
+
+void ChromeMainDelegate::InitializeMemorySystem() {
+  const base::CommandLine* const command_line =
+      base::CommandLine::ForCurrentProcess();
+  const std::string process_type =
+      command_line->GetSwitchValueASCII(switches::kProcessType);
+  const bool is_browser_process = process_type.empty();
+  const version_info::Channel channel = chrome::GetChannel();
+  const bool is_canary_dev = (channel == version_info::Channel::CANARY ||
+                              channel == version_info::Channel::DEV);
+  const bool gwp_asan_boost_sampling = is_canary_dev || is_browser_process;
+
+  memory_system::Initializer()
+      .SetGwpAsanParameters(gwp_asan_boost_sampling, process_type)
+      .SetProfilingClientParameters(channel,
+                                    GetProfileParamsProcess(*command_line))
+      .SetDispatcherParameters(memory_system::DispatcherParameters::
+                                   PoissonAllocationSamplerInclusion::kEnforce)
+      .Initialize(memory_system_);
+}

chrome/app/chrome_main_delegate.h

@@ -90,6 +90,8 @@ class ChromeMainDelegate : public content::ContentMainDelegate {
   void SetUpInstallerPreferences(const base::CommandLine& command_line);
 #endif  // BUILDFLAG(IS_MAC)
 
+  void InitializeMemorySystem();
+
   std::unique_ptr<ChromeContentBrowserClient> chrome_content_browser_client_;
   std::unique_ptr<ChromeContentUtilityClient> chrome_content_utility_client_;
   std::unique_ptr<tracing::TracingSamplerProfiler> tracing_sampler_profiler_;

components/heap_profiling/in_process/heap_profiler_controller.cc

@@ -273,7 +273,7 @@ HeapProfilerController::~HeapProfilerController() {
   g_profiling_enabled = ProfilingEnabled::kNoController;
 }
 
-void HeapProfilerController::StartIfEnabled() {
+bool HeapProfilerController::StartIfEnabled() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   const bool profiling_enabled =
       g_profiling_enabled == ProfilingEnabled::kEnabled;
@@ -287,7 +287,7 @@ void HeapProfilerController::StartIfEnabled() {
     base::UmaHistogramBoolean(kEnabledHistogramName, profiling_enabled);
   }
   if (!profiling_enabled)
-    return;
+    return false;
   HeapProfilerParameters profiler_params =
       GetHeapProfilerParametersForProcess(process_type_);
   // DecideIfCollectionIsEnabled() should return false if not supported.
@@ -303,6 +303,7 @@ void HeapProfilerController::StartIfEnabled() {
       /*use_random_interval=*/!suppress_randomness_for_testing_, stopped_,
       process_type_, creation_time_);
   ScheduleNextSnapshot(std::move(params));
+  return true;
 }
 
 void HeapProfilerController::SuppressRandomnessForTesting() {

components/heap_profiling/in_process/heap_profiler_controller.h

@@ -46,7 +46,9 @@ class HeapProfilerController {
 
   // Starts periodic heap snapshot collection. Does nothing except record a
   // metric if heap profiling is disabled.
-  void StartIfEnabled();
+  // Returns true if heap profiling is enabled and was successfully started,
+  // false otherwise.
+  bool StartIfEnabled();
 
   // Uses the exact parameter values for the sampling interval and time between
   // samples, instead of a distribution around those values. This must be called