Reenable Purge+Suspend Memory Growth metrics.

content/renderer/render_thread_impl.cc

Index: content/renderer/render_thread_impl.cc
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc
index f1130851bcb1adc31dd82aed3e7ee0f58d140706..a612f7bec64eb9f1aa964a235b4f7502fea6c941 100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -23,6 +23,7 @@
 #include "base/memory/shared_memory.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/field_trial.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/path_service.h"
 #include "base/process/process_metrics.h"
@@ -1695,6 +1696,24 @@ void RenderThreadImpl::OnProcessPurgeAndSuspend() {
     return;
 
   purge_and_suspend_memory_metrics_ = memory_metrics;
+  GetRendererScheduler()->DefaultTaskRunner()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&RenderThreadImpl::RecordPurgeAndSuspendMemoryGrowthMetrics,
+                 base::Unretained(this), process_foregrounded_count_,
+                 RECORD_30MIN_AFTER_PURGE),
+      base::TimeDelta::FromMinutes(30));
+  GetRendererScheduler()->DefaultTaskRunner()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&RenderThreadImpl::RecordPurgeAndSuspendMemoryGrowthMetrics,
+                 base::Unretained(this), process_foregrounded_count_,
+                 RECORD_60MIN_AFTER_PURGE),
+      base::TimeDelta::FromMinutes(60));
+  GetRendererScheduler()->DefaultTaskRunner()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&RenderThreadImpl::RecordPurgeAndSuspendMemoryGrowthMetrics,
+                 base::Unretained(this), process_foregrounded_count_,
+                 RECORD_90MIN_AFTER_PURGE),
+      base::TimeDelta::FromMinutes(90));
 }
 
 // TODO(tasak): Replace the following GetMallocUsage() with memory-infra
@@ -1796,16 +1815,39 @@ bool RenderThreadImpl::GetRendererMemoryMetrics(
        ? current.allocator - previous.allocator         \
        : 0)
 
-#define UMA_HISTOGRAM_MEMORY_GROWTH_KB(basename, suffix, memory_usage) \
-  {                                                                    \
-    std::string histogram_name =                                       \
-        base::StringPrintf("%s.%s", basename, suffix);                 \
-    UMA_HISTOGRAM_MEMORY_KB(histogram_name, memory_usage);             \
-  }
+#define UMA_PURGE_AND_SUSPEND_MEMORY_GROWTH(suffix, memory_metrics)        \

[Ilya Sherman, 2017/05/23 15:21:58]

Rather than using a macro here, please revert to passing a string suffix, and
just append the suffix below.

[tasak, 2017/05/24 05:08:19]

I see. I misunderstood the previous comment.
So by using base::UmaHistogramMemoryKB, it is possible to use generated
histogram names....

Done.

+  base::UmaHistogramMemoryKB(                                              \
+      "PurgeAndSuspend.Experimental.MemoryGrowth.PartitionAllocKB" suffix, \

[Ilya Sherman, 2017/05/23 15:21:58]

By the way, I think you lost the final dot in the histogram name?

[tasak, 2017/05/24 05:08:19]

Yeah, it's a critical bug... Thanks.
Fixed by reverting the code.

+      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_, \
+                        partition_alloc_kb));                              \
+  base::UmaHistogramMemoryKB(                                              \
+      "PurgeAndSuspend.Experimental.MemoryGrowth.BlinkGCKB" suffix,        \
+      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_, \
+                        blink_gc_kb));                                     \
+  base::UmaHistogramMemoryKB(                                              \
+      "PurgeAndSuspend.Experimental.MemoryGrowth.MallocKB" suffix,         \
+      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_, \
+                        malloc_mb) *                                       \
+          1024);                                                           \
+  base::UmaHistogramMemoryKB(                                              \
+      "PurgeAndSuspend.Experimental.MemoryGrowth.DiscardableKB" suffix,    \
+      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_, \
+                        discardable_kb));                                  \
+  base::UmaHistogramMemoryKB(                                              \
+      "PurgeAndSuspend.Experimental.MemoryGrowth."                         \
+      "V8MainThreadIsolateKB" suffix,                                      \
+      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_, \
+                        v8_main_thread_isolate_mb) *                       \
+          1024);                                                           \
+  base::UmaHistogramMemoryKB(                                              \
+      "PurgeAndSuspend.Experimental.MemoryGrowth.TotalAllocatedKB" suffix, \
+      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_, \
+                        total_allocated_mb) *                              \
+          1024);
 
 void RenderThreadImpl::RecordPurgeAndSuspendMemoryGrowthMetrics(
-    const char* suffix,
-    int foregrounded_count_when_purged) {
+    int foregrounded_count_when_purged,
+    RenderThreadImpl::RecordMemoryGrowthTiming record_timing) {
   // If this renderer is resumed, we should not update UMA.
   if (!RendererIsHidden())
     return;
@@ -1816,33 +1858,17 @@ void RenderThreadImpl::RecordPurgeAndSuspendMemoryGrowthMetrics(
   if (!GetRendererMemoryMetrics(&memory_metrics))
     return;
 
-  UMA_HISTOGRAM_MEMORY_GROWTH_KB(
-      "PurgeAndSuspend.Experimental.MemoryGrowth.PartitionAllocKB", suffix,
-      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,
-                        partition_alloc_kb));
-  UMA_HISTOGRAM_MEMORY_GROWTH_KB(
-      "PurgeAndSuspend.Experimental.MemoryGrowth.BlinkGCKB", suffix,
-      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,
-                        blink_gc_kb));
-  UMA_HISTOGRAM_MEMORY_GROWTH_KB(
-      "PurgeAndSuspend.Experimental.MemoryGrowth.MallocKB", suffix,
-      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,
-                        malloc_mb) *
-          1024);
-  UMA_HISTOGRAM_MEMORY_GROWTH_KB(
-      "PurgeAndSuspend.Experimental.MemoryGrowth.DiscardableKB", suffix,
-      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,
-                        discardable_kb));
-  UMA_HISTOGRAM_MEMORY_GROWTH_KB(
-      "PurgeAndSuspend.Experimental.MemoryGrowth.V8MainThreadIsolateKB", suffix,
-      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,
-                        v8_main_thread_isolate_mb) *
-          1024);
-  UMA_HISTOGRAM_MEMORY_GROWTH_KB(
-      "PurgeAndSuspend.Experimental.MemoryGrowth.TotalAllocatedKB", suffix,
-      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,
-                        total_allocated_mb) *
-          1024);
+  switch (record_timing) {
+    case RECORD_30MIN_AFTER_PURGE:
+      UMA_PURGE_AND_SUSPEND_MEMORY_GROWTH("30min", memory_metrics);
+      break;
+    case RECORD_60MIN_AFTER_PURGE:
+      UMA_PURGE_AND_SUSPEND_MEMORY_GROWTH("60min", memory_metrics);
+      break;
+    case RECORD_90MIN_AFTER_PURGE:
+      UMA_PURGE_AND_SUSPEND_MEMORY_GROWTH("90min", memory_metrics);
+      break;
+  }
 }
 
 scoped_refptr<gpu::GpuChannelHost> RenderThreadImpl::EstablishGpuChannelSync() {