[tracing] Simplify and improve thread-hopping logic of memory-infra
commitca7a49016df9b30f326811246d70f7472dea4d66
authorprimiano <primiano@chromium.org>
Tue, 7 Jul 2015 10:28:41 +0000 (7 03:28 -0700)
committerCommit bot <commit-bot@chromium.org>
Tue, 7 Jul 2015 10:29:08 +0000 (7 10:29 +0000)
treef8c55ca487bf9d533e5edad322337192410b017d
parentbd11cd11925a1fd9da7a7d5d57b53ae546f278bf
[tracing] Simplify and improve thread-hopping logic of memory-infra

By contract MemoryDumpManager invokes the MemoryDumpProvider's
OnMemoryDump() method on the thread specified (by the MDP) in the
RegisterDumpProvider() call.
At the same time, it guarantees that at most one MDP is enabled at any
time. This is to avoid to have the MDPs dealing with thread-safety.

Assuming to have the following situation:
MDP_1 @ Thread_A, MDP_2 @ Thread_B, MDP_3 @ Thread_A, MDP_4 @ Thread_B

The way ProcessMemoryDump would have acted before this change is:
  CreateProcessDump():
    PostTask(Thread_A, ContinueAsyncProcessDump(MDP_1))
    PostTask(Thread_B, ContinueAsyncProcessDump(MDP_2))
    PostTask(Thread_A, ContinueAsyncProcessDump(MDP_3))
    PostTask(Thread_B, ContinueAsyncProcessDump(MDP_4))
  On each ContinueAsyncProcessDump() in each thread:
    AutoLock(lock_)
      MDP_X->OnMemoryDump()
      if (last_dump)
        FinalizeAndAddToTrace()

This strategy has two problems:
 1) performance-wise: it is suboptimal, as it performs more thread hops
    than necessary (ABAB instead of just AB).
 2) It is deadlock prone: essentially this spawns N tasks on various
    threads and then linearizes them on the |lock_|. This can cause
    deadlocks if other tasks on the message loops have dependencies.
    This is the case of crbug.com/505725, which is fixed by this CL.

The new strategy is conceptually simpler, smarter and avoids
inter-locking the message loops, as follows:
- The MDPs are kept in order according to their thread affinity.
- When CreateProcessDump() is invoked, only one task (the first in the
  set) is posted.
- Each task, which corresponds to an invocation of
  ContinueAsyncProcessDump() invokes the dump provider without taking
  the lock_. When done posts the next task (or finalizes the dump).
By posting one task at a time we still guarantee that the OnMemoryDump
calls are linearized (per PMD) even if the lock is not taken.
Furthermore, given the order of the MDP set, the thread hopping is
reduced to the minimum.

The only tricky part of this CL is dealing with changes of the dump
providers (i.e. calls to (Un)RegisterDumpProvider) while an
asynchronous process dump is in progress. In this (very unlikely)
case the choice is to abort the current dump.

BUG=497144,505725

Review URL: https://codereview.chromium.org/1222153004

Cr-Commit-Position: refs/heads/master@{#337580}
base/trace_event/memory_dump_manager.cc
base/trace_event/memory_dump_manager.h
base/trace_event/memory_dump_manager_unittest.cc
content/browser/tracing/tracing_controller_impl.cc