[MemoryInfra] Support dump providers running on SequencedTaskRunner

base/trace_event/memory_dump_manager.h

 // 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.
 
 #ifndef BASE_TRACE_EVENT_MEMORY_DUMP_MANAGER_H_
 #define BASE_TRACE_EVENT_MEMORY_DUMP_MANAGER_H_
 
 #include <stdint.h>
 
 #include <map>
@@ skipping 51 matching lines @@
   void Initialize(MemoryDumpManagerDelegate* delegate, bool is_coordinator);
 
   // (Un)Registers a MemoryDumpProvider instance.
   // Args:
   //  - mdp: the MemoryDumpProvider instance to be registered. MemoryDumpManager
   //      does NOT take memory ownership of |mdp|, which is expected to either
   //      be a singleton or unregister itself.
   //  - name: a friendly name (duplicates allowed). Used for debugging and
   //      run-time profiling of memory-infra internals. Must be a long-lived
   //      C string.
-  //  - task_runner: if non-null, all the calls to |mdp| will be
-  //      issued on the given thread. Otherwise, |mdp| should be able to
-  //      handle calls on arbitrary threads.
+  //  - task_runner: either a SingleThreadTaskRunner or SequencedTaskRunner. All
+  //      the calls to |mdp| will be run on the given |task_runner|.
+  //      RegisterDumpProviderWithSequencedTaskRunner does not accept null
+  //      |task_runner|, otherwise if passed null |mdp| should be able to handle

[Primiano Tucci (use gerrit), 2016/02/08 12:03:15]

I'd make this comment a bit simpler and skip the part 
"RegisterDumpProviderWithSequencedTaskRunner does not accept null"
should be kind of obvious and if not, they will hit a dcheck.

[ssid, 2016/02/11 01:45:03]

Done.

+  //      calls on arbitrary threads.
   //  - options: extra optional arguments. See memory_dump_provider.h.
   void RegisterDumpProvider(
       MemoryDumpProvider* mdp,
       const char* name,
       const scoped_refptr<SingleThreadTaskRunner>& task_runner);
   void RegisterDumpProvider(
       MemoryDumpProvider* mdp,
       const char* name,
       const scoped_refptr<SingleThreadTaskRunner>& task_runner,
-      const MemoryDumpProvider::Options& options);
+      MemoryDumpProvider::Options options);
+  void RegisterDumpProviderWithSequencedTaskRunner(
+      MemoryDumpProvider* mdp,
+      const char* name,
+      const scoped_refptr<SequencedTaskRunner>& task_runner,
+      MemoryDumpProvider::Options options);
   void UnregisterDumpProvider(MemoryDumpProvider* mdp);
 
   // Unregisters an unbound dump provider and takes care about its deletion
   // asynchronously. Can be used only for for dump providers with no
   // task-runner affinity.
   // This method takes ownership of the dump provider and guarantees that:
   //  - The |mdp| will be deleted at some point in the near future.
   //  - Its deletion will not happen concurrently with the OnMemoryDump() call.
   // Note that OnMemoryDump() calls can still happen after this method returns.
   void UnregisterAndDeleteDumpProviderSoon(scoped_ptr<MemoryDumpProvider> mdp);
@@ skipping 51 matching lines @@
   // Descriptor used to hold information about registered MDPs.
   // Some important considerations about lifetime of this object:
   // - In nominal conditions, all the MemoryDumpProviderInfo instances live in
   //   the |dump_providers_| collection (% unregistration while dumping).
   // - Upon each dump they (actually their scoped_refptr-s) are copied into
   //   the ProcessMemoryDumpAsyncState. This is to allow removal (see below).
   // - When the MDP.OnMemoryDump() is invoked, the corresponding MDPInfo copy
   //   inside ProcessMemoryDumpAsyncState is removed.
   // - In most cases, the MDPInfo is destroyed within UnregisterDumpProvider().
   // - If UnregisterDumpProvider() is called while a dump is in progress, the
-  //   MDPInfo is destroyed in the epilogue of ContinueAsyncProcessDump(), when
+  //   MDPInfo is destroyed in the epilogue of SetupNextMemoryDump(), when
   //   the copy inside ProcessMemoryDumpAsyncState is erase()-d.
   // - The non-const fields of MemoryDumpProviderInfo are safe to access only
-  //   in the |task_runner| thread, unless the thread has been destroyed.
+  //   on tasks running in the |task_runner|, unless the thread has been
+  //   destroyed.
   struct MemoryDumpProviderInfo
       : public RefCountedThreadSafe<MemoryDumpProviderInfo> {
-    // Define a total order based on the thread (i.e. |task_runner|) affinity,
-    // so that all MDP belonging to the same thread are adjacent in the set.
+    // Define a total order based on the |task_runner| affinity, so that MDPs
+    // belonging to the same SequencedTaskRunner are adjacent in the set.
     struct Comparator {
       bool operator()(const scoped_refptr<MemoryDumpProviderInfo>& a,
                       const scoped_refptr<MemoryDumpProviderInfo>& b) const;
     };
     using OrderedSet =
         std::set<scoped_refptr<MemoryDumpProviderInfo>, Comparator>;
 
     MemoryDumpProviderInfo(
         MemoryDumpProvider* dump_provider,
         const char* name,
-        const scoped_refptr<SingleThreadTaskRunner>& task_runner,
+        const scoped_refptr<SequencedTaskRunner>& task_runner,
         const MemoryDumpProvider::Options& options);
 
     MemoryDumpProvider* const dump_provider;
 
     // Used to transfer ownership for UnregisterAndDeleteDumpProviderSoon().
     // nullptr in all other cases.
     scoped_ptr<MemoryDumpProvider> owned_dump_provider;
 
     // Human readable name, for debugging and testing. Not necessarily unique.
     const char* const name;
 
-    // The task_runner affinity. Can be nullptr, in which case the dump provider
+    // The task runner affinity. Can be nullptr, in which case the dump provider
     // will be invoked on |dump_thread_|.
-    const scoped_refptr<SingleThreadTaskRunner> task_runner;
+    const scoped_refptr<SequencedTaskRunner> task_runner;
 
     // The |options| arg passed to RegisterDumpProvider().
     const MemoryDumpProvider::Options options;
 
     // For fail-safe logic (auto-disable failing MDPs).
     int consecutive_failures;
 
     // Flagged either by the auto-disable logic or during unregistration.
     bool disabled;
 
    private:
     friend class base::RefCountedThreadSafe<MemoryDumpProviderInfo>;
     ~MemoryDumpProviderInfo();
 
     DISALLOW_COPY_AND_ASSIGN(MemoryDumpProviderInfo);
   };
 
   // Holds the state of a process memory dump that needs to be carried over
-  // across threads in order to fulfil an asynchronous CreateProcessDump()
-  // request. At any time exactly one thread owns a ProcessMemoryDumpAsyncState.
+  // across task runners in order to fulfil an asynchronous CreateProcessDump()
+  // request. At any time exactly one task runner owns a
+  // ProcessMemoryDumpAsyncState.
   struct ProcessMemoryDumpAsyncState {
     ProcessMemoryDumpAsyncState(
         MemoryDumpRequestArgs req_args,
         const MemoryDumpProviderInfo::OrderedSet& dump_providers,
         const scoped_refptr<MemoryDumpSessionState>& session_state,
         MemoryDumpCallback callback,
         const scoped_refptr<SingleThreadTaskRunner>& dump_thread_task_runner);
     ~ProcessMemoryDumpAsyncState();
 
     // Gets or creates the memory dump container for the given target process.
@@ skipping 23 matching lines @@
     bool dump_successful;
 
     // The thread on which FinalizeDumpAndAddToTrace() (and hence |callback|)
     // should be invoked. This is the thread on which the initial
     // CreateProcessDump() request was called.
     const scoped_refptr<SingleThreadTaskRunner> callback_task_runner;
 
     // The thread on which unbound dump providers should be invoked.
     // This is essentially |dump_thread_|.task_runner() but needs to be kept
     // as a separate variable as it needs to be accessed by arbitrary dumpers'
-    // threads outside of the lock_ to avoid races when disabling tracing.
+    // task runners outside of the lock_ to avoid races when disabling tracing.

[Primiano Tucci (use gerrit), 2016/02/08 12:03:15]

keep this as thread, task_runner doesn't add a lot and makes the sentence less
readable

[ssid, 2016/02/11 01:45:03]

Done.

     // It is immutable for all the duration of a tracing session.
     const scoped_refptr<SingleThreadTaskRunner> dump_thread_task_runner;
 
    private:
     DISALLOW_COPY_AND_ASSIGN(ProcessMemoryDumpAsyncState);
   };
 
   static const int kMaxConsecutiveFailuresCount;
   static const char* const kSystemAllocatorPoolName;
 
   MemoryDumpManager();
   ~MemoryDumpManager() override;
 
   static void SetInstanceForTesting(MemoryDumpManager* instance);
   static void FinalizeDumpAndAddToTrace(
       scoped_ptr<ProcessMemoryDumpAsyncState> pmd_async_state);
 
   // Enable heap profiling if kEnableHeapProfiling is specified.
   void EnableHeapProfilingIfNeeded();
 
   // Internal, used only by MemoryDumpManagerDelegate.
   // Creates a memory dump for the current process and appends it to the trace.
   // |callback| will be invoked asynchronously upon completion on the same
   // thread on which CreateProcessDump() was called.
   void CreateProcessDump(const MemoryDumpRequestArgs& args,
                          const MemoryDumpCallback& callback);
 
-  // Continues the ProcessMemoryDump started by CreateProcessDump(), hopping
-  // across threads as needed as specified by MDPs in RegisterDumpProvider().
-  void ContinueAsyncProcessDump(
-      ProcessMemoryDumpAsyncState* owned_pmd_async_state);
+  // SetupNextMemoryDump, InvokeOnMemoryDump and FinalizeCurrentDump are called
+  // in order for each dump provider to obtain a memory dump:
+
+  // Calls InvokeOnMemoryDump for the next MDP on the task runner specified by
+  // the MDP while registration. On failure to do so, calls FinalizeCurrentDump
+  // directly.
+  void SetupNextMemoryDump(
+      scoped_ptr<ProcessMemoryDumpAsyncState> pmd_async_state);
+
+  // Invokes OnMemoryDump of the last MDP and calls FinalizeCurrentDump. Should

[Primiano Tucci (use gerrit), 2016/02/08 12:03:15]

remove "of the last MDP" it's really misleading.
I know what you mean here, but without context it feels like you use this
function only once at the end, which is not true.

Also "and calls FinalizeCurrentDump" add "at the end"

[ssid, 2016/02/11 01:45:03]

Done.

+  // be called on the MDP task runner.
+  void InvokeOnMemoryDump(ProcessMemoryDumpAsyncState* owned_pmd_async_state);
+
+  // Finishes current dump and calls either SetupNextMemoryDump for next dump
+  // provider or FinalizeDumpAndAddToTrace if we covered all providers.
+  void FinalizeCurrentDump(
+      scoped_ptr<ProcessMemoryDumpAsyncState> pmd_async_state);
+
+  // Helper for RegierDumpProvider* functions.
+  void RegisterDumpProviderInternal(
+      MemoryDumpProvider* mdp,
+      const char* name,
+      const scoped_refptr<SequencedTaskRunner>& task_runner,
+      const MemoryDumpProvider::Options& options);
 
   // Helper for the public UnregisterDumpProvider* functions.
   void UnregisterDumpProviderInternal(MemoryDumpProvider* mdp,
                                       bool take_mdp_ownership_and_delete_async);
 
-  // An ordererd set of registered MemoryDumpProviderInfo(s), sorted by thread
-  // affinity (MDPs belonging to the same thread are adjacent).
+  // An ordererd set of registered MemoryDumpProviderInfo(s), sorted by task
+  // runner affinity (MDPs belonging to the same task runners are adjacent).
   MemoryDumpProviderInfo::OrderedSet dump_providers_;
 
   // Shared among all the PMDs to keep state scoped to the tracing session.
   scoped_refptr<MemoryDumpSessionState> session_state_;
 
   MemoryDumpManagerDelegate* delegate_;  // Not owned.
 
   // When true, this instance is in charge of coordinating periodic dumps.
   bool is_coordinator_;
 
   // Protects from concurrent accesses to the |dump_providers_*| and |delegate_|
   // to guard against disabling logging while dumping on another thread.
   Lock lock_;
 
   // Optimization to avoid attempting any memory dump (i.e. to not walk an empty
   // dump_providers_enabled_ list) when tracing is not enabled.
   subtle::AtomicWord memory_tracing_enabled_;
 
   // For time-triggered periodic dumps.
   RepeatingTimer periodic_dump_timer_;
 
-  // Thread used for MemoryDumpProviders which don't specify a thread affinity.
+  // Thread used for MemoryDumpProviders which don't specify a task runner
+  // affinity.
   scoped_ptr<Thread> dump_thread_;
 
   // The unique id of the child process. This is created only for tracing and is
   // expected to be valid only when tracing is enabled.
   uint64_t tracing_process_id_;
 
   // When true, calling |RegisterMemoryDumpProvider| is a no-op.
   bool dumper_registrations_ignored_for_testing_;
 
   // Whether new memory dump providers should be told to enable heap profiling.
@@ skipping 23 matching lines @@
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MemoryDumpManagerDelegate);
 };
 
 }  // namespace trace_event
 }  // namespace base
 
 #endif  // BASE_TRACE_EVENT_MEMORY_DUMP_MANAGER_H_