Use MemoryUsageMonitor for oom intervention and crash memory metrics

Use MemoryUsageMonitor from OomInterventionImpl and CrashMemoryMetricsReporter

Bug: 920922
Change-Id: Iff9f69d87b870ea6acc6c760058745de0d1a052a
Reviewed-on: https://chromium-review.googlesource.com/c/1411392
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuzu Saijo <yuzus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626845}

third_party/blink/renderer/controller/crash_memory_metrics_reporter_impl.cc

@@ -4,10 +4,6 @@
 
 #include "third_party/blink/renderer/controller/crash_memory_metrics_reporter_impl.h"
 
-#include <ctype.h>
-#include <fcntl.h>
-#include <unistd.h>
-
 #include "base/allocator/partition_allocator/oom_callback.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/process/memory.h"
@@ -20,25 +16,6 @@
 
 namespace blink {
 
-namespace {
-
-// Roughly calculates amount of memory which is used to execute pages.
-uint64_t BlinkMemoryWorkloadCalculator() {
-  v8::Isolate* isolate = V8PerIsolateData::MainThreadIsolate();
-  DCHECK(isolate);
-  v8::HeapStatistics heap_statistics;
-  isolate->GetHeapStatistics(&heap_statistics);
-  // TODO: Add memory usage for worker threads.
-  size_t v8_size =
-      heap_statistics.total_heap_size() + heap_statistics.malloced_memory();
-  size_t blink_gc_size = ProcessHeap::TotalAllocatedObjectSize() +
-                         ProcessHeap::TotalMarkedObjectSize();
-  size_t partition_alloc_size = WTF::Partitions::TotalSizeOfCommittedPages();
-  return v8_size + blink_gc_size + partition_alloc_size;
-}
-
-}  // namespace
-
 // static
 void CrashMemoryMetricsReporterImpl::Bind(
     mojom::blink::CrashMemoryMetricsReporterRequest request) {
@@ -68,6 +45,11 @@ void CrashMemoryMetricsReporterImpl::SetSharedMemory(
   shared_metrics_mapping_ = shared_metrics_buffer.Map();
 }
 
+void CrashMemoryMetricsReporterImpl::OnMemoryPing(MemoryUsage usage) {
+  WriteIntoSharedMemory(
+      CrashMemoryMetricsReporterImpl::MemoryUsageToMetrics(usage));
+}
+
 void CrashMemoryMetricsReporterImpl::WriteIntoSharedMemory(
     const OomInterventionMetrics& metrics) {
   if (!shared_metrics_mapping_.IsValid())
@@ -96,31 +78,30 @@ void CrashMemoryMetricsReporterImpl::OnOOMCallback() {
 
 OomInterventionMetrics
 CrashMemoryMetricsReporterImpl::GetCurrentMemoryMetrics() {
-  // This can only be called after ResetFileDescriptors().
-  DCHECK(statm_fd_.is_valid() && status_fd_.is_valid());
+  return MemoryUsageToMetrics(
+      MemoryUsageMonitor::Instance().GetCurrentMemoryUsage());
+}
 
+// static
+OomInterventionMetrics CrashMemoryMetricsReporterImpl::MemoryUsageToMetrics(
+    MemoryUsage usage) {
   OomInterventionMetrics metrics = {};
-  metrics.current_blink_usage_kb = BlinkMemoryWorkloadCalculator() / 1024;
-  uint64_t private_footprint, swap, vm_size;
-  if (MemoryUsageMonitorAndroid::CalculateProcessMemoryFootprint(
-          statm_fd_.get(), status_fd_.get(), &private_footprint, &swap,
-          &vm_size)) {
-    metrics.current_private_footprint_kb = private_footprint / 1024;
-    metrics.current_swap_kb = swap / 1024;
-    metrics.current_vm_size_kb = vm_size / 1024;
-  }
+
+  DCHECK(!std::isnan(usage.private_footprint_bytes));
+  DCHECK(!std::isnan(usage.swap_bytes));
+  DCHECK(!std::isnan(usage.vm_size_bytes));
+  metrics.current_blink_usage_kb =
+      (usage.v8_bytes + usage.blink_gc_bytes + usage.partition_alloc_bytes) /
+      1024;
+
+  DCHECK(!std::isnan(usage.private_footprint_bytes));
+  DCHECK(!std::isnan(usage.swap_bytes));
+  DCHECK(!std::isnan(usage.vm_size_bytes));
+  metrics.current_private_footprint_kb = usage.private_footprint_bytes / 1024;
+  metrics.current_swap_kb = usage.swap_bytes / 1024;
+  metrics.current_vm_size_kb = usage.vm_size_bytes / 1024;
   metrics.allocation_failed = 0;  // false
   return metrics;
 }
 
-bool CrashMemoryMetricsReporterImpl::ResetFileDiscriptors() {
-  // See https://goo.gl/KjWnZP For details about why we read these files from
-  // sandboxed renderer. Keep these files open when detection is enabled.
-  if (!statm_fd_.is_valid())
-    statm_fd_.reset(open("/proc/self/statm", O_RDONLY));
-  if (!status_fd_.is_valid())
-    status_fd_.reset(open("/proc/self/status", O_RDONLY));
-  return !statm_fd_.is_valid() || !status_fd_.is_valid();
-}
-
 }  // namespace blink

third_party/blink/renderer/controller/crash_memory_metrics_reporter_impl.h

@@ -10,15 +10,18 @@
 #include "third_party/blink/public/common/oom_intervention/oom_intervention_types.h"
 #include "third_party/blink/public/mojom/crash/crash_memory_metrics_reporter.mojom-blink.h"
 #include "third_party/blink/renderer/controller/controller_export.h"
+#include "third_party/blink/renderer/controller/memory_usage_monitor.h"
 
 namespace blink {
 
 // Writes data about renderer into shared memory that will be read by browser.
 class CONTROLLER_EXPORT CrashMemoryMetricsReporterImpl
-    : public mojom::blink::CrashMemoryMetricsReporter {
+    : public mojom::blink::CrashMemoryMetricsReporter,
+      public MemoryUsageMonitor::Observer {
  public:
   static CrashMemoryMetricsReporterImpl& Instance();
   static void Bind(mojom::blink::CrashMemoryMetricsReporterRequest);
+  static OomInterventionMetrics MemoryUsageToMetrics(MemoryUsage);
 
   ~CrashMemoryMetricsReporterImpl() override;
 
@@ -26,7 +29,8 @@ class CONTROLLER_EXPORT CrashMemoryMetricsReporterImpl
   void SetSharedMemory(
       base::UnsafeSharedMemoryRegion shared_metrics_buffer) override;
 
-  void WriteIntoSharedMemory(const OomInterventionMetrics& metrics);
+  // MemoryUsageMonitor::Observer:
+  void OnMemoryPing(MemoryUsage) override;
 
   // This method tracks when an allocation failure occurs. It should be hooked
   // into all platform allocation failure handlers in a process such as
@@ -38,22 +42,16 @@ class CONTROLLER_EXPORT CrashMemoryMetricsReporterImpl
   // This function needs to be called after ResetFileDescriptors.
   OomInterventionMetrics GetCurrentMemoryMetrics();
 
-  // This function resets statm_fd_ & status_fd_ to prepare for getting metrics.
-  bool ResetFileDiscriptors();
-
  protected:
   CrashMemoryMetricsReporterImpl();
 
  private:
   FRIEND_TEST_ALL_PREFIXES(OomInterventionImplTest, CalculateProcessFootprint);
 
+  void WriteIntoSharedMemory(const OomInterventionMetrics& metrics);
+
   base::WritableSharedMemoryMapping shared_metrics_mapping_;
   mojo::Binding<mojom::blink::CrashMemoryMetricsReporter> binding_;
-
-  // The file descriptor to current process proc files. The files are kept open
-  // when detection is on to reduce measurement overhead.
-  base::ScopedFD statm_fd_;
-  base::ScopedFD status_fd_;
 };
 }  // namespace blink
 

third_party/blink/renderer/controller/memory_usage_monitor.cc

@@ -28,6 +28,10 @@ void MemoryUsageMonitor::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
+bool MemoryUsageMonitor::HasObserver(Observer* observer) {
+  return observers_.HasObserver(observer);
+}
+
 void MemoryUsageMonitor::StartMonitoringIfNeeded() {
   if (timer_.IsActive())
     return;

third_party/blink/renderer/controller/memory_usage_monitor.h

@@ -36,12 +36,13 @@ class CONTROLLER_EXPORT MemoryUsageMonitor {
   virtual ~MemoryUsageMonitor() = default;
 
   // Returns the current memory usage.
-  MemoryUsage GetCurrentMemoryUsage();
+  virtual MemoryUsage GetCurrentMemoryUsage();
 
   // Ensures that an observer is only added once.
   void AddObserver(Observer*);
   // Observers must be removed before they are destroyed.
   void RemoveObserver(Observer*);
+  bool HasObserver(Observer*);
 
   bool TimerIsActive() const { return timer_.IsActive(); }
 

third_party/blink/renderer/controller/oom_intervention_impl.cc

@@ -11,10 +11,10 @@
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_gc_for_context_dispose.h"
 #include "third_party/blink/renderer/controller/crash_memory_metrics_reporter_impl.h"
+#include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/page/page.h"
 #include "third_party/blink/renderer/platform/bindings/v8_per_isolate_data.h"
-#include "third_party/blink/renderer/platform/wtf/allocator/partitions.h"
 
 namespace blink {
 
@@ -25,14 +25,13 @@ void OomInterventionImpl::Create(mojom::blink::OomInterventionRequest request) {
 }
 
 OomInterventionImpl::OomInterventionImpl()
-    : timer_(Thread::MainThread()->GetTaskRunner(),
-             this,
-             &OomInterventionImpl::Check),
-      delayed_report_timer_(Thread::MainThread()->GetTaskRunner(),
+    : delayed_report_timer_(Thread::MainThread()->GetTaskRunner(),
                             this,
                             &OomInterventionImpl::TimerFiredUMAReport) {}
 
-OomInterventionImpl::~OomInterventionImpl() {}
+OomInterventionImpl::~OomInterventionImpl() {
+  MemoryUsageMonitorInstance().RemoveObserver(this);
+}
 
 void OomInterventionImpl::StartDetection(
     mojom::blink::OomInterventionHostPtr host,
@@ -42,26 +41,29 @@ void OomInterventionImpl::StartDetection(
     bool purge_v8_memory_enabled) {
   host_ = std::move(host);
 
-  // Disable intervention if we cannot get memory details of current process.
-  if (CrashMemoryMetricsReporterImpl::Instance().ResetFileDiscriptors())
-    return;
-
   detection_args_ = std::move(detection_args);
   renderer_pause_enabled_ = renderer_pause_enabled;
   navigate_ads_enabled_ = navigate_ads_enabled;
   purge_v8_memory_enabled_ = purge_v8_memory_enabled;
 
-  timer_.Start(TimeDelta(), TimeDelta::FromSeconds(1), FROM_HERE);
+  MemoryUsageMonitorInstance().AddObserver(this);
 }
 
-OomInterventionMetrics OomInterventionImpl::GetCurrentMemoryMetrics() {
-  return CrashMemoryMetricsReporterImpl::Instance().GetCurrentMemoryMetrics();
+MemoryUsageMonitor& OomInterventionImpl::MemoryUsageMonitorInstance() {
+  return MemoryUsageMonitor::Instance();
 }
 
-void OomInterventionImpl::Check(TimerBase*) {
+void OomInterventionImpl::OnMemoryPing(MemoryUsage usage) {
+  // Ignore pings without process memory usage information.
+  if (std::isnan(usage.private_footprint_bytes) ||
+      std::isnan(usage.swap_bytes) || std::isnan(usage.vm_size_bytes))
+    return;
+  Check(CrashMemoryMetricsReporterImpl::MemoryUsageToMetrics(usage));
+}
+
+void OomInterventionImpl::Check(OomInterventionMetrics current_memory) {
   DCHECK(host_);
 
-  OomInterventionMetrics current_memory = GetCurrentMemoryMetrics();
   bool oom_detected = false;
 
   oom_detected |= detection_args_->blink_workload_threshold > 0 &&
@@ -100,7 +102,7 @@ void OomInterventionImpl::Check(TimerBase*) {
     }
 
     host_->OnHighMemoryUsage();
-    timer_.Stop();
+    MemoryUsageMonitorInstance().RemoveObserver(this);
     // Send memory pressure notification to trigger GC.
     Thread::MainThread()->GetTaskRunner()->PostTask(FROM_HERE,
                                                     base::BindOnce(&TriggerGC));
@@ -134,13 +136,11 @@ void OomInterventionImpl::ReportMemoryStats(
       "Memory.Experimental.OomIntervention.RendererVmSize",
       base::saturated_cast<base::Histogram::Sample>(
           current_memory.current_vm_size_kb / 1024));
-
-  CrashMemoryMetricsReporterImpl::Instance().WriteIntoSharedMemory(
-      current_memory);
 }
 
 void OomInterventionImpl::TimerFiredUMAReport(TimerBase*) {
-  OomInterventionMetrics current_memory = GetCurrentMemoryMetrics();
+  OomInterventionMetrics current_memory =
+      CrashMemoryMetricsReporterImpl::Instance().GetCurrentMemoryMetrics();
   switch (number_of_report_needed_--) {
     case 3:
       base::UmaHistogramSparse(

third_party/blink/renderer/controller/oom_intervention_impl.h

@@ -5,11 +5,10 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CONTROLLER_OOM_INTERVENTION_IMPL_H_
 #define THIRD_PARTY_BLINK_RENDERER_CONTROLLER_OOM_INTERVENTION_IMPL_H_
 
-#include "base/files/scoped_file.h"
 #include "third_party/blink/public/common/oom_intervention/oom_intervention_types.h"
 #include "third_party/blink/public/platform/oom_intervention.mojom-blink.h"
 #include "third_party/blink/renderer/controller/controller_export.h"
-#include "third_party/blink/renderer/core/dom/document.h"
+#include "third_party/blink/renderer/controller/memory_usage_monitor.h"
 #include "third_party/blink/renderer/core/page/scoped_page_pauser.h"
 #include "third_party/blink/renderer/platform/timer.h"
 
@@ -20,7 +19,8 @@ class OomInterventionImplTest;
 // Implementation of OOM intervention. This pauses all pages by using
 // ScopedPagePauser when near-OOM situation is detected.
 class CONTROLLER_EXPORT OomInterventionImpl
-    : public mojom::blink::OomIntervention {
+    : public mojom::blink::OomIntervention,
+      public MemoryUsageMonitor::Observer {
  public:
   static void Create(mojom::blink::OomInterventionRequest);
 
@@ -34,17 +34,20 @@ class CONTROLLER_EXPORT OomInterventionImpl
                       bool navigate_ads_enabled,
                       bool purge_v8_memory_enabled) override;
 
+  // MemoryUsageMonitor::Observer:
+  void OnMemoryPing(MemoryUsage) override;
+
  private:
   FRIEND_TEST_ALL_PREFIXES(OomInterventionImplTest, DetectedAndDeclined);
-  FRIEND_TEST_ALL_PREFIXES(OomInterventionImplTest, CalculateProcessFootprint);
   FRIEND_TEST_ALL_PREFIXES(OomInterventionImplTest, StopWatchingAfterDetection);
   FRIEND_TEST_ALL_PREFIXES(OomInterventionImplTest,
                            ContinueWatchingWithoutDetection);
   FRIEND_TEST_ALL_PREFIXES(OomInterventionImplTest, V1DetectionAdsNavigation);
 
   // Overridden by test.
-  virtual OomInterventionMetrics GetCurrentMemoryMetrics();
-  void Check(TimerBase*);
+  virtual MemoryUsageMonitor& MemoryUsageMonitorInstance();
+
+  void Check(OomInterventionMetrics);
 
   void ReportMemoryStats(OomInterventionMetrics& current_memory);
 
@@ -58,7 +61,6 @@ class CONTROLLER_EXPORT OomInterventionImpl
   bool renderer_pause_enabled_ = false;
   bool navigate_ads_enabled_ = false;
   bool purge_v8_memory_enabled_ = false;
-  TaskRunnerTimer<OomInterventionImpl> timer_;
   std::unique_ptr<ScopedPagePauser> pauser_;
   OomInterventionMetrics metrics_at_intervention_;
   int number_of_report_needed_ = 0;