[tracing] Implement polling in MemoryDumpManager

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 <utility>
 
 #include "base/allocator/features.h"
 #include "base/atomic_sequence_num.h"
 #include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/debug/debugging_flags.h"
 #include "base/debug/stack_trace.h"
 #include "base/memory/ptr_util.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/heap_profiler.h"
 #include "base/trace_event/heap_profiler_allocation_context_tracker.h"
 #include "base/trace_event/heap_profiler_event_filter.h"
 #include "base/trace_event/heap_profiler_stack_frame_deduplicator.h"
 #include "base/trace_event/heap_profiler_type_name_deduplicator.h"
 #include "base/trace_event/malloc_dump_provider.h"
 #include "base/trace_event/memory_dump_provider.h"
+#include "base/trace_event/memory_dump_scheduler.h"
 #include "base/trace_event/memory_dump_session_state.h"
 #include "base/trace_event/memory_infra_background_whitelist.h"
 #include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/trace_event_argument.h"
 #include "build/build_config.h"
 
 #if defined(OS_ANDROID)
 #include "base/trace_event/java_heap_dump_provider_android.h"
 #endif
@@ skipping 116 matching lines @@
                    LeakySingletonTraits<MemoryDumpManager>>::get();
 }
 
 // static
 void MemoryDumpManager::SetInstanceForTesting(MemoryDumpManager* instance) {
   g_instance_for_testing = instance;
 }
 
 MemoryDumpManager::MemoryDumpManager()
     : delegate_(nullptr),
-      is_coordinator_(false),
       memory_tracing_enabled_(0),
       tracing_process_id_(kInvalidTracingProcessId),
       dumper_registrations_ignored_for_testing_(false),
       heap_profiling_enabled_(false) {
   g_next_guid.GetNext();  // Make sure that first guid is not zero.
 
   // At this point the command line may not be initialized but we try to
   // enable the heap profiler to capture allocations as soon as possible.
   EnableHeapProfilingIfNeeded();
 
@@ skipping 237 matching lines @@
   // to just skip it, without actually invoking the |mdp|, which might be
   // destroyed by the caller soon after this method returns.
   (*mdp_iter)->disabled = true;
   dump_providers_.erase(mdp_iter);
 }
 
 void MemoryDumpManager::RegisterPollingMDPOnDumpThread(
     scoped_refptr<MemoryDumpManager::MemoryDumpProviderInfo> mdpinfo) {
   AutoLock lock(lock_);
   dump_providers_for_polling_.insert(mdpinfo);
+
+  // Notify ready for polling when first polling supported provider is
+  // registered. This handles the case where OnTraceLogEnabled() did not notify
+  // ready since no polling supported mdp has yet been registered.
+  if (dump_providers_for_polling_.size() == 1)
+    dump_scheduler_->NotifyPollingSupported();
 }
 
 void MemoryDumpManager::UnregisterPollingMDPOnDumpThread(
     scoped_refptr<MemoryDumpManager::MemoryDumpProviderInfo> mdpinfo) {
   mdpinfo->dump_provider->SuspendFastMemoryPolling();
 
   AutoLock lock(lock_);
   dump_providers_for_polling_.erase(mdpinfo);
+  DCHECK(!dump_providers_for_polling_.empty())
+      << "All polling MDPs cannot be unregistered, since it will cause polling "

[Primiano Tucci (use gerrit), 2017/01/26 02:52:22]

i'd remove the  "since... " part here.

[ssid, 2017/01/26 21:48:57]

Done.

+         "without without any MDPs";
 }
 
 void MemoryDumpManager::RequestGlobalDump(
     MemoryDumpType dump_type,
     MemoryDumpLevelOfDetail level_of_detail,
     const MemoryDumpCallback& callback) {
   // Bail out immediately if tracing is not enabled at all or if the dump mode
   // is not allowed.
   if (!UNLIKELY(subtle::NoBarrier_Load(&memory_tracing_enabled_)) ||
       !IsDumpModeAllowed(level_of_detail)) {
@@ skipping 61 matching lines @@
     // here. SetupNextMemoryDump() is robust enough to tolerate it and will
     // NACK the dump.
     pmd_async_state.reset(new ProcessMemoryDumpAsyncState(
         args, dump_providers_, session_state_, callback,
         dump_thread_ ? dump_thread_->task_runner() : nullptr));
 
     // Safety check to prevent reaching here without calling RequestGlobalDump,
     // with disallowed modes. If |session_state_| is null then tracing is
     // disabled.
     CHECK(!session_state_ ||
-          session_state_->memory_dump_config().allowed_dump_modes.count(
-              args.level_of_detail));
+          session_state_->IsDumpModeAllowed(args.level_of_detail));
   }
 
   TRACE_EVENT_WITH_FLOW0(kTraceCategory, "MemoryDumpManager::CreateProcessDump",
                          TRACE_ID_MANGLE(args.dump_guid),
                          TRACE_EVENT_FLAG_FLOW_OUT);
 
   // Start the process dump. This involves task runner hops as specified by the
   // MemoryDumpProvider(s) in RegisterDumpProvider()).
   SetupNextMemoryDump(std::move(pmd_async_state));
 }
@@ skipping 152 matching lines @@
                                                                   args);
     bool dump_successful = mdpinfo->dump_provider->OnMemoryDump(args, pmd);
     mdpinfo->consecutive_failures =
         dump_successful ? 0 : mdpinfo->consecutive_failures + 1;
   }
 
   pmd_async_state->pending_dump_providers.pop_back();
   SetupNextMemoryDump(std::move(pmd_async_state));
 }
 
-void MemoryDumpManager::PollFastMemoryTotal(uint64_t* memory_total) {
+bool MemoryDumpManager::PollFastMemoryTotal(uint64_t* memory_total) {
+#if DCHECK_IS_ON()
+  {
+    AutoLock lock(lock_);

[Primiano Tucci (use gerrit), 2017/01/26 02:52:22]

I think I wrote this in another cl but forgot the discussion.
I think there is a tiny window where you can hit this DCHECK even if everything
is still legit.
the window would be:
- OnTraceLogDisabled
- the dump_thread_ is std::move'd in the local dump_thread variable.
- dump thread is being stopped (but still active) but dump_thread_ is nullptr
now.
- this function gets called on the dump thread -> crash

[ssid, 2017/01/26 21:48:57]

Yes that is true. That is why I am holding a lock here to access dump_thread_
and we have a condition that says if(dump_thread_)
If you feel that this check is not necessary I can remove it. I felt that we
need to have this check since MDM no longer calls this method and a different
class calls it.

+    if (dump_thread_)
+      DCHECK(dump_thread_->task_runner()->BelongsToCurrentThread());
+  }
+#endif
+  if (dump_providers_for_polling_.empty())
+    return false;
+
   *memory_total = 0;
   // Note that we call PollFastMemoryTotal() even if the dump provider is
   // disabled (unregistered). This is to avoid taking lock while polling.
   for (const auto& mdpinfo : dump_providers_for_polling_) {
     uint64_t value = 0;
     mdpinfo->dump_provider->PollFastMemoryTotal(&value);
     *memory_total += value;
   }
-  return;
+  return true;
 }
 
 // static
 void MemoryDumpManager::FinalizeDumpAndAddToTrace(
     std::unique_ptr<ProcessMemoryDumpAsyncState> pmd_async_state) {
   HEAP_PROFILER_SCOPED_IGNORE;
   DCHECK(pmd_async_state->pending_dump_providers.empty());
   const uint64_t dump_guid = pmd_async_state->req_args.dump_guid;
   if (!pmd_async_state->callback_task_runner->BelongsToCurrentThread()) {
     scoped_refptr<SingleThreadTaskRunner> callback_task_runner =
@@ skipping 58 matching lines @@
   // while the |lock_| is taken;
   TraceLog::GetInstance()->InitializeThreadLocalEventBufferIfSupported();
 
   // Spin-up the thread used to invoke unbound dump providers.
   std::unique_ptr<Thread> dump_thread(new Thread("MemoryInfra"));
   if (!dump_thread->Start()) {
     LOG(ERROR) << "Failed to start the memory-infra thread for tracing";
     return;
   }
 
-  const TraceConfig trace_config =
-      TraceLog::GetInstance()->GetCurrentTraceConfig();
+  const TraceConfig::MemoryDumpConfig memory_dump_config =

[Primiano Tucci (use gerrit), 2017/01/26 02:52:22]

since you don't need to copy this anymore, you can jsut make this a const
MDConfig& and keep just a cheap reference

[ssid, 2017/01/26 21:48:57]

Done.

[ssid, 2017/01/27 00:43:24]

Actually this would need keep alive a copy of trace config here.

+      TraceLog::GetInstance()->GetCurrentTraceConfig().memory_dump_config();
   scoped_refptr<MemoryDumpSessionState> session_state =
       new MemoryDumpSessionState;
-  session_state->SetMemoryDumpConfig(trace_config.memory_dump_config());
+  session_state->SetAllowedDumpModes(memory_dump_config.allowed_dump_modes);
+  session_state->set_heap_profiler_breakdown_threshold_bytes(
+      memory_dump_config.heap_profiler_options.breakdown_threshold_bytes);
   if (heap_profiling_enabled_) {
     // If heap profiling is enabled, the stack frame deduplicator and type name
     // deduplicator will be in use. Add a metadata events to write the frames
     // and type IDs.
     session_state->SetStackFrameDeduplicator(
         WrapUnique(new StackFrameDeduplicator));
 
     session_state->SetTypeNameDeduplicator(
         WrapUnique(new TypeNameDeduplicator));
 
     TRACE_EVENT_API_ADD_METADATA_EVENT(
         TraceLog::GetCategoryGroupEnabled("__metadata"), "stackFrames",
         "stackFrames",
         MakeUnique<SessionStateConvertableProxy<StackFrameDeduplicator>>(
             session_state, &MemoryDumpSessionState::stack_frame_deduplicator));
 
     TRACE_EVENT_API_ADD_METADATA_EVENT(
         TraceLog::GetCategoryGroupEnabled("__metadata"), "typeNames",
         "typeNames",
         MakeUnique<SessionStateConvertableProxy<TypeNameDeduplicator>>(
             session_state, &MemoryDumpSessionState::type_name_deduplicator));
   }
 
+  std::unique_ptr<MemoryDumpScheduler> dump_scheduler(
+      WrapUnique(new MemoryDumpScheduler(this, dump_thread->task_runner())));

[Primiano Tucci (use gerrit), 2017/01/26 02:52:23]

no need to WrapUnique if you do this explicitly.
Essentially either you do:
unique_ptr<X> var(new X(...));
or
auto var = MakeUnique(new X(...))

so you can safely remove the WrapUnique here

[ssid, 2017/01/26 21:48:57]

Done.

+  DCHECK_LE(memory_dump_config.triggers.size(), 3u);

[Primiano Tucci (use gerrit), 2017/01/26 02:52:23]

shouldn't this dcheck be in the scheduler?

[ssid, 2017/01/26 21:48:56]

We cannot easily have this in scheduler since we need to keep track of how many
triggers are being added. That would add an extra state in scheduler. Felt it
was useless.
It also makes sense to be here because the AddTrigger() api has a restriction of
at max 3 triggers.

+  for (const auto& trigger : memory_dump_config.triggers) {
+    DCHECK(session_state->IsDumpModeAllowed(trigger.level_of_detail));

[Primiano Tucci (use gerrit), 2017/01/26 02:52:23]

shoudln't this be:
if (!allowed) {
  NOTREACHED();
  continue;
} 

to avoid accidentally doing non-allowed dumps in release builds, where dcheck
has no effect?

[ssid, 2017/01/26 21:48:56]

Done.

+    dump_scheduler->AddTrigger(trigger.trigger_type, trigger.level_of_detail,
+                               trigger.min_time_between_dumps_ms);
+  }
+
   {
     AutoLock lock(lock_);
 
     DCHECK(delegate_);  // At this point we must have a delegate.
     session_state_ = session_state;
 
     DCHECK(!dump_thread_);
     dump_thread_ = std::move(dump_thread);
+    dump_scheduler_ = std::move(dump_scheduler);
+
+    subtle::NoBarrier_Store(&memory_tracing_enabled_, 1);
 
     dump_providers_for_polling_.clear();
     for (const auto& mdpinfo : dump_providers_) {
       if (mdpinfo->options.is_fast_polling_supported)
         dump_providers_for_polling_.insert(mdpinfo);
     }
+    // Notify polling supported only if some polling supported provider was
+    // registered, else RegisterPollingMDPOnDumpThread() will notify when first
+    // polling MDP registers.
+    if (!dump_providers_for_polling_.empty())
+      dump_scheduler_->NotifyPollingSupported();
 
-    subtle::NoBarrier_Store(&memory_tracing_enabled_, 1);
-
-    if (!is_coordinator_)
-      return;
+    // Only coordinator process triggers periodic global memory dumps.
+    if (is_coordinator_)
+      dump_scheduler_->NotifyPeriodicTriggerSupported();
   }
 
-  // Enable periodic dumps if necessary.
-  periodic_dump_timer_.Start(trace_config.memory_dump_config().triggers);
 }
 
 void MemoryDumpManager::OnTraceLogDisabled() {
   // There might be a memory dump in progress while this happens. Therefore,
   // ensure that the MDM state which depends on the tracing enabled / disabled
   // state is always accessed by the dumping methods holding the |lock_|.
   if (!subtle::NoBarrier_Load(&memory_tracing_enabled_))
     return;
   subtle::NoBarrier_Store(&memory_tracing_enabled_, 0);
   std::unique_ptr<Thread> dump_thread;
+  std::unique_ptr<MemoryDumpScheduler> scheduler = nullptr;

[Primiano Tucci (use gerrit), 2017/01/26 02:52:23]

no need for the = nullptr part, it should be implicit.

[ssid, 2017/01/26 21:48:57]

Done.

   {
     AutoLock lock(lock_);
     dump_thread = std::move(dump_thread_);
     session_state_ = nullptr;
+    scheduler = std::move(dump_scheduler_);
   }
+  scheduler->DisableAllTriggers();
 
   // Thread stops are blocking and must be performed outside of the |lock_|
   // or will deadlock (e.g., if SetupNextMemoryDump() tries to acquire it).
-  periodic_dump_timer_.Stop();
   if (dump_thread)
     dump_thread->Stop();
 
   // |dump_providers_for_polling_| must be cleared only after the dump thread is
   // stopped (polling tasks are done).
   {
     AutoLock lock(lock_);
     for (const auto& mdpinfo : dump_providers_for_polling_)
       mdpinfo->dump_provider->SuspendFastMemoryPolling();
     dump_providers_for_polling_.clear();
   }
 }
 
 bool MemoryDumpManager::IsDumpModeAllowed(MemoryDumpLevelOfDetail dump_mode) {
   AutoLock lock(lock_);
   if (!session_state_)
     return false;
-  return session_state_->memory_dump_config().allowed_dump_modes.count(
-             dump_mode) != 0;
+  return session_state_->IsDumpModeAllowed(dump_mode);
 }
 
 uint64_t MemoryDumpManager::GetTracingProcessId() const {
   return delegate_->GetTracingProcessId();
 }
 
 MemoryDumpManager::MemoryDumpProviderInfo::MemoryDumpProviderInfo(
     MemoryDumpProvider* dump_provider,
     const char* name,
     scoped_refptr<SequencedTaskRunner> task_runner,
@@ skipping 45 matching lines @@
                                              const MemoryDumpArgs& dump_args) {
   auto iter = process_dumps.find(pid);
   if (iter == process_dumps.end()) {
     std::unique_ptr<ProcessMemoryDump> new_pmd(
         new ProcessMemoryDump(session_state, dump_args));
     iter = process_dumps.insert(std::make_pair(pid, std::move(new_pmd))).first;
   }
   return iter->second.get();
 }
 
-MemoryDumpManager::PeriodicGlobalDumpTimer::PeriodicGlobalDumpTimer() {}
-
-MemoryDumpManager::PeriodicGlobalDumpTimer::~PeriodicGlobalDumpTimer() {
-  Stop();
-}
-
-void MemoryDumpManager::PeriodicGlobalDumpTimer::Start(
-    const std::vector<TraceConfig::MemoryDumpConfig::Trigger>& triggers_list) {
-  if (triggers_list.empty())
-    return;
-
-  // At the moment the periodic support is limited to at most one periodic
-  // trigger per dump mode. All intervals should be an integer multiple of the
-  // smallest interval specified.
-  periodic_dumps_count_ = 0;
-  uint32_t min_timer_period_ms = std::numeric_limits<uint32_t>::max();
-  uint32_t light_dump_period_ms = 0;
-  uint32_t heavy_dump_period_ms = 0;
-  DCHECK_LE(triggers_list.size(), 3u);
-  auto* mdm = MemoryDumpManager::GetInstance();
-  for (const TraceConfig::MemoryDumpConfig::Trigger& config : triggers_list) {
-    DCHECK_NE(0u, config.min_time_between_dumps_ms);
-    DCHECK_EQ(MemoryDumpType::PERIODIC_INTERVAL, config.trigger_type)
-        << "Only periodic_interval triggers are suppported";
-    switch (config.level_of_detail) {
-      case MemoryDumpLevelOfDetail::BACKGROUND:
-        DCHECK(mdm->IsDumpModeAllowed(MemoryDumpLevelOfDetail::BACKGROUND));
-        break;
-      case MemoryDumpLevelOfDetail::LIGHT:
-        DCHECK_EQ(0u, light_dump_period_ms);
-        DCHECK(mdm->IsDumpModeAllowed(MemoryDumpLevelOfDetail::LIGHT));
-        light_dump_period_ms = config.min_time_between_dumps_ms;
-        break;
-      case MemoryDumpLevelOfDetail::DETAILED:
-        DCHECK_EQ(0u, heavy_dump_period_ms);
-        DCHECK(mdm->IsDumpModeAllowed(MemoryDumpLevelOfDetail::DETAILED));
-        heavy_dump_period_ms = config.min_time_between_dumps_ms;
-        break;
-    }
-    min_timer_period_ms =
-        std::min(min_timer_period_ms, config.min_time_between_dumps_ms);
-  }
-
-  DCHECK_EQ(0u, light_dump_period_ms % min_timer_period_ms);
-  light_dump_rate_ = light_dump_period_ms / min_timer_period_ms;
-  DCHECK_EQ(0u, heavy_dump_period_ms % min_timer_period_ms);
-  heavy_dump_rate_ = heavy_dump_period_ms / min_timer_period_ms;
-
-  timer_.Start(FROM_HERE, TimeDelta::FromMilliseconds(min_timer_period_ms),
-               base::Bind(&PeriodicGlobalDumpTimer::RequestPeriodicGlobalDump,
-                          base::Unretained(this)));
-}
-
-void MemoryDumpManager::PeriodicGlobalDumpTimer::Stop() {
-  if (IsRunning()) {
-    timer_.Stop();
-  }
-}
-
-bool MemoryDumpManager::PeriodicGlobalDumpTimer::IsRunning() {
-  return timer_.IsRunning();
-}
-
-void MemoryDumpManager::PeriodicGlobalDumpTimer::RequestPeriodicGlobalDump() {
-  MemoryDumpLevelOfDetail level_of_detail = MemoryDumpLevelOfDetail::BACKGROUND;
-  if (light_dump_rate_ > 0 && periodic_dumps_count_ % light_dump_rate_ == 0)
-    level_of_detail = MemoryDumpLevelOfDetail::LIGHT;
-  if (heavy_dump_rate_ > 0 && periodic_dumps_count_ % heavy_dump_rate_ == 0)
-    level_of_detail = MemoryDumpLevelOfDetail::DETAILED;
-  ++periodic_dumps_count_;
-
-  MemoryDumpManager::GetInstance()->RequestGlobalDump(
-      MemoryDumpType::PERIODIC_INTERVAL, level_of_detail);
-}
-
 }  // namespace trace_event
 }  // namespace base