[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 a813d85a11a21d70c61b3c720d6a16642bcc3743..6e87f2d128bcfbc93ca8ab2d7819a0144c0199c5 100644
--- a/base/trace_event/memory_dump_manager.cc
+++ b/base/trace_event/memory_dump_manager.cc
@@ -38,32 +38,38 @@ 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_shortest_dump_interval_ms;
 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)
-    g_periodic_dumps_count = 0;
+  MemoryDumpArgs dump_args = {MemoryDumpArgs::LevelOfDetail::LOW};
+  TraceConfig trace_config = TraceLog::GetInstance()->GetCurrentTraceConfig();
+
+  // Assign the highest dump args among the periodic dump triggers whose
+  // interval end at current time.
+  for (const TraceConfig::MemoryDumpTriggerConfig& config :

[Primiano Tucci (use gerrit), 2015/08/26 10:25:06]

All this seems way too complex. I don't like the idea of re-reading and copying
the TraceConfig at each dump. I think you here want to do something way simpler,
which is:

Just add a g_heavy_dumps_rate and set it below when you parse the traceconfig.
In this method just check 
if (g_heavy_dumps_rate > 0 && )g_periodic_dumps_count % g_heavy_dumps_rate) ==
0)
  do heavy dump
else 
  do light dump

+       trace_config.memory_dump_config()) {
+    if (g_periodic_dumps_count %
+            (config.periodic_interval_ms / g_shortest_dump_interval_ms) ==
+        0) {
+      if (dump_args.level_of_detail < config.level_of_detail)
+        dump_args.level_of_detail = config.level_of_detail;
+    }
+  }
+  g_periodic_dumps_count++;
 
-  MemoryDumpArgs dump_args = {dump_level_of_detail};
   MemoryDumpManager::GetInstance()->RequestGlobalDump(
       MemoryDumpType::PERIODIC_INTERVAL, dump_args);
 }
 
 }  // namespace
 
-// TODO(ssid): Start using TraceConfig and remove this.
 // static
 const char* const MemoryDumpManager::kTraceCategory =
     TRACE_DISABLED_BY_DEFAULT("memory-infra");
@@ -423,15 +429,35 @@ void MemoryDumpManager::OnTraceLogEnabled() {
   // when running memory benchmarks until they can be enabled/disabled in
   // base::trace_event::TraceConfig. See https://goo.gl/5Hj3o0.
   // 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;
+  const TraceConfig trace_config =
+      TraceLog::GetInstance()->GetCurrentTraceConfig();
+  const TraceConfig::MemoryDumpConfig& config_list =
+      trace_config.memory_dump_config();
+  if (!config_list.size())

[Primiano Tucci (use gerrit), 2015/08/26 10:25:06]

if (config_list.empty()) ?

+    return;
+
+  g_shortest_dump_interval_ms = std::numeric_limits<uint32>::max();
+  for (const TraceConfig::MemoryDumpTriggerConfig& config : config_list) {
+    if (g_shortest_dump_interval_ms > config.periodic_interval_ms)
+      g_shortest_dump_interval_ms = config.periodic_interval_ms;
+  }
+  DCHECK_NE(g_shortest_dump_interval_ms, std::numeric_limits<uint32>::max());
+
+  // It does not make sense to have dumps at varying intervals.

[Primiano Tucci (use gerrit), 2015/08/26 10:25:06]

Is not that it doesnt' make sense.
The deal is that we don't want to support it, which is different.
Please adjust the comment... and the code according to the comment above.

+  for (const TraceConfig::MemoryDumpTriggerConfig& config : config_list)
+    DCHECK_EQ(0u, config.periodic_interval_ms % g_shortest_dump_interval_ms);
+
+  periodic_dump_timer_.Start(
+      FROM_HERE, TimeDelta::FromMilliseconds(g_shortest_dump_interval_ms),
+      base::Bind(&RequestPeriodicGlobalDump));
 }
 
 void MemoryDumpManager::OnTraceLogDisabled() {