[tracing] MemoryDumpManager uses TraceConfig to set the periodic dump timers

base/trace_event/memory_dump_manager.cc

Index: base/trace_event/memory_dump_manager.cc
diff --git a/base/trace_event/memory_dump_manager.cc b/base/trace_event/memory_dump_manager.cc
index cd1503b51952634ed8b19475c8176da4c151a3fc..a35da8a3fa3e4927a3ead97eb37f891b0c85b187 100644
--- a/base/trace_event/memory_dump_manager.cc
+++ b/base/trace_event/memory_dump_manager.cc
@@ -38,22 +38,21 @@ namespace trace_event {
 
 namespace {
 
-// Throttle mmaps at a rate of once every kHeavyMmapsDumpsRate standard dumps.
-const int kHeavyDumpsRate = 8;  // 250 ms * 8 = 2000 ms.
-const int kDumpIntervalMs = 250;
 const int kTraceEventNumArgs = 1;
 const char* kTraceEventArgNames[] = {"dumps"};
 const unsigned char kTraceEventArgTypes[] = {TRACE_VALUE_TYPE_CONVERTABLE};
 
 StaticAtomicSequenceNumber g_next_guid;
 uint32 g_periodic_dumps_count = 0;
+uint32 g_heavy_dumps_rate = 0;
 MemoryDumpManager* g_instance_for_testing = nullptr;
 
 void RequestPeriodicGlobalDump() {
   MemoryDumpArgs::LevelOfDetail dump_level_of_detail =
       g_periodic_dumps_count == 0 ? MemoryDumpArgs::LevelOfDetail::HIGH
                                   : MemoryDumpArgs::LevelOfDetail::LOW;
-  if (++g_periodic_dumps_count == kHeavyDumpsRate)
+
+  if (++g_periodic_dumps_count == g_heavy_dumps_rate)

[Primiano Tucci (use gerrit), 2015/09/03 08:39:51]

since g_heavy_dumps_rate can be 0 this should become

if (g_heavy_dumps_rate > 0 && ++g_periodic_dumps_count == g_heavy_dumps_rate),
otherwise you will end up doing one heavy dump every 4BLN (when the uint32 wraps
over) if heavy dumps are disabled. Unlikely but also undesiderable :)

[ssid, 2015/09/03 16:30:18]

Done.

     g_periodic_dumps_count = 0;
 
   MemoryDumpArgs dump_args = {dump_level_of_detail};
@@ -419,18 +418,37 @@ void MemoryDumpManager::OnTraceLogEnabled() {
   subtle::NoBarrier_Store(&memory_tracing_enabled_, 1);
 
   // TODO(primiano): This is a temporary hack to disable periodic memory dumps
-  // when running memory benchmarks until they can be enabled/disabled in
-  // base::trace_event::TraceConfig. See https://goo.gl/5Hj3o0.
+  // when running memory benchmarks until telemetry uses TraceConfig to
+  // enable/disable periodic dumps.
   // The same mechanism should be used to disable periodic dumps in tests.
-  if (delegate_->IsCoordinatorProcess() &&
-      !CommandLine::ForCurrentProcess()->HasSwitch(
-          "enable-memory-benchmarking") &&
-      !disable_periodic_dumps_for_testing_) {
-    g_periodic_dumps_count = 0;
-    periodic_dump_timer_.Start(FROM_HERE,
-                               TimeDelta::FromMilliseconds(kDumpIntervalMs),
-                               base::Bind(&RequestPeriodicGlobalDump));
+  if (!delegate_->IsCoordinatorProcess() ||
+      CommandLine::ForCurrentProcess()->HasSwitch(
+          "enable-memory-benchmarking") ||
+      disable_periodic_dumps_for_testing_) {
+    return;
+  }
+
+  g_periodic_dumps_count = 0;

[Primiano Tucci (use gerrit), 2015/09/03 08:39:50]

Please add a comment here saying:

// Enable periodic dumps. At the moment the periodic support is limited to at
most one low-detail periodic dump and at most one high-detail periodic dump. If
both are specified the high-detail period must be an integer multiple of the
low-level one.

[ssid, 2015/09/03 16:30:18]

Done.

+  const TraceConfig trace_config =
+      TraceLog::GetInstance()->GetCurrentTraceConfig();
+  const TraceConfig::MemoryDumpConfig& config_list =
+      trace_config.memory_dump_config();
+  if (config_list.empty())
+    return;
+
+  uint32 timer_period_ms = std::numeric_limits<uint32>::max();

[Primiano Tucci (use gerrit), 2015/09/03 08:39:50]

nit: rename to min_timer_period_ms;

[ssid, 2015/09/03 16:30:18]

Done.

+  uint32 heavy_dump_period_ms = 0;

[Primiano Tucci (use gerrit), 2015/09/03 08:39:50]

Also add a DCHECK_LE(config_list.size(), 2)

[ssid, 2015/09/03 16:30:18]

Done.

+  for (const TraceConfig::MemoryDumpTriggerConfig& config : config_list) {
+    if (config.level_of_detail == MemoryDumpArgs::LevelOfDetail::HIGH)

[Primiano Tucci (use gerrit), 2015/09/03 08:39:50]

Also add a:

if (config.periodic_interval_ms == 0) {
  DCHECK(false);
  continue;
}

[ssid, 2015/09/03 16:30:18]

Done.

+      heavy_dump_period_ms = config.periodic_interval_ms;
+    timer_period_ms = std::min(timer_period_ms, config.periodic_interval_ms);
   }
+  DCHECK_EQ(0u, heavy_dump_period_ms % timer_period_ms);
+  g_heavy_dumps_rate = heavy_dump_period_ms / timer_period_ms;
+
+  periodic_dump_timer_.Start(FROM_HERE,
+                             TimeDelta::FromMilliseconds(timer_period_ms),
+                             base::Bind(&RequestPeriodicGlobalDump));
 }
 
 void MemoryDumpManager::OnTraceLogDisabled() {