[tracing] Move memory-infra dumps to dedicated thread

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/base_switches.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/thread_task_runner_handle.h"
+#include "base/threading/thread.h"
 #include "base/trace_event/memory_dump_provider.h"
 #include "base/trace_event/memory_dump_session_state.h"
 #include "base/trace_event/memory_profiler_allocation_context.h"
 #include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_event_argument.h"
 #include "build/build_config.h"
 
 #if !defined(OS_NACL)
 #include "base/trace_event/process_memory_totals_dump_provider.h"
 #endif
@@ skipping 251 matching lines @@
 
 void MemoryDumpManager::CreateProcessDump(const MemoryDumpRequestArgs& args,
                                           const MemoryDumpCallback& callback) {
   TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(kTraceCategory, "ProcessMemoryDump",
                                     TRACE_ID_MANGLE(args.dump_guid));
 
   scoped_ptr<ProcessMemoryDumpAsyncState> pmd_async_state;
   {
     AutoLock lock(lock_);
     pmd_async_state.reset(new ProcessMemoryDumpAsyncState(
-        args, dump_providers_.begin(), session_state_, callback));
+        args, dump_providers_.begin(), session_state_, callback,
+        dump_thread_->task_runner()));
   }
 
   TRACE_EVENT_WITH_FLOW0(kTraceCategory, "MemoryDumpManager::CreateProcessDump",
                          TRACE_ID_MANGLE(args.dump_guid),
                          TRACE_EVENT_FLAG_FLOW_OUT);
 
   // Start the thread hop. |dump_providers_| are kept sorted by thread, so
   // ContinueAsyncProcessDump will hop at most once per thread (w.r.t. thread
   // affinity specified by the MemoryDumpProvider(s) in RegisterDumpProvider()).
   ContinueAsyncProcessDump(pmd_async_state.Pass());
@@ skipping 28 matching lines @@
   // DO NOT put any LOG() statement in the locked sections, as in some contexts
   // (GPU process) LOG() ends up performing PostTask/IPCs.
   MemoryDumpProvider* mdp;
   bool skip_dump = false;
   {
     AutoLock lock(lock_);
 
     auto mdp_info = pmd_async_state->next_dump_provider;
     mdp = mdp_info->dump_provider;
     dump_provider_name = mdp_info->name;
+
+    // If the dump provider did not specify a thread affinity, dump on
+    // |dump_thread_|.
+    SingleThreadTaskRunner* task_runner = mdp_info->task_runner.get();
+    if (!task_runner)
+      task_runner = pmd_async_state->dump_thread_task_runner.get();
+
+    // |dump_thread_| might have been Stop()-ed at this point (if tracing was
+    // disabled in the meanwhile). In such case the PostTask() below will fail.
+    // The task_runner handle, however, should always be non-null.

[petrcermak, 2015/11/02 10:35:20]

nit: s/The task_runner handle/|task_runner|/

[Primiano Tucci (use gerrit), 2015/11/02 15:07:07]

Done.

+    DCHECK(task_runner);
+
     if (mdp_info->disabled || mdp_info->unregistered) {
       skip_dump = true;
-    } else if (mdp_info->task_runner &&
-               !mdp_info->task_runner->BelongsToCurrentThread()) {
+    } else if (!task_runner->BelongsToCurrentThread()) {
       // It's time to hop onto another thread.
 
       // Copy the callback + arguments just for the unlikley case in which
       // PostTask fails. In such case the Bind helper will destroy the
       // pmd_async_state and we must keep a copy of the fields to notify the
       // abort.
       MemoryDumpCallback callback = pmd_async_state->callback;
       scoped_refptr<SingleThreadTaskRunner> callback_task_runner =
-          pmd_async_state->task_runner;
+          pmd_async_state->callback_task_runner;
 
-      const bool did_post_task = mdp_info->task_runner->PostTask(
+      const bool did_post_task = task_runner->PostTask(
           FROM_HERE, Bind(&MemoryDumpManager::ContinueAsyncProcessDump,
                           Unretained(this), Passed(pmd_async_state.Pass())));
       if (did_post_task)
         return;
 
       // The thread is gone. At this point the best thing we can do is to
       // disable the dump provider and abort this dump.
       mdp_info->disabled = true;
       return AbortDumpLocked(callback, callback_task_runner, dump_guid);
     }
@@ skipping 41 matching lines @@
   if (finalize)
     return FinalizeDumpAndAddToTrace(pmd_async_state.Pass());
 
   ContinueAsyncProcessDump(pmd_async_state.Pass());
 }
 
 // static
 void MemoryDumpManager::FinalizeDumpAndAddToTrace(
     scoped_ptr<ProcessMemoryDumpAsyncState> pmd_async_state) {
   const uint64_t dump_guid = pmd_async_state->req_args.dump_guid;
-  if (!pmd_async_state->task_runner->BelongsToCurrentThread()) {
-    scoped_refptr<SingleThreadTaskRunner> task_runner =
-        pmd_async_state->task_runner;
-    task_runner->PostTask(FROM_HERE,
-                          Bind(&MemoryDumpManager::FinalizeDumpAndAddToTrace,
-                               Passed(pmd_async_state.Pass())));
+  if (!pmd_async_state->callback_task_runner->BelongsToCurrentThread()) {
+    scoped_refptr<SingleThreadTaskRunner> callback_task_runner =
+        pmd_async_state->callback_task_runner;
+    callback_task_runner->PostTask(
+        FROM_HERE, Bind(&MemoryDumpManager::FinalizeDumpAndAddToTrace,
+                        Passed(pmd_async_state.Pass())));
     return;
   }
 
   TRACE_EVENT_WITH_FLOW0(kTraceCategory,
                          "MemoryDumpManager::FinalizeDumpAndAddToTrace",
                          TRACE_ID_MANGLE(dump_guid), TRACE_EVENT_FLAG_FLOW_IN);
 
   TracedValue* traced_value = new TracedValue();
   scoped_refptr<ConvertableToTraceFormat> event_value(traced_value);
   pmd_async_state->process_memory_dump.AsValueInto(traced_value);
@@ skipping 36 matching lines @@
   bool enabled;
   TRACE_EVENT_CATEGORY_GROUP_ENABLED(kTraceCategory, &enabled);
   if (!enabled)
     return;
 
   // Initialize the TraceLog for the current thread. This is to avoid that the
   // TraceLog memory dump provider is registered lazily in the PostTask() below
   // while the |lock_| is taken;
   TraceLog::GetInstance()->InitializeThreadLocalEventBufferIfSupported();
 
+  // Spin-up the thread used to invoke unbound dump providers.
+  scoped_ptr<Thread> dump_thread(new Thread("MemoryInfra"));
+  if (!dump_thread->Start()) {
+    LOG(ERROR) << "Failed to start the memory-infra thread for tracing";
+    return;
+  }
+
   AutoLock lock(lock_);
 
   DCHECK(delegate_);  // At this point we must have a delegate.
 
   scoped_refptr<StackFrameDeduplicator> stack_frame_deduplicator = nullptr;
 
   if (heap_profiling_enabled_) {
     // If heap profiling is enabled, the stack frame deduplicator will be in
     // use. Add a metadata event to write its frames.
     stack_frame_deduplicator = new StackFrameDeduplicator;
     TRACE_EVENT_API_ADD_METADATA_EVENT("stackFrames", "stackFrames",
                                        stack_frame_deduplicator);
   }
 
+  dump_thread_ = dump_thread.Pass();

[petrcermak, 2015/11/02 10:35:20]

Shouldn't you DCHECK that dump_thread_ was null before this?

[Primiano Tucci (use gerrit), 2015/11/02 15:07:07]

Done.

   session_state_ = new MemoryDumpSessionState(stack_frame_deduplicator);
 
   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
@@ skipping 29 matching lines @@
   }
   DCHECK_EQ(0u, heavy_dump_period_ms % min_timer_period_ms);
   g_heavy_dumps_rate = heavy_dump_period_ms / min_timer_period_ms;
 
   periodic_dump_timer_.Start(FROM_HERE,
                              TimeDelta::FromMilliseconds(min_timer_period_ms),
                              base::Bind(&RequestPeriodicGlobalDump));
 }
 
 void MemoryDumpManager::OnTraceLogDisabled() {
-  AutoLock lock(lock_);
+  subtle::NoBarrier_Store(&memory_tracing_enabled_, 0);
+  scoped_ptr<Thread> dump_thread;
+  {
+    AutoLock lock(lock_);
+    dump_thread = dump_thread_.Pass();
+    session_state_ = nullptr;
+  }
+
+  // Thread stops are blocking and must be performed outside of the |lock_|

[petrcermak, 2015/11/02 10:35:20]

nit: Does the comment apply to line 566 (periodic_dump_timer_.Stop();) as well?
If not, then the line should come before the comment.

[Primiano Tucci (use gerrit), 2015/11/02 15:07:07]

yes, it does.

+  // or will deadlock (e.g., if ContinueAsyncProcessDump() tries to acquire it).
   periodic_dump_timer_.Stop();
-  subtle::NoBarrier_Store(&memory_tracing_enabled_, 0);
-  session_state_ = nullptr;
+  if (dump_thread)
+    dump_thread->Stop();
 }
 
 uint64_t MemoryDumpManager::GetTracingProcessId() const {
   return delegate_->GetTracingProcessId();
 }
 
 MemoryDumpManager::MemoryDumpProviderInfo::MemoryDumpProviderInfo(
     MemoryDumpProvider* dump_provider,
     const char* name,
     const scoped_refptr<SingleThreadTaskRunner>& task_runner)
     : dump_provider(dump_provider),
       name(name),
       task_runner(task_runner),
       consecutive_failures(0),
       disabled(false),
       unregistered(false) {}
 
 MemoryDumpManager::MemoryDumpProviderInfo::~MemoryDumpProviderInfo() {}
 
 bool MemoryDumpManager::MemoryDumpProviderInfo::operator<(
     const MemoryDumpProviderInfo& other) const {
   if (task_runner == other.task_runner)
     return dump_provider < other.dump_provider;
-  return task_runner < other.task_runner;
+  // Ensure that unbound providers (task_runner == nullptr) always run last.
+  return !(task_runner < other.task_runner);
 }
 
 MemoryDumpManager::ProcessMemoryDumpAsyncState::ProcessMemoryDumpAsyncState(
     MemoryDumpRequestArgs req_args,
     MemoryDumpProviderInfoSet::iterator next_dump_provider,
     const scoped_refptr<MemoryDumpSessionState>& session_state,
-    MemoryDumpCallback callback)
+    MemoryDumpCallback callback,
+    const scoped_refptr<SingleThreadTaskRunner>& dump_thread_task_runner)
     : process_memory_dump(session_state),
       req_args(req_args),
       next_dump_provider(next_dump_provider),
       callback(callback),
-      task_runner(MessageLoop::current()->task_runner()) {}
+      callback_task_runner(MessageLoop::current()->task_runner()),
+      dump_thread_task_runner(dump_thread_task_runner) {}
 
 MemoryDumpManager::ProcessMemoryDumpAsyncState::~ProcessMemoryDumpAsyncState() {
 }
 
 }  // namespace trace_event
 }  // namespace base