memory-infra: Switch to MemoryPeakDetector and simplify MemoryDumpScheduler

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 <inttypes.h>
 #include <stdio.h>
 
 #include <algorithm>
@@ skipping 16 matching lines @@
 #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/memory_peak_detector.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
 
 namespace base {
@@ skipping 71 matching lines @@
   void EstimateTraceMemoryOverhead(
       TraceEventMemoryOverhead* overhead) override {
     return (session_state.get()->*getter_function)()
         ->EstimateTraceMemoryOverhead(overhead);
   }
 
   scoped_refptr<MemoryDumpSessionState> session_state;
   GetterFunctPtr const getter_function;
 };
 
+void OnPeakDetected(MemoryDumpLevelOfDetail level_of_detail) {
+  MemoryDumpManager::GetInstance()->RequestGlobalDump(
+      MemoryDumpType::PEAK_MEMORY_USAGE, level_of_detail);
+}
+
 }  // namespace
 
 // static
 const char* const MemoryDumpManager::kTraceCategory =
     TRACE_DISABLED_BY_DEFAULT("memory-infra");
 
 // static
 const char* const MemoryDumpManager::kLogPrefix = "Memory-infra dump";
 
 // static
@@ skipping 191 matching lines @@
   }
 
   {
     AutoLock lock(lock_);
     bool already_registered = !dump_providers_.insert(mdpinfo).second;
     // This actually happens in some tests which don't have a clean tear-down
     // path for RenderThreadImpl::Init().
     if (already_registered)
       return;
 
-    // The list of polling MDPs is populated OnTraceLogEnabled(). This code
-    // deals with the case of a MDP capable of fast polling that is registered
-    // after the OnTraceLogEnabled()
-    if (options.is_fast_polling_supported && dump_thread_) {
-      dump_thread_->task_runner()->PostTask(
-          FROM_HERE, Bind(&MemoryDumpManager::RegisterPollingMDPOnDumpThread,
-                          Unretained(this), mdpinfo));
-    }
+    if (options.is_fast_polling_supported)
+      MemoryPeakDetector::GetInstance()->NotifyMemoryDumpProvidersChanged();
   }
 
   if (heap_profiling_enabled_)
     mdp->OnHeapProfilingEnabled(true);
 }
 
 void MemoryDumpManager::UnregisterDumpProvider(MemoryDumpProvider* mdp) {
   UnregisterDumpProviderInternal(mdp, false /* delete_async */);
 }
 
@@ skipping 18 matching lines @@
   }
 
   if (mdp_iter == dump_providers_.end())
     return;  // Not registered / already unregistered.
 
   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.
     // - Either in SetupNextMemoryDump() or InvokeOnMemoryDump() when MDPInfo is
     //   removed from |pending_dump_providers|.
-    // - When the provider is removed from |dump_providers_for_polling_|.
+    // - When the provider is removed from other clients (MemoryPeakDetector).
     DCHECK(!(*mdp_iter)->owned_dump_provider);
     (*mdp_iter)->owned_dump_provider = std::move(owned_mdp);
   } else if (strict_thread_check_blacklist_.count((*mdp_iter)->name) == 0 ||
              subtle::NoBarrier_Load(&memory_tracing_enabled_)) {
     // If dump provider's name is on |strict_thread_check_blacklist_|, then the
     // DCHECK is fired only when tracing is enabled. Otherwise the DCHECK is
     // fired even when tracing is not enabled (stricter).
     // TODO(ssid): Remove this condition after removing all the dump providers
     // in the blacklist and the buildbots are no longer flakily hitting the
     // DCHECK, crbug.com/643438.
 
     // If you hit this DCHECK, your dump provider has a bug.
     // Unregistration of a MemoryDumpProvider is safe only if:
     // - 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 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->RunsTasksOnCurrentThread())
         << "MemoryDumpProvider \"" << (*mdp_iter)->name << "\" attempted to "
         << "unregister itself in a racy way. Please file a crbug.";
   }
 
-  if ((*mdp_iter)->options.is_fast_polling_supported && dump_thread_) {
+  if ((*mdp_iter)->options.is_fast_polling_supported) {
     DCHECK(take_mdp_ownership_and_delete_async);
-    dump_thread_->task_runner()->PostTask(
-        FROM_HERE, Bind(&MemoryDumpManager::UnregisterPollingMDPOnDumpThread,
-                        Unretained(this), *mdp_iter));
+    MemoryPeakDetector::GetInstance()->NotifyMemoryDumpProvidersChanged();
   }
 
   // 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 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;
   dump_providers_.erase(mdp_iter);
 }
 
-void MemoryDumpManager::RegisterPollingMDPOnDumpThread(
-    scoped_refptr<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)
-    MemoryDumpScheduler::GetInstance()->EnablePollingIfNeeded();
-}
-
-void MemoryDumpManager::UnregisterPollingMDPOnDumpThread(
-    scoped_refptr<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.";
-}
-
 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)) {
     VLOG(1) << kLogPrefix << " failed because " << kTraceCategory
             << " tracing category is not enabled or the requested dump mode is "
@@ skipping 14 matching lines @@
       MemoryDumpTypeToString(dump_type), "level_of_detail",
       MemoryDumpLevelOfDetailToString(level_of_detail));
   MemoryDumpCallback wrapped_callback = Bind(&OnGlobalDumpDone, callback);
 
   // The delegate will coordinate the IPC broadcast and at some point invoke
   // CreateProcessDump() to get a dump for the current process.
   MemoryDumpRequestArgs args = {guid, dump_type, level_of_detail};
   delegate_->RequestGlobalMemoryDump(args, wrapped_callback);
 }
 
+void MemoryDumpManager::GetDumpProvidersForPolling(
+    std::vector<scoped_refptr<MemoryDumpProviderInfo>>* providers) {
+  AutoLock lock(lock_);
+  for (const scoped_refptr<MemoryDumpProviderInfo>& mdp : dump_providers_) {

[hjd, 2017/04/06 12:14:17]

We could DCHECK providers is empty or use a set so we can't end up appending the
same mdps to the list over and over.

[Primiano Tucci (use gerrit), 2017/04/10 19:16:41]

Good point. done.

+    if (mdp->options.is_fast_polling_supported)
+      providers->push_back(mdp);
+  }
+}
+
 void MemoryDumpManager::RequestGlobalDump(
     MemoryDumpType dump_type,
     MemoryDumpLevelOfDetail level_of_detail) {
   RequestGlobalDump(dump_type, level_of_detail, MemoryDumpCallback());
 }
 
 bool MemoryDumpManager::IsDumpProviderRegisteredForTesting(
     MemoryDumpProvider* provider) {
   AutoLock lock(lock_);
 
@@ skipping 32 matching lines @@
     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_->IsDumpModeAllowed(args.level_of_detail));
 
-    MemoryDumpScheduler::GetInstance()->NotifyDumpTriggered();
+    // If enabled, holds back the peak detector resetting its estimation window.
+    MemoryPeakDetector::GetInstance()->Clear();
   }
 
   // Start the process dump. This involves task runner hops as specified by the
   // MemoryDumpProvider(s) in RegisterDumpProvider()).
   SetupNextMemoryDump(std::move(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
@@ skipping 155 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));
 }
 
-bool MemoryDumpManager::PollFastMemoryTotal(uint64_t* memory_total) {
-#if DCHECK_IS_ON()
-  {
-    AutoLock lock(lock_);
-    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 true;
-}
-
 // static
 uint32_t MemoryDumpManager::GetDumpsSumKb(const std::string& pattern,
                                           const ProcessMemoryDump* pmd) {
   uint64_t sum = 0;
   for (const auto& kv : pmd->allocator_dumps()) {
     auto name = StringPiece(kv.first);
     if (MatchPattern(name, pattern))
       sum += kv.second->GetSize();
   }
   return sum / 1024;
@@ skipping 126 matching lines @@
         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));
   }
 
-  {
-    AutoLock lock(lock_);
+  AutoLock lock(lock_);
 
-    DCHECK(delegate_);  // At this point we must have a delegate.
-    session_state_ = session_state;
+  DCHECK(delegate_);  // At this point we must have a delegate.
+  session_state_ = session_state;
 
-    DCHECK(!dump_thread_);
-    dump_thread_ = std::move(dump_thread);
+  DCHECK(!dump_thread_);
+  dump_thread_ = std::move(dump_thread);
 
-    subtle::NoBarrier_Store(&memory_tracing_enabled_, 1);
+  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);
+  MemoryDumpScheduler::Config periodic_config = {};
+  MemoryPeakDetector::Config peak_config = {};
+  for (const auto& trigger : memory_dump_config.triggers) {
+    if (!session_state_->IsDumpModeAllowed(trigger.level_of_detail)) {
+      NOTREACHED();
+      continue;
     }
-
-    MemoryDumpScheduler* dump_scheduler = MemoryDumpScheduler::GetInstance();
-    dump_scheduler->Setup(this, dump_thread_->task_runner());
-    DCHECK_LE(memory_dump_config.triggers.size(), 3u);
-    for (const auto& trigger : memory_dump_config.triggers) {
-      if (!session_state_->IsDumpModeAllowed(trigger.level_of_detail)) {
-        NOTREACHED();
-        continue;
-      }
-      dump_scheduler->AddTrigger(trigger.trigger_type, trigger.level_of_detail,
-                                 trigger.min_time_between_dumps_ms);
+    if (trigger.trigger_type == MemoryDumpType::PERIODIC_INTERVAL) {
+      periodic_config.triggers.push_back(
+          {trigger.level_of_detail, trigger.min_time_between_dumps_ms});
+    } else if (trigger.trigger_type == MemoryDumpType::PEAK_MEMORY_USAGE) {
+      MemoryPeakDetector::GetInstance()->Setup(
+          Bind(&MemoryDumpManager::GetDumpProvidersForPolling,
+               Unretained(this)),
+          dump_thread_->task_runner(),
+          Bind(&OnPeakDetected, trigger.level_of_detail));

[ssid, 2017/04/05 22:04:22]

Actually there is no reason to have a Setup() and Start() function in
PeakDetector anymore. We needed 2 earlier because we added triggers and then
enabled only when we have dump providers. If we are going to call it from the
same place here then why have 2 functions?
I tried to point this in the first CL, but I thought you had some other issue
here.

[Primiano Tucci (use gerrit), 2017/04/10 19:16:41]

I want to keep them separate because they will be useful for uma when the peak
detector will be used by the service ;-)

+      peak_config.polling_interval_ms = 10;
+      peak_config.min_time_between_peaks_ms = trigger.min_time_between_dumps_ms;
+      peak_config.enable_verbose_poll_tracing =
+          trigger.level_of_detail == MemoryDumpLevelOfDetail::DETAILED;
     }
-
-    // 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->EnablePollingIfNeeded();
-
-    // Only coordinator process triggers periodic global memory dumps.
-    if (delegate_->IsCoordinator())
-      dump_scheduler->EnablePeriodicTriggerIfNeeded();
   }
 

[ssid, 2017/04/05 22:04:22]

Can we not add support for periodic+peak triggers in single session now?
I can't think of anything that would break. But, it sounds not very useful
anyway.

[Primiano Tucci (use gerrit), 2017/04/10 19:16:41]

Is there a specific reason? Nothing should break right? I think we might want to
use it at some point soon-ish in telemetry.

[ssid, 2017/04/10 21:35:50]

yeah should work fine. I will give it a try once the refactoring is done.

+  if (peak_config.min_time_between_peaks_ms)
+    MemoryPeakDetector::GetInstance()->Start(peak_config);

[ssid, 2017/04/05 22:04:22]

can move this inot the if above.

[Primiano Tucci (use gerrit), 2017/04/10 19:16:41]

?

[ssid, 2017/04/10 21:35:51]

I meant this can be done in the
if (trigger.trigger_type == MemoryDumpType::PEAK_MEMORY_USAGE)
condition above instead of checking for time being > 0 here. Easier to read.

[Primiano Tucci (use gerrit), 2017/04/11 11:43:08]

Ahh. Done

+
+  // Only coordinator process triggers periodic global memory dumps.
+  if (delegate_->IsCoordinator() && !periodic_config.triggers.empty()) {
+    MemoryDumpScheduler::GetInstance()->Start(periodic_config,
+                                              dump_thread_->task_runner());
+  }
 }
 
 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;
   {
     AutoLock lock(lock_);
+    MemoryDumpScheduler::GetInstance()->Stop();
+    MemoryPeakDetector::GetInstance()->TearDown();
     dump_thread = std::move(dump_thread_);
     session_state_ = nullptr;
-    MemoryDumpScheduler::GetInstance()->DisableAllTriggers();
   }
 
   // Thread stops are blocking and must be performed outside of the |lock_|
   // or will deadlock (e.g., if SetupNextMemoryDump() tries to acquire it).
   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_->IsDumpModeAllowed(dump_mode);
 }
 
 MemoryDumpManager::ProcessMemoryDumpAsyncState::ProcessMemoryDumpAsyncState(
@@ skipping 22 matching lines @@
   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();
 }
 
 }  // namespace trace_event
 }  // namespace base