diff options
author | ssid <ssid@chromium.org> | 2016-02-25 10:58:30 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-02-25 18:59:51 +0000 |
commit | 7542f07f67d1e614024e6f4f7ef08fe7dfe70a18 (patch) | |
tree | 590c852995e0f343b60675018400efecbd59c041 | |
parent | a584caed25758379d066f5a6a513acbaed314ed0 (diff) | |
download | chromium_src-7542f07f67d1e614024e6f4f7ef08fe7dfe70a18.zip chromium_src-7542f07f67d1e614024e6f4f7ef08fe7dfe70a18.tar.gz chromium_src-7542f07f67d1e614024e6f4f7ef08fe7dfe70a18.tar.bz2 |
[MemoryInfra] Support dump providers running on SequencedTaskRunner
Some components in chrome require to be on a SequencedTaskRunner while
accessing memory information. This CL adds support for registration and
taking a memory dump on a specified SequencedTaskRunner.
- Support registration using SequencedTaskRunner. Keep the old
register methods for legacy reasons, and since we can't overload
RegisterDumpProvider with a base class parameter, introduce new api
for registration.
- Add a bool in Options to specify if the dump provider uses
SingleThreadtaskrunner, which is usually the case.
- Memory dumps happen on specified SequencedTaskRunner. Keep the old
optimization of grouping dump providers by task runner and do not
make thread hops if registered with SingleThreadTaskRunner and on
right thread.
- Ensure Unregistration should happen on the task runner as well. So,
it is still guaranteed that OnMemoryDump and UnregisterDumpProvider
calls happen on same sequenced task runner, which guarantees no
overlap of tasks.
BUG=555645
TBR=oysteine@chromium.org
Review URL: https://codereview.chromium.org/1618703004
Cr-Commit-Position: refs/heads/master@{#377632}
-rw-r--r-- | base/trace_event/memory_dump_manager.cc | 237 | ||||
-rw-r--r-- | base/trace_event/memory_dump_manager.h | 63 | ||||
-rw-r--r-- | base/trace_event/memory_dump_manager_unittest.cc | 117 | ||||
-rw-r--r-- | base/trace_event/memory_dump_provider.h | 12 | ||||
-rw-r--r-- | components/tracing/process_metrics_memory_dump_provider.cc | 3 |
5 files changed, 308 insertions, 124 deletions
diff --git a/base/trace_event/memory_dump_manager.cc b/base/trace_event/memory_dump_manager.cc index a549202..c24a659 100644 --- a/base/trace_event/memory_dump_manager.cc +++ b/base/trace_event/memory_dump_manager.cc @@ -185,6 +185,36 @@ void MemoryDumpManager::RegisterDumpProvider( MemoryDumpProvider* mdp, const char* name, const scoped_refptr<SingleThreadTaskRunner>& task_runner, + MemoryDumpProvider::Options options) { + options.dumps_on_single_thread_task_runner = true; + RegisterDumpProviderInternal(mdp, name, task_runner, options); +} + +void MemoryDumpManager::RegisterDumpProvider( + MemoryDumpProvider* mdp, + const char* name, + const scoped_refptr<SingleThreadTaskRunner>& task_runner) { + // Set |dumps_on_single_thread_task_runner| to true because all providers + // without task runner are run on dump thread. + MemoryDumpProvider::Options options; + options.dumps_on_single_thread_task_runner = true; + RegisterDumpProviderInternal(mdp, name, task_runner, options); +} + +void MemoryDumpManager::RegisterDumpProviderWithSequencedTaskRunner( + MemoryDumpProvider* mdp, + const char* name, + const scoped_refptr<SequencedTaskRunner>& task_runner, + MemoryDumpProvider::Options options) { + DCHECK(task_runner); + options.dumps_on_single_thread_task_runner = false; + RegisterDumpProviderInternal(mdp, name, task_runner, options); +} + +void MemoryDumpManager::RegisterDumpProviderInternal( + MemoryDumpProvider* mdp, + const char* name, + const scoped_refptr<SequencedTaskRunner>& task_runner, const MemoryDumpProvider::Options& options) { if (dumper_registrations_ignored_for_testing_) return; @@ -205,13 +235,6 @@ void MemoryDumpManager::RegisterDumpProvider( mdp->OnHeapProfilingEnabled(true); } -void MemoryDumpManager::RegisterDumpProvider( - MemoryDumpProvider* mdp, - const char* name, - const scoped_refptr<SingleThreadTaskRunner>& task_runner) { - RegisterDumpProvider(mdp, name, task_runner, MemoryDumpProvider::Options()); -} - void MemoryDumpManager::UnregisterDumpProvider(MemoryDumpProvider* mdp) { UnregisterDumpProviderInternal(mdp, false /* delete_async */); } @@ -242,28 +265,29 @@ void MemoryDumpManager::UnregisterDumpProviderInternal( if (take_mdp_ownership_and_delete_async) { // The MDP will be deleted whenever the MDPInfo struct will, that is either: // - At the end of this function, if no dump is in progress. - // - In the prologue of the ContinueAsyncProcessDump(). + // - Either in SetupNextMemoryDump() or InvokeOnMemoryDump() when MDPInfo is + // removed from |pending_dump_providers|. DCHECK(!(*mdp_iter)->owned_dump_provider); (*mdp_iter)->owned_dump_provider = std::move(owned_mdp); } else if (subtle::NoBarrier_Load(&memory_tracing_enabled_)) { // If you hit this DCHECK, your dump provider has a bug. // Unregistration of a MemoryDumpProvider is safe only if: - // - The MDP has specified a thread affinity (via task_runner()) AND - // the unregistration happens on the same thread (so the MDP cannot + // - The MDP has specified a sequenced task runner affinity AND the + // unregistration happens on the same task runner. So that the MDP cannot // unregister and be in the middle of a OnMemoryDump() at the same time. - // - The MDP has NOT specified a thread affinity and its ownership is + // - The MDP has NOT specified a task runner affinity and its ownership is // transferred via UnregisterAndDeleteDumpProviderSoon(). // In all the other cases, it is not possible to guarantee that the // unregistration will not race with OnMemoryDump() calls. DCHECK((*mdp_iter)->task_runner && - (*mdp_iter)->task_runner->BelongsToCurrentThread()) + (*mdp_iter)->task_runner->RunsTasksOnCurrentThread()) << "MemoryDumpProvider \"" << (*mdp_iter)->name << "\" attempted to " << "unregister itself in a racy way. Please file a crbug."; } // The MDPInfo instance can still be referenced by the // |ProcessMemoryDumpAsyncState.pending_dump_providers|. For this reason - // the MDPInfo is flagged as disabled. It will cause ContinueAsyncProcessDump + // the MDPInfo is flagged as disabled. It will cause InvokeOnMemoryDump() // 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; @@ -323,7 +347,7 @@ void MemoryDumpManager::CreateProcessDump(const MemoryDumpRequestArgs& args, { AutoLock lock(lock_); // |dump_thread_| can be nullptr is tracing was disabled before reaching - // here. ContinueAsyncProcessDump is robust enough to tolerate it and will + // 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, @@ -334,113 +358,134 @@ void MemoryDumpManager::CreateProcessDump(const MemoryDumpRequestArgs& args, 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.release()); + // Start the process dump. This involves task runner hops as specified by the + // MemoryDumpProvider(s) in RegisterDumpProvider()). + SetupNextMemoryDump(std::move(pmd_async_state)); } -// At most one ContinueAsyncProcessDump() can be active at any time for a given -// PMD, regardless of status of the |lock_|. |lock_| is used here purely to -// ensure consistency w.r.t. (un)registrations of |dump_providers_|. -// The linearization of dump providers' OnMemoryDump invocations is achieved by -// means of subsequent PostTask(s). -// -// 1) Prologue: -// - If this was the last hop, create a trace event, add it to the trace -// and finalize (invoke callback). -// - Check if we are on the right thread. If not hop and continue there. -// - Check if the dump provider is disabled, if so skip the dump. -// 2) Invoke the dump provider's OnMemoryDump() (unless skipped). -// 3) Epilogue: -// - Unregister the dump provider if it failed too many times consecutively. -// - Pop() the MDP from the |pending_dump_providers| list, eventually -// destroying the MDPInfo if that was unregistered in the meantime. -void MemoryDumpManager::ContinueAsyncProcessDump( - ProcessMemoryDumpAsyncState* owned_pmd_async_state) { +// PostTask InvokeOnMemoryDump() to the dump provider's sequenced task runner. A +// PostTask is always required for a generic SequencedTaskRunner to ensure that +// no other task is running on it concurrently. SetupNextMemoryDump() and +// InvokeOnMemoryDump() are called alternatively which linearizes the dump +// provider's OnMemoryDump invocations. +// At most one of either SetupNextMemoryDump() or InvokeOnMemoryDump() can be +// active at any time for a given PMD, regardless of status of the |lock_|. +// |lock_| is used in these functions purely to ensure consistency w.r.t. +// (un)registrations of |dump_providers_|. +void MemoryDumpManager::SetupNextMemoryDump( + scoped_ptr<ProcessMemoryDumpAsyncState> pmd_async_state) { // Initalizes the ThreadLocalEventBuffer to guarantee that the TRACE_EVENTs // in the PostTask below don't end up registering their own dump providers // (for discounting trace memory overhead) while holding the |lock_|. TraceLog::GetInstance()->InitializeThreadLocalEventBufferIfSupported(); + // If this was the last hop, create a trace event, add it to the trace and + // finalize process dump (invoke callback). + if (pmd_async_state->pending_dump_providers.empty()) + return FinalizeDumpAndAddToTrace(std::move(pmd_async_state)); + + // Read MemoryDumpProviderInfo thread safety considerations in + // memory_dump_manager.h when accessing |mdpinfo| fields. + MemoryDumpProviderInfo* mdpinfo = + pmd_async_state->pending_dump_providers.back().get(); + + // If the dump provider did not specify a task runner affinity, dump on + // |dump_thread_|. Note that |dump_thread_| might have been destroyed + // meanwhile. + SequencedTaskRunner* task_runner = mdpinfo->task_runner.get(); + if (!task_runner) { + DCHECK(mdpinfo->options.dumps_on_single_thread_task_runner); + task_runner = pmd_async_state->dump_thread_task_runner.get(); + if (!task_runner) { + // If tracing was disabled before reaching CreateProcessDump() the + // dump_thread_ would have been already torn down. Nack current dump and + // continue. + pmd_async_state->dump_successful = false; + pmd_async_state->pending_dump_providers.pop_back(); + return SetupNextMemoryDump(std::move(pmd_async_state)); + } + } + + if (mdpinfo->options.dumps_on_single_thread_task_runner && + task_runner->RunsTasksOnCurrentThread()) { + // If |dumps_on_single_thread_task_runner| is true then no PostTask is + // required if we are on the right thread. + return InvokeOnMemoryDump(pmd_async_state.release()); + } + + bool did_post_task = task_runner->PostTask( + FROM_HERE, Bind(&MemoryDumpManager::InvokeOnMemoryDump, Unretained(this), + Unretained(pmd_async_state.get()))); + + if (did_post_task) { + // Ownership is tranferred to InvokeOnMemoryDump(). + ignore_result(pmd_async_state.release()); + return; + } + + // PostTask usually fails only if the process or thread is shut down. So, the + // dump provider is disabled here. But, don't disable unbound dump providers. + // The utility thread is normally shutdown when disabling the trace and + // getting here in this case is expected. + if (mdpinfo->task_runner) { + LOG(ERROR) << "Disabling MemoryDumpProvider \"" << mdpinfo->name + << "\". Failed to post task on the task runner provided."; + + // A locked access is required to R/W |disabled| (for the + // UnregisterAndDeleteDumpProviderSoon() case). + AutoLock lock(lock_); + mdpinfo->disabled = true; + } + + // PostTask failed. Ignore the dump provider and continue. + pmd_async_state->pending_dump_providers.pop_back(); + SetupNextMemoryDump(std::move(pmd_async_state)); +} + +// This function is called on the right task runner for current MDP. It is +// either the task runner specified by MDP or |dump_thread_task_runner| if the +// MDP did not specify task runner. Invokes the dump provider's OnMemoryDump() +// (unless disabled). +void MemoryDumpManager::InvokeOnMemoryDump( + ProcessMemoryDumpAsyncState* owned_pmd_async_state) { // In theory |owned_pmd_async_state| should be a scoped_ptr. The only reason - // why it isn't is because of the corner case logic of |did_post_task| below, - // which needs to take back the ownership of the |pmd_async_state| when a - // thread goes away and consequently the PostTask() fails. + // why it isn't is because of the corner case logic of |did_post_task| + // above, which needs to take back the ownership of the |pmd_async_state| when + // the PostTask() fails. // Unfortunately, PostTask() destroys the scoped_ptr arguments upon failure // to prevent accidental leaks. Using a scoped_ptr would prevent us to to // skip the hop and move on. Hence the manual naked -> scoped ptr juggling. auto pmd_async_state = make_scoped_ptr(owned_pmd_async_state); owned_pmd_async_state = nullptr; - if (pmd_async_state->pending_dump_providers.empty()) - return FinalizeDumpAndAddToTrace(std::move(pmd_async_state)); - // Read MemoryDumpProviderInfo thread safety considerations in // memory_dump_manager.h when accessing |mdpinfo| fields. MemoryDumpProviderInfo* mdpinfo = pmd_async_state->pending_dump_providers.back().get(); - // If the dump provider did not specify a thread affinity, dump on - // |dump_thread_|. Note that |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, but |task_runner| should always be non-null. - SingleThreadTaskRunner* task_runner = mdpinfo->task_runner.get(); - if (!task_runner) - task_runner = pmd_async_state->dump_thread_task_runner.get(); - - bool post_task_failed = false; - if (!task_runner) { - // If tracing was disabled before reaching CreateProcessDump() |task_runner| - // will be null, as the dump_thread_ would have been already torn down. - post_task_failed = true; - pmd_async_state->dump_successful = false; - } else if (!task_runner->BelongsToCurrentThread()) { - // It's time to hop onto another thread. - post_task_failed = !task_runner->PostTask( - FROM_HERE, Bind(&MemoryDumpManager::ContinueAsyncProcessDump, - Unretained(this), Unretained(pmd_async_state.get()))); - if (!post_task_failed) { - // Ownership is tranferred to the next ContinueAsyncProcessDump(). - ignore_result(pmd_async_state.release()); - return; - } - } + DCHECK(!mdpinfo->task_runner || + mdpinfo->task_runner->RunsTasksOnCurrentThread()); - // At this point either: - // - The MDP has a task runner affinity and we are on the right thread. - // - The MDP has a task runner affinity but the underlying thread is gone, - // hence the above |post_task_failed| == true. - // - The MDP does NOT have a task runner affinity. A locked access is required - // to R/W |disabled| (for the UnregisterAndDeleteDumpProviderSoon() case). bool should_dump; - const char* disabled_reason = nullptr; { + // A locked access is required to R/W |disabled| (for the + // UnregisterAndDeleteDumpProviderSoon() case). AutoLock lock(lock_); - if (!mdpinfo->disabled) { - if (mdpinfo->consecutive_failures >= kMaxConsecutiveFailuresCount) { - mdpinfo->disabled = true; - disabled_reason = "Dump failed multiple times consecutively."; - } else if (post_task_failed && mdpinfo->task_runner) { - // Don't disable unbound dump providers. The utility thread is normally - // shutdown when disabling the trace and getting here in this case is - // expected. - mdpinfo->disabled = true; - disabled_reason = "The thread it was meant to dump onto is gone."; - } + + // Unregister the dump provider if it failed too many times consecutively. + if (!mdpinfo->disabled && + mdpinfo->consecutive_failures >= kMaxConsecutiveFailuresCount) { + mdpinfo->disabled = true; + LOG(ERROR) << "Disabling MemoryDumpProvider \"" << mdpinfo->name + << "\". Dump failed multiple times consecutively."; } - should_dump = !mdpinfo->disabled && !post_task_failed; + should_dump = !mdpinfo->disabled; } // AutoLock lock(lock_); - if (disabled_reason) { - LOG(ERROR) << "Disabling MemoryDumpProvider \"" << mdpinfo->name << "\". " - << disabled_reason; - } - if (should_dump) { // Invoke the dump provider. TRACE_EVENT_WITH_FLOW1(kTraceCategory, - "MemoryDumpManager::ContinueAsyncProcessDump", + "MemoryDumpManager::InvokeOnMemoryDump", TRACE_ID_MANGLE(pmd_async_state->req_args.dump_guid), TRACE_EVENT_FLAG_FLOW_IN | TRACE_EVENT_FLAG_FLOW_OUT, "dump_provider.name", mdpinfo->name); @@ -455,10 +500,10 @@ void MemoryDumpManager::ContinueAsyncProcessDump( bool dump_successful = mdpinfo->dump_provider->OnMemoryDump(args, pmd); mdpinfo->consecutive_failures = dump_successful ? 0 : mdpinfo->consecutive_failures + 1; - } // if (!mdpinfo->disabled) + } pmd_async_state->pending_dump_providers.pop_back(); - ContinueAsyncProcessDump(pmd_async_state.release()); + SetupNextMemoryDump(std::move(pmd_async_state)); } // static @@ -612,7 +657,7 @@ void MemoryDumpManager::OnTraceLogDisabled() { } // Thread stops are blocking and must be performed outside of the |lock_| - // or will deadlock (e.g., if ContinueAsyncProcessDump() tries to acquire it). + // or will deadlock (e.g., if SetupNextMemoryDump() tries to acquire it). periodic_dump_timer_.Stop(); if (dump_thread) dump_thread->Stop(); @@ -625,7 +670,7 @@ uint64_t MemoryDumpManager::GetTracingProcessId() const { MemoryDumpManager::MemoryDumpProviderInfo::MemoryDumpProviderInfo( MemoryDumpProvider* dump_provider, const char* name, - const scoped_refptr<SingleThreadTaskRunner>& task_runner, + const scoped_refptr<SequencedTaskRunner>& task_runner, const MemoryDumpProvider::Options& options) : dump_provider(dump_provider), name(name), diff --git a/base/trace_event/memory_dump_manager.h b/base/trace_event/memory_dump_manager.h index b713079..e9b09f8 100644 --- a/base/trace_event/memory_dump_manager.h +++ b/base/trace_event/memory_dump_manager.h @@ -69,9 +69,9 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver { // - 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|. If passed + // null |mdp| should be able to handle calls on arbitrary threads. // - options: extra optional arguments. See memory_dump_provider.h. void RegisterDumpProvider( MemoryDumpProvider* mdp, @@ -81,7 +81,12 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver { 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 @@ -153,14 +158,15 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver { // 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 - // the copy inside ProcessMemoryDumpAsyncState is erase()-d. + // MDPInfo is destroyed in SetupNextMemoryDump() or InvokeOnMemoryDump(), + // 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; @@ -171,7 +177,7 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver { 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; @@ -183,9 +189,9 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver { // 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; @@ -204,8 +210,9 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver { }; // 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, @@ -277,17 +284,30 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver { 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); + // Calls InvokeOnMemoryDump() for the next MDP on the task runner specified by + // the MDP while registration. On failure to do so, skips and continues to + // next MDP. + void SetupNextMemoryDump( + scoped_ptr<ProcessMemoryDumpAsyncState> pmd_async_state); + + // Invokes OnMemoryDump() of the next MDP and calls SetupNextMemoryDump() at + // the end to continue the ProcessMemoryDump. Should be called on the MDP task + // runner. + void InvokeOnMemoryDump(ProcessMemoryDumpAsyncState* owned_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. @@ -309,7 +329,8 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver { // 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 diff --git a/base/trace_event/memory_dump_manager_unittest.cc b/base/trace_event/memory_dump_manager_unittest.cc index 89a6e3d..138ba69 100644 --- a/base/trace_event/memory_dump_manager_unittest.cc +++ b/base/trace_event/memory_dump_manager_unittest.cc @@ -19,6 +19,7 @@ #include "base/test/trace_event_analyzer.h" #include "base/thread_task_runner_handle.h" #include "base/threading/platform_thread.h" +#include "base/threading/sequenced_worker_pool.h" #include "base/threading/thread.h" #include "base/trace_event/memory_dump_provider.h" #include "base/trace_event/process_memory_dump.h" @@ -50,18 +51,44 @@ namespace { void RegisterDumpProvider( MemoryDumpProvider* mdp, - const scoped_refptr<base::SingleThreadTaskRunner>& task_runner, - const MemoryDumpProvider::Options& options) { + const scoped_refptr<base::SequencedTaskRunner>& task_runner, + const MemoryDumpProvider::Options& options, + bool dumps_on_single_thread_task_runner) { MemoryDumpManager* mdm = MemoryDumpManager::GetInstance(); mdm->set_dumper_registrations_ignored_for_testing(false); - mdm->RegisterDumpProvider(mdp, "TestDumpProvider", task_runner, options); + const char* kMDPName = "TestDumpProvider"; + if (dumps_on_single_thread_task_runner) { + scoped_refptr<base::SingleThreadTaskRunner> single_thread_task_runner = + static_cast<base::SingleThreadTaskRunner*>(task_runner.get()); + mdm->RegisterDumpProvider(mdp, kMDPName, + std::move(single_thread_task_runner), options); + } else { + mdm->RegisterDumpProviderWithSequencedTaskRunner(mdp, kMDPName, task_runner, + options); + } mdm->set_dumper_registrations_ignored_for_testing(true); } +void RegisterDumpProvider( + MemoryDumpProvider* mdp, + const scoped_refptr<base::SingleThreadTaskRunner>& task_runner, + const MemoryDumpProvider::Options& options) { + RegisterDumpProvider(mdp, task_runner, options, + true /* dumps_on_single_thread_task_runner */); +} + void RegisterDumpProvider(MemoryDumpProvider* mdp) { RegisterDumpProvider(mdp, nullptr, MemoryDumpProvider::Options()); } +void RegisterDumpProviderWithSequencedTaskRunner( + MemoryDumpProvider* mdp, + const scoped_refptr<base::SequencedTaskRunner>& task_runner, + const MemoryDumpProvider::Options& options) { + RegisterDumpProvider(mdp, task_runner, options, + false /* dumps_on_single_thread_task_runner */); +} + void OnTraceDataCollected(Closure quit_closure, trace_event::TraceResultBuffer* buffer, const scoped_refptr<RefCountedString>& json, @@ -111,6 +138,46 @@ class MockMemoryDumpProvider : public MemoryDumpProvider { bool enable_mock_destructor; }; +class TestSequencedTaskRunner : public SequencedTaskRunner { + public: + TestSequencedTaskRunner() + : worker_pool_( + new SequencedWorkerPool(2 /* max_threads */, "Test Task Runner")), + enabled_(true), + num_of_post_tasks_(0) {} + + void set_enabled(bool value) { enabled_ = value; } + unsigned no_of_post_tasks() const { return num_of_post_tasks_; } + + bool PostNonNestableDelayedTask(const tracked_objects::Location& from_here, + const Closure& task, + TimeDelta delay) override { + NOTREACHED(); + return false; + } + + bool PostDelayedTask(const tracked_objects::Location& from_here, + const Closure& task, + TimeDelta delay) override { + num_of_post_tasks_++; + if (enabled_) + return worker_pool_->PostSequencedWorkerTask(token_, from_here, task); + return false; + } + + bool RunsTasksOnCurrentThread() const override { + return worker_pool_->IsRunningSequenceOnCurrentThread(token_); + } + + private: + ~TestSequencedTaskRunner() override {} + + scoped_refptr<SequencedWorkerPool> worker_pool_; + const SequencedWorkerPool::SequenceToken token_; + bool enabled_; + unsigned num_of_post_tasks_; +}; + class MemoryDumpManagerTest : public testing::Test { public: MemoryDumpManagerTest() : testing::Test(), kDefaultOptions() {} @@ -442,6 +509,50 @@ TEST_F(MemoryDumpManagerTest, RespectTaskRunnerAffinity) { DisableTracing(); } +// Check that the memory dump calls are always posted on task runner for +// SequencedTaskRunner case and that the dump provider gets disabled when +// PostTask fails, but the dump still succeeds. +TEST_F(MemoryDumpManagerTest, PostTaskForSequencedTaskRunner) { + InitializeMemoryDumpManager(false /* is_coordinator */); + std::vector<MockMemoryDumpProvider> mdps(3); + scoped_refptr<TestSequencedTaskRunner> task_runner1( + make_scoped_refptr(new TestSequencedTaskRunner())); + scoped_refptr<TestSequencedTaskRunner> task_runner2( + make_scoped_refptr(new TestSequencedTaskRunner())); + RegisterDumpProviderWithSequencedTaskRunner(&mdps[0], task_runner1, + kDefaultOptions); + RegisterDumpProviderWithSequencedTaskRunner(&mdps[1], task_runner2, + kDefaultOptions); + RegisterDumpProviderWithSequencedTaskRunner(&mdps[2], task_runner2, + kDefaultOptions); + // |mdps[0]| should be disabled permanently after first dump. + EXPECT_CALL(mdps[0], OnMemoryDump(_, _)).Times(0); + EXPECT_CALL(mdps[1], OnMemoryDump(_, _)).Times(2); + EXPECT_CALL(mdps[2], OnMemoryDump(_, _)).Times(2); + EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)).Times(2); + + EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory); + + task_runner1->set_enabled(false); + last_callback_success_ = false; + RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED, + MemoryDumpLevelOfDetail::DETAILED); + // Tasks should be individually posted even if |mdps[1]| and |mdps[2]| belong + // to same task runner. + EXPECT_EQ(1u, task_runner1->no_of_post_tasks()); + EXPECT_EQ(2u, task_runner2->no_of_post_tasks()); + EXPECT_TRUE(last_callback_success_); + + task_runner1->set_enabled(true); + last_callback_success_ = false; + RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED, + MemoryDumpLevelOfDetail::DETAILED); + EXPECT_EQ(2u, task_runner1->no_of_post_tasks()); + EXPECT_EQ(4u, task_runner2->no_of_post_tasks()); + EXPECT_TRUE(last_callback_success_); + DisableTracing(); +} + // Checks that providers get disabled after 3 consecutive failures, but not // otherwise (e.g., if interleaved). TEST_F(MemoryDumpManagerTest, DisableFailingDumpers) { diff --git a/base/trace_event/memory_dump_provider.h b/base/trace_event/memory_dump_provider.h index f03fcbd..79ab793 100644 --- a/base/trace_event/memory_dump_provider.h +++ b/base/trace_event/memory_dump_provider.h @@ -26,14 +26,20 @@ class BASE_EXPORT MemoryDumpProvider { public: // Optional arguments for MemoryDumpManager::RegisterDumpProvider(). struct Options { - Options() : target_pid(kNullProcessId) {} - explicit Options(ProcessId target_pid) : target_pid(target_pid) {} + Options() + : target_pid(kNullProcessId), + dumps_on_single_thread_task_runner(false) {} // If the dump provider generates dumps on behalf of another process, - // |target_process| contains the pid of that process. + // |target_pid| contains the pid of that process. // The default value is kNullProcessId, which means that the dump provider // generates dumps for the current process. ProcessId target_pid; + + // |dumps_on_single_thread_task_runner| is true if the dump provider runs on + // a SingleThreadTaskRunner, which is usually the case. It is faster to run + // all providers that run on the same thread together without thread hops. + bool dumps_on_single_thread_task_runner; }; virtual ~MemoryDumpProvider() {} diff --git a/components/tracing/process_metrics_memory_dump_provider.cc b/components/tracing/process_metrics_memory_dump_provider.cc index 141764b..e299a79 100644 --- a/components/tracing/process_metrics_memory_dump_provider.cc +++ b/components/tracing/process_metrics_memory_dump_provider.cc @@ -207,7 +207,8 @@ void ProcessMetricsMemoryDumpProvider::RegisterForProcess( base::ProcessId process) { scoped_ptr<ProcessMetricsMemoryDumpProvider> metrics_provider( new ProcessMetricsMemoryDumpProvider(process)); - base::trace_event::MemoryDumpProvider::Options options(process); + base::trace_event::MemoryDumpProvider::Options options; + options.target_pid = process; base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider( metrics_provider.get(), "ProcessMemoryMetrics", nullptr, options); bool did_insert = |