Create MemoryPressureStatsCollector.

components/memory_pressure/memory_pressure_stats_collector.cc

Index: components/memory_pressure/memory_pressure_stats_collector.cc
diff --git a/components/memory_pressure/memory_pressure_stats_collector.cc b/components/memory_pressure/memory_pressure_stats_collector.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f027282a37a88bb16a7972149f07183715863871
--- /dev/null
+++ b/components/memory_pressure/memory_pressure_stats_collector.cc
@@ -0,0 +1,157 @@
+// 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 "components/memory_pressure/memory_pressure_stats_collector.h"
+
+#include "base/metrics/histogram.h"
+
+namespace memory_pressure {
+
+namespace {
+
+using MemoryPressureLevel = MemoryPressureListener::MemoryPressureLevel;
+
+// A special memory pressure level that is used to indicate that the stats
+// collector has not yet been called.
+const MemoryPressureLevel MEMORY_PRESSURE_LEVEL_INVALID =
+    static_cast<MemoryPressureLevel>(
+        MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE - 1);
+
+// Converts a memory pressure level to an UMA enumeration value.
+MemoryPressureLevelUMA MemoryPressureLevelToUmaEnumValue(
+    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_COUNT;

[grt (UTC plus 2), 2015/09/08 17:22:46]

this will lead to an out-of-bounds write on line 77. please either handle the
boundary value down there, or return a valid value here.

[chrisha, 2015/09/08 18:48:42]

This is just to keep the compiler happy (MSVS PGO in particular gets confused
when terminating a function with a NOTREACHED and no valid return), as the
NOTREACHED will actually prevent the 'return' from executing. But I'm fine with
returning a valid value.

+}
+
+// Converts an UMA enumeration value to a memory pressure level.
+MemoryPressureLevel MemoryPressureLevelFromUmaEnumValue(
+    MemoryPressureLevelUMA level) {
+  switch (level) {
+    case UMA_MEMORY_PRESSURE_LEVEL_NONE:
+      return MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE;
+    case UMA_MEMORY_PRESSURE_LEVEL_MODERATE:
+      return MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE;
+    case UMA_MEMORY_PRESSURE_LEVEL_CRITICAL:
+      return MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL;
+  }

[grt (UTC plus 2), 2015/09/08 17:22:46]

    case UMA_MEMORY_PRESSURE_LEVEL_COUNT:
      NOTREACHED();
      break;
to avoid clang compile failure (see https://codereview.chromium.org/1320093003)

[chrisha, 2015/09/08 18:48:43]

Done.

+  NOTREACHED();
+  return MEMORY_PRESSURE_LEVEL_INVALID;

[grt (UTC plus 2), 2015/09/08 17:22:46]

returning something that isn't a real MemoryPressureLevel value seems risky. is
there a different way to handle this case?

[chrisha, 2015/09/08 18:48:42]

Same comment as above. It will never actually run, but is present to satisfy
some compilers (like your clang comment). Rewritten to return NONE.

+}
+
+}  // namespace
+
+MemoryPressureStatsCollector::MemoryPressureStatsCollector(
+    base::TickClock* tick_clock)
+    : tick_clock_(tick_clock),
+      last_pressure_level_(MEMORY_PRESSURE_LEVEL_INVALID) {}
+
+void MemoryPressureStatsCollector::UpdateStatistics(
+    MemoryPressureLevel current_pressure_level) {
+  base::TimeTicks now = tick_clock_->NowTicks();
+
+  // Special case: first call to the collector. Observations have just started
+  // so there's nothing to report.
+  if (last_pressure_level_ == MEMORY_PRESSURE_LEVEL_INVALID) {

[grt (UTC plus 2), 2015/09/08 17:22:46]

can you do away with the MEMORY_PRESSURE_LEVEL_INVALID constant an instead use:
  if (last_update_time_.is_null()) {

[chrisha, 2015/09/08 18:48:43]

sgtm. Done.

+    last_pressure_level_ = current_pressure_level;
+    last_update_time_ = now;
+    return;
+  }
+
+  // If the pressure level has transitioned then report this.
+  if (last_pressure_level_ != current_pressure_level)
+    ReportLevelChange(last_pressure_level_, current_pressure_level);
+
+  // Increment the appropriate cumulative bucket.
+  int index = MemoryPressureLevelToUmaEnumValue(current_pressure_level);
+  base::TimeDelta time_since_update = now - last_update_time_;

[grt (UTC plus 2), 2015/09/08 17:22:46]

nit: inline the delta computation on the line below

[chrisha, 2015/09/08 18:48:43]

Done.

+  unreported_cumulative_time_[index] += time_since_update;
+
+  // Update last pressure related state.
+  last_pressure_level_ = current_pressure_level;
+  last_update_time_ = now;
+
+  // Make reports about the amount of time spent cumulatively at each level.
+  for (size_t i = 0; i < arraysize(unreported_cumulative_time_); ++i) {
+    // Report the largest number of whole seconds possible at this moment and
+    // carry around the rest for a future report.
+    if (unreported_cumulative_time_[i].is_zero())
+      continue;
+    int64_t seconds = unreported_cumulative_time_[i].InSeconds();
+    if (seconds == 0)
+      continue;
+    unreported_cumulative_time_[i] -= base::TimeDelta::FromSeconds(seconds);
+
+    ReportCumulativeTime(MemoryPressureLevelFromUmaEnumValue(
+                             static_cast<MemoryPressureLevelUMA>(i)),
+                         static_cast<int>(seconds));
+  }
+}
+
+namespace {
+
+// Converts a pressure state change to an UMA enumeration value.

[grt (UTC plus 2), 2015/09/08 17:22:46]

optional nit: move this into the unnamed namespace at the top of the file.
personally, i like to have at most one block of private parts in an impl file.

[chrisha, 2015/09/08 18:48:43]

I tend to prefer having private implementation details defined closer to the
site of use, and find this pattern a great help for larger .cc files. But this
file is small enough that I'm fine moving things to the top.

+MemoryPressureLevelChangeUMA MemoryPressureLevelChangeToUmaEnumValue(
+    MemoryPressureLevel old_level,
+    MemoryPressureLevel new_level) {
+  switch (old_level) {
+    case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE: {
+      if (new_level == MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE)
+        return UMA_MEMORY_PRESSURE_LEVEL_CHANGE_NONE_TO_MODERATE;
+      if (new_level == MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL)

[grt (UTC plus 2), 2015/09/08 17:22:46]

each of these will silently fall through if a new MEMORY_PRESSURE_LEVEL is ever
added. i suggest changing these to:
  if (new_level == FOO)
    return BLAH_BLAH;
  DCHECK_EQ(BAR, new_level);
  return BLAH_BLORF;
so that there's no chance of there ever being an accidental fall-through.
alternatively, add a "break;" in each case so that the NOTREACHED is hit and
UMA_MEMORY_PRESSURE_LEVEL_CHANGE_COUNT is returned.

[chrisha, 2015/09/08 18:48:43]

I've gone with the break, as this is a case of "should never happen".

+        return UMA_MEMORY_PRESSURE_LEVEL_CHANGE_NONE_TO_CRITICAL;
+    }
+    case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE: {
+      if (new_level == MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE)
+        return UMA_MEMORY_PRESSURE_LEVEL_CHANGE_MODERATE_TO_NONE;
+      if (new_level == MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL)
+        return UMA_MEMORY_PRESSURE_LEVEL_CHANGE_MODERATE_TO_CRITICAL;
+    }
+    case MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL: {
+      if (new_level == MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE)
+        return UMA_MEMORY_PRESSURE_LEVEL_CHANGE_CRITICAL_TO_MODERATE;
+      if (new_level == MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE)
+        return UMA_MEMORY_PRESSURE_LEVEL_CHANGE_CRITICAL_TO_MODERATE;
+    }
+  }

[grt (UTC plus 2), 2015/09/08 17:22:46]

add a case for UMA_MEMORY_PRESSURE_LEVEL_COUNT that does a NOTREACHED(); break;
so that clang doesn't complain after you land this.

[chrisha, 2015/09/08 18:48:43]

This switches off the MemoryPressureListern enum, which has no corresponding
_COUNT value. Should have no problems with clang.

[grt (UTC plus 2), 2015/09/08 19:22:10]

oh yeah, duh.

[chrisha, 2015/09/08 19:57:53]

Acknowledged.

+  NOTREACHED();
+  return UMA_MEMORY_PRESSURE_LEVEL_CHANGE_COUNT;
+}
+
+}  // namespace
+
+// static
+void MemoryPressureStatsCollector::ReportCumulativeTime(
+    MemoryPressureLevel pressure_level,
+    int seconds) {
+  // 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(pressure_level), seconds),
+      base::LinearHistogram::FactoryGet(
+          "Memory.PressureLevel", 1, UMA_MEMORY_PRESSURE_LEVEL_COUNT,
+          UMA_MEMORY_PRESSURE_LEVEL_COUNT + 1,
+          base::HistogramBase::kUmaTargetedHistogramFlag));
+}
+
+// static
+void MemoryPressureStatsCollector::ReportLevelChange(
+    MemoryPressureLevel old_pressure_level,
+    MemoryPressureLevel new_pressure_level) {
+  UMA_HISTOGRAM_ENUMERATION("Memory.PressureLevelChange",
+                            MemoryPressureLevelChangeToUmaEnumValue(
+                                old_pressure_level, new_pressure_level),
+                            UMA_MEMORY_PRESSURE_LEVEL_CHANGE_COUNT);
+}
+
+}  // namespace memory_pressure