Per offline discussion, leaving "ChromeOS.MemoryPressureLevel" code in place...

Per offline discussion, leaving "ChromeOS.MemoryPressureLevel" code in place for ChromeOS while history builds up in "Memory.PressureLevel"

BUG=655304

Review-Url: https://codereview.chromium.org/2434103003
Cr-Commit-Position: refs/heads/master@{#430292}

base/BUILD.gn

@@ -1843,6 +1843,7 @@ test("base_unittests") {
     "memory/memory_pressure_listener_unittest.cc",
     "memory/memory_pressure_monitor_chromeos_unittest.cc",
     "memory/memory_pressure_monitor_mac_unittest.cc",
+    "memory/memory_pressure_monitor_unittest.cc",
     "memory/memory_pressure_monitor_win_unittest.cc",
     "memory/ptr_util_unittest.cc",
     "memory/ref_counted_memory_unittest.cc",

base/memory/memory_pressure_monitor.cc

@@ -5,12 +5,39 @@
 #include "base/memory/memory_pressure_monitor.h"
 
 #include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
 
 namespace base {
 namespace {
 
 MemoryPressureMonitor* g_monitor = nullptr;
 
+// Enumeration of UMA memory pressure levels. This needs to be kept in sync with
+// histograms.xml and the memory pressure levels defined in
+// MemoryPressureListener.
+enum MemoryPressureLevelUMA {
+  UMA_MEMORY_PRESSURE_LEVEL_NONE = 0,
+  UMA_MEMORY_PRESSURE_LEVEL_MODERATE = 1,
+  UMA_MEMORY_PRESSURE_LEVEL_CRITICAL = 2,
+  // This must be the last value in the enum.
+  UMA_MEMORY_PRESSURE_LEVEL_COUNT,
+};
+
+// Converts a memory pressure level to an UMA enumeration value.
+MemoryPressureLevelUMA MemoryPressureLevelToUmaEnumValue(
+    base::MemoryPressureListener::MemoryPressureLevel level) {
+  switch (level) {
+    case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE:
+      return UMA_MEMORY_PRESSURE_LEVEL_NONE;
+    case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE:
+      return UMA_MEMORY_PRESSURE_LEVEL_MODERATE;
+    case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL:
+      return UMA_MEMORY_PRESSURE_LEVEL_CRITICAL;
+  }
+  NOTREACHED();
+  return UMA_MEMORY_PRESSURE_LEVEL_NONE;
+}
+
 }  // namespace
 
 MemoryPressureMonitor::MemoryPressureMonitor() {
@@ -27,5 +54,18 @@ MemoryPressureMonitor::~MemoryPressureMonitor() {
 MemoryPressureMonitor* MemoryPressureMonitor::Get() {
   return g_monitor;
 }
+void MemoryPressureMonitor::RecordMemoryPressure(
+    base::MemoryPressureListener::MemoryPressureLevel level,
+    int ticks) {
+  // Use the more primitive STATIC_HISTOGRAM_POINTER_BLOCK macro because the
+  // simple UMA_HISTOGRAM macros don't expose 'AddCount' functionality.
+  STATIC_HISTOGRAM_POINTER_BLOCK(
+      "Memory.PressureLevel",
+      AddCount(MemoryPressureLevelToUmaEnumValue(level), ticks),
+      base::LinearHistogram::FactoryGet(
+          "Memory.PressureLevel", 1, UMA_MEMORY_PRESSURE_LEVEL_COUNT,
+          UMA_MEMORY_PRESSURE_LEVEL_COUNT + 1,
+          base::HistogramBase::kUmaTargetedHistogramFlag));
+}
 
 }  // namespace base

base/memory/memory_pressure_monitor.h

@@ -31,6 +31,9 @@ class BASE_EXPORT MemoryPressureMonitor {
   // Return the singleton MemoryPressureMonitor.
   static MemoryPressureMonitor* Get();
 
+  // Record memory pressure UMA statistic. A tick is 5 seconds.
+  static void RecordMemoryPressure(MemoryPressureLevel level, int ticks);
+
   // Returns the currently observed memory pressure.
   virtual MemoryPressureLevel GetCurrentPressureLevel() const = 0;
 

base/memory/memory_pressure_monitor_chromeos.cc

@@ -108,6 +108,7 @@ MemoryPressureMonitor::MemoryPressureMonitor(
     : current_memory_pressure_level_(
           MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE),
       moderate_pressure_repeat_count_(0),
+      seconds_since_reporting_(0),
       moderate_pressure_threshold_percent_(
           GetModerateMemoryThresholdInPercent(thresholds)),
       critical_pressure_threshold_percent_(
@@ -158,8 +159,13 @@ void MemoryPressureMonitor::StopObserving() {
 
 void MemoryPressureMonitor::CheckMemoryPressureAndRecordStatistics() {
   CheckMemoryPressure();
-
+  if (seconds_since_reporting_++ == 5) {
+    seconds_since_reporting_ = 0;
+    RecordMemoryPressure(current_memory_pressure_level_, 1);
+  }
   // Record UMA histogram statistics for the current memory pressure level.
+  // TODO(lgrey): Remove this once there's a usable history for the
+  // "Memory.PressureLevel" statistic
   MemoryPressureLevelUMA memory_pressure_level_uma(MEMORY_PRESSURE_LEVEL_NONE);
   switch (current_memory_pressure_level_) {
     case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE:

base/memory/memory_pressure_monitor_chromeos.h

@@ -101,6 +101,13 @@ class BASE_EXPORT MemoryPressureMonitor : public base::MemoryPressureMonitor {
   // gets used to count the number of events since the last event occured.
   int moderate_pressure_repeat_count_;
 
+  // The "Memory.PressureLevel" statistic is recorded every
+  // 5 seconds, but the timer to report "ChromeOS.MemoryPressureLevel"
+  // fires every second. This counter is used to allow reporting
+  // "Memory.PressureLevel" correctly without adding another
+  // timer.
+  int seconds_since_reporting_;
+
   // The thresholds for moderate and critical pressure.
   const int moderate_pressure_threshold_percent_;
   const int critical_pressure_threshold_percent_;

base/memory/memory_pressure_monitor_mac.cc

@@ -8,6 +8,8 @@
 #include <stddef.h>
 #include <sys/sysctl.h>
 
+#include <cmath>
+
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/mac/mac_util.h"
@@ -33,29 +35,21 @@ MemoryPressureMonitor::MemoryPressureLevelForMacMemoryPressure(
   return MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE;
 }
 
-void MemoryPressureMonitor::NotifyMemoryPressureChanged(
-    dispatch_source_s* event_source,
-    const MemoryPressureMonitor::DispatchCallback& dispatch_callback) {
-  int mac_memory_pressure = dispatch_source_get_data(event_source);
-  MemoryPressureListener::MemoryPressureLevel memory_pressure_level =
-      MemoryPressureLevelForMacMemoryPressure(mac_memory_pressure);
-  dispatch_callback.Run(memory_pressure_level);
-}
-
 MemoryPressureMonitor::MemoryPressureMonitor()
-    // The MemoryPressureListener doesn't want to know about transitions to
-    // MEMORY_PRESSURE_LEVEL_NONE so don't watch for
-    // DISPATCH_MEMORYPRESSURE_NORMAL notifications.
     : memory_level_event_source_(dispatch_source_create(
           DISPATCH_SOURCE_TYPE_MEMORYPRESSURE,
           0,
-          DISPATCH_MEMORYPRESSURE_WARN | DISPATCH_MEMORYPRESSURE_CRITICAL,
+          DISPATCH_MEMORYPRESSURE_WARN | DISPATCH_MEMORYPRESSURE_CRITICAL |
+              DISPATCH_MEMORYPRESSURE_NORMAL,
           dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0))),
       dispatch_callback_(
-          base::Bind(&MemoryPressureListener::NotifyMemoryPressure)) {
+          base::Bind(&MemoryPressureListener::NotifyMemoryPressure)),
+      last_pressure_change_(CFAbsoluteTimeGetCurrent()),
+      reporting_error_(0) {
+  last_pressure_level_ = GetCurrentPressureLevel();
   dispatch_source_set_event_handler(memory_level_event_source_, ^{
-    NotifyMemoryPressureChanged(memory_level_event_source_.get(),
-                                dispatch_callback_);
+    OnMemoryPressureChanged(memory_level_event_source_.get(),
+                            dispatch_callback_);
   });
   dispatch_resume(memory_level_event_source_);
 }
@@ -72,6 +66,35 @@ MemoryPressureMonitor::GetCurrentPressureLevel() const {
                &length, nullptr, 0);
   return MemoryPressureLevelForMacMemoryPressure(mac_memory_pressure);
 }
+void MemoryPressureMonitor::OnMemoryPressureChanged(
+    dispatch_source_s* event_source,
+    const MemoryPressureMonitor::DispatchCallback& dispatch_callback) {
+  int mac_memory_pressure = dispatch_source_get_data(event_source);
+  MemoryPressureListener::MemoryPressureLevel memory_pressure_level =
+      MemoryPressureLevelForMacMemoryPressure(mac_memory_pressure);
+  CFTimeInterval now = CFAbsoluteTimeGetCurrent();
+  CFTimeInterval since_last_change = now - last_pressure_change_;
+  last_pressure_change_ = now;
+
+  double ticks_to_report;
+  reporting_error_ =
+      modf(since_last_change + reporting_error_, &ticks_to_report);
+
+  // Sierra fails to call the handler when pressure returns to normal,
+  // which would skew our data. For example, if pressure went to 'warn'
+  // at T0, back to 'normal' at T1, then to 'critical' at T10, we would
+  // report 10 ticks of 'warn' instead of 1 tick of 'warn' and 9 ticks
+  // of 'normal'.
+  // This is rdar://29114314
+  if (mac::IsAtMostOS10_11())
+    RecordMemoryPressure(last_pressure_level_,
+                         static_cast<int>(ticks_to_report));
+
+  last_pressure_level_ = memory_pressure_level;
+  if (memory_pressure_level !=
+      MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE)
+    dispatch_callback.Run(memory_pressure_level);
+}
 
 void MemoryPressureMonitor::SetDispatchCallback(
     const DispatchCallback& callback) {

base/memory/memory_pressure_monitor_mac.h

@@ -5,6 +5,7 @@
 #ifndef BASE_MEMORY_MEMORY_PRESSURE_MONITOR_MAC_H_
 #define BASE_MEMORY_MEMORY_PRESSURE_MONITOR_MAC_H_
 
+#include <CoreFoundation/CFDate.h>
 #include <dispatch/dispatch.h>
 
 #include "base/base_export.h"
@@ -35,14 +36,22 @@ class BASE_EXPORT MemoryPressureMonitor : public base::MemoryPressureMonitor {
 
   static MemoryPressureLevel
       MemoryPressureLevelForMacMemoryPressure(int mac_memory_pressure);
-  static void NotifyMemoryPressureChanged(
-      dispatch_source_s* event_source,
-      const DispatchCallback& dispatch_callback);
+  void OnMemoryPressureChanged(dispatch_source_s* event_source,
+                               const DispatchCallback& dispatch_callback);
 
   ScopedDispatchObject<dispatch_source_t> memory_level_event_source_;
 
   DispatchCallback dispatch_callback_;
 
+  CFTimeInterval last_pressure_change_;
+
+  MemoryPressureLevel last_pressure_level_;
+
+  // The UMA statistic is recorded in 5 second increments. This
+  // accumulates the remaining time to be rolled into the next
+  // call.
+  CFTimeInterval reporting_error_;
+
   DISALLOW_COPY_AND_ASSIGN(MemoryPressureMonitor);
 };
 

base/memory/memory_pressure_monitor_unittest.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/memory/memory_pressure_monitor.h"
+
+#include "base/macros.h"
+#include "base/memory/memory_pressure_listener.h"
+#include "base/test/histogram_tester.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+
+TEST(MemoryPressureMonitorTest, RecordMemoryPressure) {
+  base::HistogramTester tester;
+  const char* kHistogram = "Memory.PressureLevel";
+
+  MemoryPressureMonitor::RecordMemoryPressure(
+      MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE, 3);
+  tester.ExpectTotalCount(kHistogram, 3);
+  tester.ExpectBucketCount(kHistogram, 0, 3);
+
+  MemoryPressureMonitor::RecordMemoryPressure(
+      MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE, 2);
+  tester.ExpectTotalCount(kHistogram, 5);
+  tester.ExpectBucketCount(kHistogram, 1, 2);
+
+  MemoryPressureMonitor::RecordMemoryPressure(
+      MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL, 1);
+  tester.ExpectTotalCount(kHistogram, 6);
+  tester.ExpectBucketCount(kHistogram, 2, 1);
+}
+}  // namespace base

base/memory/memory_pressure_monitor_win.cc

@@ -6,7 +6,6 @@
 
 #include <windows.h>
 
-#include "base/metrics/histogram_macros.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
@@ -18,32 +17,6 @@ namespace {
 
 static const DWORDLONG kMBBytes = 1024 * 1024;
 
-// Enumeration of UMA memory pressure levels. This needs to be kept in sync with
-// histograms.xml and the memory pressure levels defined in
-// MemoryPressureListener.
-enum MemoryPressureLevelUMA {
-  UMA_MEMORY_PRESSURE_LEVEL_NONE = 0,
-  UMA_MEMORY_PRESSURE_LEVEL_MODERATE = 1,
-  UMA_MEMORY_PRESSURE_LEVEL_CRITICAL = 2,
-  // This must be the last value in the enum.
-  UMA_MEMORY_PRESSURE_LEVEL_COUNT,
-};
-
-// Converts a memory pressure level to an UMA enumeration value.
-MemoryPressureLevelUMA MemoryPressureLevelToUmaEnumValue(
-    MemoryPressureListener::MemoryPressureLevel level) {
-  switch (level) {
-    case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE:
-      return UMA_MEMORY_PRESSURE_LEVEL_NONE;
-    case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE:
-      return UMA_MEMORY_PRESSURE_LEVEL_MODERATE;
-    case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL:
-      return UMA_MEMORY_PRESSURE_LEVEL_CRITICAL;
-  }
-  NOTREACHED();
-  return UMA_MEMORY_PRESSURE_LEVEL_NONE;
-}
-
 }  // namespace
 
 // The following constants have been lifted from similar values in the ChromeOS
@@ -210,10 +183,7 @@ void MemoryPressureMonitor::CheckMemoryPressureAndRecordStatistics() {
 
   CheckMemoryPressure();
 
-  UMA_HISTOGRAM_ENUMERATION(
-      "Memory.PressureLevel",
-      MemoryPressureLevelToUmaEnumValue(current_memory_pressure_level_),
-      UMA_MEMORY_PRESSURE_LEVEL_COUNT);
+  RecordMemoryPressure(current_memory_pressure_level_, 1);
 }
 
 MemoryPressureListener::MemoryPressureLevel