Record renderer's memory growth 30min, 60min and 90min after purging separately.

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 9dad0206afcd8f05445f0486fe1a066391a0249b..0c148581178d41c5f960095f6706cd42c80f70c6 100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -247,6 +247,9 @@ const int kMaxResourceRequestsPerFlushWhenThrottled = 8;
 #endif
 const double kThrottledResourceRequestFlushPeriodS = 1. / 60.;
 
+// Max count of reporting PurgeAndSuspend.Experimental.MemoryGrowth.*.
+const int kMaxCountOfReportMemoryGrowthMetrics = 2;
+
 // Maximum allocation size allowed for image scaling filters that
 // require pre-scaling. Skia will fallback to a filter that doesn't
 // require pre-scaling if the default filter would require an
@@ -904,6 +907,7 @@ void RenderThreadImpl::Init(
   record_purge_suspend_growth_metric_closure_.Reset(
       base::Bind(&RenderThreadImpl::RecordPurgeAndSuspendMemoryGrowthMetrics,
                  base::Unretained(this)));
+  memory_growth_report_count_ = 0;
   needs_to_record_first_active_paint_ = false;
 
   base::MemoryCoordinatorClientRegistry::GetInstance()->Register(this);
@@ -1683,15 +1687,10 @@ void RenderThreadImpl::OnProcessPurgeAndSuspend() {
 
   purge_and_suspend_memory_metrics_ = memory_metrics;
   // record how many memory usage increases after purged.
+  memory_growth_report_count_ = 0;
   GetRendererScheduler()->DefaultTaskRunner()->PostDelayedTask(
       FROM_HERE, record_purge_suspend_growth_metric_closure_.callback(),
-      base::TimeDelta::FromMinutes(5));
-  GetRendererScheduler()->DefaultTaskRunner()->PostDelayedTask(
-      FROM_HERE, record_purge_suspend_growth_metric_closure_.callback(),
-      base::TimeDelta::FromMinutes(10));
-  GetRendererScheduler()->DefaultTaskRunner()->PostDelayedTask(
-      FROM_HERE, record_purge_suspend_growth_metric_closure_.callback(),
-      base::TimeDelta::FromMinutes(15));

[Ilya Sherman, 2017/05/09 03:58:58]

I think this previous structure was both simpler and clearer.  Why did you
decide to move away from it?  IMO it would be much nicer to simply bind the
strings "30min" etc. here.

[tasak, 2017/05/10 09:57:02]

Done.

+      base::TimeDelta::FromMinutes(30));
 }
 
 // TODO(tasak): Replace the following GetMallocUsage() with memory-infra
@@ -1793,39 +1792,63 @@ bool RenderThreadImpl::GetRendererMemoryMetrics(
        ? current.allocator - previous.allocator         \
        : 0)
 
-void RenderThreadImpl::RecordPurgeAndSuspendMemoryGrowthMetrics() const {
+#define UMA_HISTOGRAM_MEMORY_GROWTH(name)                                     \
+  UMA_HISTOGRAM_MEMORY_KB(                                                    \
+      "PurgeAndSuspend.Experimental.MemoryGrowth.PartitionAllocKB" name,      \
+      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,    \
+                        partition_alloc_kb));                                 \
+  UMA_HISTOGRAM_MEMORY_KB(                                                    \
+      "PurgeAndSuspend.Experimental.MemoryGrowth.BlinkGCKB" name,             \
+      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,    \
+                        blink_gc_kb));                                        \
+  UMA_HISTOGRAM_MEMORY_KB(                                                    \
+      "PurgeAndSuspend.Experimental.MemoryGrowth.MallocKB" name,              \
+      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,    \
+                        malloc_mb) *                                          \
+          1024);                                                              \
+  UMA_HISTOGRAM_MEMORY_KB(                                                    \
+      "PurgeAndSuspend.Experimental.MemoryGrowth.DiscardableKB" name,         \
+      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,    \
+                        discardable_kb));                                     \
+  UMA_HISTOGRAM_MEMORY_KB(                                                    \
+      "PurgeAndSuspend.Experimental.MemoryGrowth.V8MainThreadIsolateKB" name, \
+      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,    \
+                        v8_main_thread_isolate_mb) *                          \
+          1024);                                                              \
+  UMA_HISTOGRAM_MEMORY_KB(                                                    \
+      "PurgeAndSuspend.Experimental.MemoryGrowth.TotalAllocatedKB" name,      \
+      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,    \
+                        total_allocated_mb) *                                 \
+          1024);
+
+void RenderThreadImpl::RecordPurgeAndSuspendMemoryGrowthMetrics() {
   // If this renderer is resumed, we should not update UMA.
   if (!RendererIsHidden())
     return;
+  if (memory_growth_report_count_ > kMaxCountOfReportMemoryGrowthMetrics)
+    return;
 
   RendererMemoryMetrics memory_metrics;
   if (!GetRendererMemoryMetrics(&memory_metrics))
     return;
 
-  UMA_HISTOGRAM_MEMORY_KB(
-      "PurgeAndSuspend.Experimental.MemoryGrowth.PartitionAllocKB",
-      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,
-                        partition_alloc_kb));
-  UMA_HISTOGRAM_MEMORY_KB(
-      "PurgeAndSuspend.Experimental.MemoryGrowth.BlinkGCKB",
-      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,
-                        blink_gc_kb));
-  UMA_HISTOGRAM_MEMORY_KB(
-      "PurgeAndSuspend.Experimental.MemoryGrowth.MallocKB",
-      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,
-                        malloc_mb) * 1024);
-  UMA_HISTOGRAM_MEMORY_KB(
-      "PurgeAndSuspend.Experimental.MemoryGrowth.DiscardableKB",
-      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,
-                        discardable_kb));
-  UMA_HISTOGRAM_MEMORY_KB(
-      "PurgeAndSuspend.Experimental.MemoryGrowth.V8MainThreadIsolateKB",
-      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,
-                        v8_main_thread_isolate_mb) * 1024);
-  UMA_HISTOGRAM_MEMORY_KB(
-      "PurgeAndSuspend.Experimental.MemoryGrowth.TotalAllocatedKB",
-      GET_MEMORY_GROWTH(memory_metrics, purge_and_suspend_memory_metrics_,
-                        total_allocated_mb) * 1024);
+  switch (memory_growth_report_count_) {
+    case 0:
+      UMA_HISTOGRAM_MEMORY_GROWTH(".30min");
+      ++memory_growth_report_count_;
+      break;
+    case 1:
+      UMA_HISTOGRAM_MEMORY_GROWTH(".60min");
+      ++memory_growth_report_count_;
+      break;
+    case 2:
+      UMA_HISTOGRAM_MEMORY_GROWTH(".90min");

[Ilya Sherman, 2017/05/09 03:58:58]

Optional nit: I think it'd be a bit cleaner to have the dot included in the
macro.

[tasak, 2017/05/10 09:57:02]

Done.

+      ++memory_growth_report_count_;
+      break;
+  }
+  GetRendererScheduler()->DefaultTaskRunner()->PostDelayedTask(
+      FROM_HERE, record_purge_suspend_growth_metric_closure_.callback(),
+      base::TimeDelta::FromMinutes(30));
 }
 
 scoped_refptr<gpu::GpuChannelHost> RenderThreadImpl::EstablishGpuChannelSync() {