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

base/trace_event/memory_dump_manager.cc

 // 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 "base/trace_event/memory_dump_manager.h"
 
 #include <algorithm>
 
 #include "base/atomic_sequence_num.h"
 #include "base/command_line.h"
@@ skipping 20 matching lines @@
 
 #if defined(OS_WIN)
 #include "base/trace_event/winheap_dump_provider_win.h"
 #endif
 
 namespace base {
 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};
   MemoryDumpManager::GetInstance()->RequestGlobalDump(
       MemoryDumpType::PERIODIC_INTERVAL, dump_args);
 }
 
 }  // namespace
 
 // static
@@ skipping 345 matching lines @@
 
   session_state_ = new MemoryDumpSessionState();
   for (auto it = dump_providers_.begin(); it != dump_providers_.end(); ++it) {
     it->disabled = false;
     it->consecutive_failures = 0;
   }
 
   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() {
   AutoLock lock(lock_);
   periodic_dump_timer_.Stop();
   subtle::NoBarrier_Store(&memory_tracing_enabled_, 0);
   session_state_ = nullptr;
 }
 
 uint64 MemoryDumpManager::GetTracingProcessId() const {
@@ skipping 29 matching lines @@
       next_dump_provider(next_dump_provider),
       callback(callback),
       task_runner(MessageLoop::current()->task_runner()) {
 }
 
 MemoryDumpManager::ProcessMemoryDumpAsyncState::~ProcessMemoryDumpAsyncState() {
 }
 
 }  // namespace trace_event
 }  // namespace base