Add Mac memory pressure statistic reporting and consolidate platform code

base/memory/memory_pressure_monitor_mac.cc

Index: base/memory/memory_pressure_monitor_mac.cc
diff --git a/base/memory/memory_pressure_monitor_mac.cc b/base/memory/memory_pressure_monitor_mac.cc
index d3430e94ca45a1ad9150c3cfe8f3342faf02ffe6..5207aa5cde4101076f14c1a038545d7a247150b4 100644
--- a/base/memory/memory_pressure_monitor_mac.cc
+++ b/base/memory/memory_pressure_monitor_mac.cc
@@ -33,26 +33,17 @@ 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)),

[shrike, 2016/10/20 21:04:31]

I think MemoryPressureListener::NotifyMemoryPressure() still needs to be static?
I haven't used Bind() that much, but I'm not seeing from this declaration how it
can call MemoryPressureListener::NotifyMemoryPressure() on this particular
instance.

[lgrey, 2016/10/21 14:15:04]

This threw me for a loop for a bit, then I realized: the method I changed from
static is "MemoryPressureMonitor::NotifyMemoryPressureChanged" which is called
directly inside the event handler vs.
"MemoryPressureListener::NotifyMemoryPressure" which is defined elsewhere. 

I mixed these two up earlier when I was working on this as well; would it be
worth it to rename "NotifyMemoryPressureChanged" so they're more distinct?

[chrisha, 2016/10/21 18:18:58]

Yeah, the rename might help. Maybe "OnMemoryPressureChanged"?

[lgrey, 2016/10/21 18:39:35]

Done.

+      last_pressure_change_(CFAbsoluteTimeGetCurrent()) {
+  last_pressure_level_ = GetCurrentPressureLevel();
   dispatch_source_set_event_handler(memory_level_event_source_, ^{
     NotifyMemoryPressureChanged(memory_level_event_source_.get(),
                                 dispatch_callback_);
@@ -72,6 +63,28 @@ MemoryPressureMonitor::GetCurrentPressureLevel() const {
                &length, nullptr, 0);
   return MemoryPressureLevelForMacMemoryPressure(mac_memory_pressure);
 }
+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);
+  CFTimeInterval now = CFAbsoluteTimeGetCurrent();
+  CFTimeInterval since_last_change = now - last_pressure_change_;
+  last_pressure_change_ = now;
+
+  // TODO(lgrey): Two questions:
+  // 1) Is it OK to give up precision by flooring this?
+  // 2) Is there a better way to send multiple events?
+  int last_level_seconds = static_cast<int>(since_last_change);
+  for (int i = 0; i < last_level_seconds; i++) {

[shrike, 2016/10/20 21:04:30]

This is unfortunate, because last_level_seconds could be quite large. I wonder
if the UMA folks might have a better way of doing this (someone like
asvitkine@).

[lgrey, 2016/10/21 14:15:04]

asvitkine@ what do you think?

For context, this particular stat is an enumeration, but it's counting
cumulative seconds in a given memory pressure state. Windows and ChromeOS poll
every second, but Mac gets notifications on change, hence this weird workaround.
Is there any way to put more than one incidence into an enumeration bucket in a
single call?

Another possibility is we could decouple the state change notifications from
reporting with a separate timer just to report the current state.

[chrisha, 2016/10/21 18:18:58]

We had talked about switching to polling on Mac, as shrike@ had found that the
messages could sometimes be delayed by a long period of time (ie, up to a
minute).

Do we just want to move to a polling solution to be uniform? (Also, IIRC the
polling is currently at 5 seconds?)

(This increases idle cost, but we have some fun new timer coalescing stuff on
the way that will mitigate this.)

[lgrey, 2016/10/21 18:39:35]

My mistake, it looks like Chrome polls once a second and Windows polls every 5
seconds. Is the right move to standardize everything at 5 (for
Memory.PressureLevel at least)? 

+    RecordMemoryPressure(last_pressure_level_);
+  }
+  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) {