Fix OomIntervention.Reduced* metrics

The code calculated the deltas with uint64_t, causing negative values to be reported incorrectly.
This CL fixes the subtraction. And also clamps the samples to 1000 buckets (-500MB to 500MB).

Bug: 917293
Change-Id: Ie2b48ddbf1a47577692c4c2013c7a9d365c786ea
Reviewed-on: https://chromium-review.googlesource.com/c/1488475Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636678}

third_party/blink/renderer/controller/crash_memory_metrics_reporter_impl.cc

@@ -76,12 +76,6 @@ void CrashMemoryMetricsReporterImpl::OnOOMCallback() {
   CrashMemoryMetricsReporterImpl::Instance().WriteIntoSharedMemory(metrics);
 }
 
-OomInterventionMetrics
-CrashMemoryMetricsReporterImpl::GetCurrentMemoryMetrics() {
-  return MemoryUsageToMetrics(
-      MemoryUsageMonitor::Instance().GetCurrentMemoryUsage());
-}
-
 // static
 OomInterventionMetrics CrashMemoryMetricsReporterImpl::MemoryUsageToMetrics(
     MemoryUsage usage) {

third_party/blink/renderer/controller/crash_memory_metrics_reporter_impl.h

@@ -39,9 +39,6 @@ class CONTROLLER_EXPORT CrashMemoryMetricsReporterImpl
   // failures and base::TerminateBecauseOutOfMemory(), too.
   static void OnOOMCallback();
 
-  // This function needs to be called after ResetFileDescriptors.
-  OomInterventionMetrics GetCurrentMemoryMetrics();
-
  protected:
   CrashMemoryMetricsReporterImpl();
 

third_party/blink/renderer/controller/oom_intervention_impl.cc

@@ -187,45 +187,47 @@ void OomInterventionImpl::ReportMemoryStats(
           current_memory.current_vm_size_kb / 1024));
 }
 
+int ToMemoryUsageDeltaSample(uint64_t after_kb, uint64_t before_kb) {
+  int delta_mb = (base::saturated_cast<int>(after_kb) -
+                  base::saturated_cast<int>(before_kb)) /
+                 1024;
+  return std::min(std::max(delta_mb, -500), 500);
+}
+
 void OomInterventionImpl::TimerFiredUMAReport(TimerBase*) {
   OomInterventionMetrics current_memory =
-      CrashMemoryMetricsReporterImpl::Instance().GetCurrentMemoryMetrics();
+      CrashMemoryMetricsReporterImpl::MemoryUsageToMetrics(
+          MemoryUsageMonitorInstance().GetCurrentMemoryUsage());
+  int blink_usage_delta =
+      ToMemoryUsageDeltaSample(current_memory.current_blink_usage_kb,
+                               metrics_at_intervention_.current_blink_usage_kb);
+  int private_footprint_delta = ToMemoryUsageDeltaSample(
+      current_memory.current_private_footprint_kb,
+      metrics_at_intervention_.current_private_footprint_kb);
   switch (number_of_report_needed_--) {
     case 3:
       base::UmaHistogramSparse(
           "Memory.Experimental.OomIntervention.ReducedBlinkUsageAfter10secs",
-          base::saturated_cast<base::Histogram::Sample>(
-              current_memory.current_blink_usage_kb / 1024 -
-              metrics_at_intervention_.current_blink_usage_kb / 1024));
+          blink_usage_delta);
       base::UmaHistogramSparse(
           "Memory.Experimental.OomIntervention.ReducedRendererPMFAfter10secs",
-          base::saturated_cast<base::Histogram::Sample>(
-              current_memory.current_private_footprint_kb / 1024 -
-              metrics_at_intervention_.current_private_footprint_kb / 1024));
+          private_footprint_delta);
       break;
     case 2:
       base::UmaHistogramSparse(
           "Memory.Experimental.OomIntervention.ReducedBlinkUsageAfter20secs",
-          base::saturated_cast<base::Histogram::Sample>(
-              current_memory.current_blink_usage_kb / 1024 -
-              metrics_at_intervention_.current_blink_usage_kb / 1024));
+          blink_usage_delta);
       base::UmaHistogramSparse(
           "Memory.Experimental.OomIntervention.ReducedRendererPMFAfter20secs",
-          base::saturated_cast<base::Histogram::Sample>(
-              current_memory.current_private_footprint_kb / 1024 -
-              metrics_at_intervention_.current_private_footprint_kb / 1024));
+          private_footprint_delta);
       break;
     case 1:
       base::UmaHistogramSparse(
           "Memory.Experimental.OomIntervention.ReducedBlinkUsageAfter30secs",
-          base::saturated_cast<base::Histogram::Sample>(
-              current_memory.current_blink_usage_kb / 1024 -
-              metrics_at_intervention_.current_blink_usage_kb / 1024));
+          blink_usage_delta);
       base::UmaHistogramSparse(
           "Memory.Experimental.OomIntervention.ReducedRendererPMFAfter30secs",
-          base::saturated_cast<base::Histogram::Sample>(
-              current_memory.current_private_footprint_kb / 1024 -
-              metrics_at_intervention_.current_private_footprint_kb / 1024));
+          private_footprint_delta);
       delayed_report_timer_.Stop();
       break;
   }

third_party/blink/renderer/controller/oom_intervention_impl_test.cc

@@ -20,6 +20,7 @@
 #include "third_party/blink/renderer/core/page/page.h"
 #include "third_party/blink/renderer/core/testing/dummy_page_holder.h"
 #include "third_party/blink/renderer/core/testing/sim/sim_request.h"
+#include "third_party/blink/renderer/platform/testing/histogram_tester.h"
 #include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
 #include "third_party/blink/renderer/platform/testing/url_test_helpers.h"
 
@@ -304,4 +305,59 @@ TEST_F(OomInterventionImplTest, V2DetectionV8PurgeMemory) {
   EXPECT_TRUE(frame->GetDocument()->ExecutionContext::IsContextDestroyed());
 }
 
+TEST_F(OomInterventionImplTest, ReducedMemoryMetricReporting) {
+  HistogramTester histogram_tester;
+
+  uint64_t initial_blink_usage_bytes = kTestBlinkThreshold + 1024 * 1024 * 1024;
+  uint64_t initial_private_footprint_bytes = 0;
+
+  MemoryUsage usage;
+  // Set value more than the threshold to trigger intervention.
+  usage.v8_bytes = initial_blink_usage_bytes;
+  usage.blink_gc_bytes = 0;
+  usage.partition_alloc_bytes = 0;
+  usage.private_footprint_bytes = initial_private_footprint_bytes;
+  usage.swap_bytes = 0;
+  usage.vm_size_bytes = 0;
+  intervention_->mock_memory_usage_monitor()->SetMockMemoryUsage(usage);
+
+  Page* page = DetectOnceOnBlankPage();
+
+  EXPECT_TRUE(page->Paused());
+
+  usage.v8_bytes = initial_blink_usage_bytes - 2 * 1024 * 1024;
+  usage.private_footprint_bytes =
+      initial_private_footprint_bytes + 2 * 1024 * 1024;
+  intervention_->mock_memory_usage_monitor()->SetMockMemoryUsage(usage);
+  test::RunDelayedTasks(TimeDelta::FromSeconds(10));
+  histogram_tester.ExpectUniqueSample(
+      "Memory.Experimental.OomIntervention.ReducedBlinkUsageAfter10secs", -2,
+      1);
+  histogram_tester.ExpectUniqueSample(
+      "Memory.Experimental.OomIntervention.ReducedRendererPMFAfter10secs", 2,
+      1);
+
+  usage.v8_bytes = initial_blink_usage_bytes - 1;
+  usage.private_footprint_bytes = initial_private_footprint_bytes + 1;
+  intervention_->mock_memory_usage_monitor()->SetMockMemoryUsage(usage);
+  test::RunDelayedTasks(TimeDelta::FromSeconds(10));
+  histogram_tester.ExpectUniqueSample(
+      "Memory.Experimental.OomIntervention.ReducedBlinkUsageAfter20secs", 0, 1);
+  histogram_tester.ExpectUniqueSample(
+      "Memory.Experimental.OomIntervention.ReducedRendererPMFAfter20secs", 0,
+      1);
+
+  usage.v8_bytes = initial_blink_usage_bytes - 800 * 1024 * 1024;
+  usage.private_footprint_bytes =
+      initial_private_footprint_bytes + 800 * 1024 * 1024;
+  intervention_->mock_memory_usage_monitor()->SetMockMemoryUsage(usage);
+  test::RunDelayedTasks(TimeDelta::FromSeconds(10));
+  histogram_tester.ExpectUniqueSample(
+      "Memory.Experimental.OomIntervention.ReducedBlinkUsageAfter30secs", -500,
+      1);
+  histogram_tester.ExpectUniqueSample(
+      "Memory.Experimental.OomIntervention.ReducedRendererPMFAfter30secs", 500,
+      1);
+}
+
 }  // namespace blink