[Mac] Report statistics more regularly in Mac memory pressure monitor

base/memory/memory_pressure_monitor_mac.cc

 // Copyright 2015 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_mac.h"
 
 #include <dlfcn.h>
 #include <stddef.h>
 #include <sys/sysctl.h>
 
@@ skipping 26 matching lines @@
 
 MemoryPressureMonitor::MemoryPressureMonitor()
     : memory_level_event_source_(dispatch_source_create(
           DISPATCH_SOURCE_TYPE_MEMORYPRESSURE,
           0,
           DISPATCH_MEMORYPRESSURE_WARN | DISPATCH_MEMORYPRESSURE_CRITICAL |
               DISPATCH_MEMORYPRESSURE_NORMAL,
           dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0))),
       dispatch_callback_(
           base::Bind(&MemoryPressureListener::NotifyMemoryPressure)),
-      last_pressure_change_(CFAbsoluteTimeGetCurrent()),
+      last_statistic_report_(CFAbsoluteTimeGetCurrent()),

[Mark Mentovai, 2016/11/14 19:12:15]

Is this an OK initial value? It’s not associated with a valid
last_pressure_level_.

[lgrey, 2016/11/14 19:23:51]

I think so. Worst case scenario is the application is launched in warn/critical
and we report an extra tick of normal.

+      last_pressure_level_(MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE),
       reporting_error_(0) {
-  last_pressure_level_ = GetCurrentPressureLevel();
   dispatch_source_set_event_handler(memory_level_event_source_, ^{
     OnMemoryPressureChanged(memory_level_event_source_.get(),
                             dispatch_callback_);
   });
   dispatch_resume(memory_level_event_source_);
 }
 
 MemoryPressureMonitor::~MemoryPressureMonitor() {
   dispatch_source_cancel(memory_level_event_source_);
 }
 
 MemoryPressureListener::MemoryPressureLevel
-MemoryPressureMonitor::GetCurrentPressureLevel() const {
+MemoryPressureMonitor::GetCurrentPressureLevel() {
   int mac_memory_pressure;
   size_t length = sizeof(int);
   sysctlbyname("kern.memorystatus_vm_pressure_level", &mac_memory_pressure,
                &length, nullptr, 0);
-  return MemoryPressureLevelForMacMemoryPressure(mac_memory_pressure);
+  MemoryPressureListener::MemoryPressureLevel memory_pressure_level =
+      MemoryPressureLevelForMacMemoryPressure(mac_memory_pressure);
+  bool pressure_level_changed = false;
+  if (last_pressure_level_ != memory_pressure_level) {
+    pressure_level_changed = true;
+  }
+  SendStatisticsIfNecessary(pressure_level_changed);
+  last_pressure_level_ = memory_pressure_level;
+  return memory_pressure_level;
 }
+
 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));
-
+  bool pressure_level_changed = false;
+  if (last_pressure_level_ != memory_pressure_level) {
+    pressure_level_changed = true;
+  }
+  SendStatisticsIfNecessary(pressure_level_changed);
   last_pressure_level_ = memory_pressure_level;
   if (memory_pressure_level !=
       MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE)
     dispatch_callback.Run(memory_pressure_level);
 }
 
+void MemoryPressureMonitor::SendStatisticsIfNecessary(
+    bool pressure_level_changed) {
+  CFTimeInterval now = CFAbsoluteTimeGetCurrent();
+  CFTimeInterval since_last_report = now - last_statistic_report_;
+  last_statistic_report_ = now;
+
+  double ticks_to_report;
+  reporting_error_ =
+      modf(since_last_report + reporting_error_, &ticks_to_report);

[Mark Mentovai, 2016/11/14 19:12:15]

Good practice is to not use modf(), but instead use std::modf(). This is tricky:
they’re different. modf() is the C function that works on doubles. std::modf()
is the C++11 function that has overloads for different floating-point types.
Using the std:: version keeps precision constant throughout a pipeline of
floating-point operations without requiring anyone to remember whether
CFTimeInterval is a float or a double.

This file already #include <cmath>, so std::modf() is available.

This comment is true of a lot of the <math.h> stuff: use <cmath> instead, but be
sure to use std::.

[lgrey, 2016/11/14 19:23:51]

Done.

+
+  // Round up on change to ensure we capture it
+  if (pressure_level_changed && ticks_to_report < 0)
+    ticks_to_report = 1;
+
+  if (ticks_to_report > 0)
+    RecordMemoryPressure(last_pressure_level_,
+                         static_cast<int>(ticks_to_report));
+}
+
 void MemoryPressureMonitor::SetDispatchCallback(
     const DispatchCallback& callback) {
   dispatch_callback_ = callback;
 }
 
 }  // namespace mac
 }  // namespace base