Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(936)

Unified Diff: base/trace_event/memory_peak_detector.cc

Issue 2793023002: memory-infra: port peak detection logic to MemoryPeakDetector (Closed)
Patch Set: fix lambdas for MSVC Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: base/trace_event/memory_peak_detector.cc
diff --git a/base/trace_event/memory_peak_detector.cc b/base/trace_event/memory_peak_detector.cc
index c361037c2d800540edbcf72ee2bbd415e1d69fab..a1b2e12d1c53c9af186d7d380ac3ad2597060c62 100644
--- a/base/trace_event/memory_peak_detector.cc
+++ b/base/trace_event/memory_peak_detector.cc
@@ -8,9 +8,12 @@
#include "base/bind.h"
#include "base/logging.h"
+#include "base/sys_info.h"
#include "base/threading/sequenced_task_runner_handle.h"
#include "base/time/time.h"
+#include "base/trace_event/memory_dump_manager.h"
#include "base/trace_event/memory_dump_provider_info.h"
+#include "base/trace_event/trace_event.h"
namespace base {
namespace trace_event {
@@ -24,7 +27,6 @@ MemoryPeakDetector* MemoryPeakDetector::GetInstance() {
MemoryPeakDetector::MemoryPeakDetector()
: generation_(0),
state_(NOT_INITIALIZED),
- polling_interval_ms_(0),
poll_tasks_count_for_testing_(0) {}
MemoryPeakDetector::~MemoryPeakDetector() {
@@ -46,6 +48,12 @@ void MemoryPeakDetector::Setup(
task_runner_ = task_runner;
on_peak_detected_callback_ = on_peak_detected_callback;
state_ = DISABLED;
+ config_ = {};
+ ResetPollHistory();
+
+ // Set threshold to 1% of total system memory.
+ static_threshold_bytes_ =
+ static_cast<uint64_t>(SysInfo::AmountOfPhysicalMemory()) / 100;
}
void MemoryPeakDetector::TearDown() {
@@ -57,9 +65,13 @@ void MemoryPeakDetector::TearDown() {
task_runner_ = nullptr;
}
-void MemoryPeakDetector::Start() {
- task_runner_->PostTask(
- FROM_HERE, Bind(&MemoryPeakDetector::StartInternal, Unretained(this)));
+void MemoryPeakDetector::Start(MemoryPeakDetector::Config config) {
+ if (!config.polling_interval_ms) {
+ NOTREACHED();
+ return;
+ }
+ task_runner_->PostTask(FROM_HERE, Bind(&MemoryPeakDetector::StartInternal,
+ Unretained(this), config));
}
void MemoryPeakDetector::Stop() {
@@ -67,28 +79,35 @@ void MemoryPeakDetector::Stop() {
FROM_HERE, Bind(&MemoryPeakDetector::StopInternal, Unretained(this)));
}
+void MemoryPeakDetector::Clear() {
+ if (!task_runner_)
+ return; // Can be called before Setup().
+ task_runner_->PostTask(
+ FROM_HERE, Bind(&MemoryPeakDetector::ResetPollHistory, Unretained(this)));
+}
+
void MemoryPeakDetector::NotifyMemoryDumpProvidersChanged() {
- // It is possible to call this before the first Setup() call, in which case
- // we want to just make this a noop. The next Start() will fetch the MDP list.
if (!task_runner_)
- return;
+ return; // Can be called before Setup().
task_runner_->PostTask(
FROM_HERE,
Bind(&MemoryPeakDetector::ReloadDumpProvidersAndStartPollingIfNeeded,
Unretained(this)));
}
-void MemoryPeakDetector::StartInternal() {
+void MemoryPeakDetector::StartInternal(MemoryPeakDetector::Config config) {
DCHECK_EQ(DISABLED, state_);
state_ = ENABLED;
ssid 2017/04/05 21:23:46 DCHECK_GT(polling_interval_ms, 0) DCHECK_GT(min_ti
Primiano Tucci (use gerrit) 2017/04/06 20:07:10 This one is already done in ::Start() I added an e
ssid 2017/04/10 20:55:06 Acknowledged.
- polling_interval_ms_ = 1; // TODO(primiano): temporary until next CL.
+ config_ = config;
+ ResetPollHistory();
ssid 2017/04/05 21:26:46 Should we test if Start() Stop() Start() happened,
Primiano Tucci (use gerrit) 2017/04/06 20:07:10 Done. See RestartClearsState test
- // If there are any dump providers available, NotifyMemoryDumpProvidersChanged
- // will fetch them and start the polling. Otherwise this will remain in the
- // ENABLED state and the actual polling will start on the next call to
+ // If there are any dump providers available,
+ // NotifyMemoryDumpProvidersChanged will fetch them and start the polling.
+ // Otherwise this will remain in the ENABLED state and the actual polling
+ // will start on the next call to
// ReloadDumpProvidersAndStartPollingIfNeeded().
- // Depending on the sandbox model, it is possible that no polling-capable dump
- // providers will be ever available.
+ // Depending on the sandbox model, it is possible that no polling-capable
+ // dump providers will be ever available.
ReloadDumpProvidersAndStartPollingIfNeeded();
}
@@ -132,7 +151,7 @@ void MemoryPeakDetector::ReloadDumpProvidersAndStartPollingIfNeeded() {
}
void MemoryPeakDetector::PollMemoryAndDetectPeak(uint32_t expected_generation) {
- if (state_ != RUNNING || expected_generation != generation_)
+ if (state_ != RUNNING || generation_ != expected_generation)
return;
// We should never end up in a situation where state_ == RUNNING but all dump
@@ -140,24 +159,97 @@ void MemoryPeakDetector::PollMemoryAndDetectPeak(uint32_t expected_generation) {
DCHECK(!dump_providers_.empty());
poll_tasks_count_for_testing_++;
- uint64_t memory_total = 0;
+ uint64_t polled_mem_bytes = 0;
for (const scoped_refptr<MemoryDumpProviderInfo>& mdp_info :
dump_providers_) {
DCHECK(mdp_info->options.is_fast_polling_supported);
uint64_t value = 0;
mdp_info->dump_provider->PollFastMemoryTotal(&value);
- memory_total += value;
+ polled_mem_bytes += value;
}
- ignore_result(memory_total); // TODO(primiano): temporary until next CL.
+ if (config_.enable_verbose_poll_tracing) {
+ TRACE_COUNTER1(MemoryDumpManager::kTraceCategory, "PolledMemoryKB",
+ polled_mem_bytes / 1024);
ssid 2017/04/05 21:23:46 MB is easier to read in TV.
Primiano Tucci (use gerrit) 2017/04/06 20:07:10 Ahh I see, good point. done
+ }
+
+ // Peak detection logic. Design doc: https://goo.gl/0kOU4A .
+ bool is_peak = false;
+ if (skip_polls_ > 0) {
+ skip_polls_--;
+ } else if (last_dump_memory_total_ == 0) {
+ last_dump_memory_total_ = polled_mem_bytes;
+ } else if (polled_mem_bytes > 0) {
+ int64_t diff_from_last_dump = polled_mem_bytes - last_dump_memory_total_;
- // TODO(primiano): Move actual peak detection logic from the
- // MemoryDumpScheduler in next CLs.
+ DCHECK_GT(static_threshold_bytes_, 0u);
+ is_peak = static_threshold_bytes_ &&
ssid 2017/04/05 21:23:46 why this condition after dcheck above? should we n
Primiano Tucci (use gerrit) 2017/04/06 20:07:10 I expect AmountOfPhysicalMemory can be 0 in tests
+ (diff_from_last_dump > static_threshold_bytes_);
+
+ if (!is_peak)
+ is_peak = DetectPeakUsingSlidingWindowStddev(polled_mem_bytes);
+ }
+
+ if (is_peak) {
+ TRACE_EVENT_INSTANT1(MemoryDumpManager::kTraceCategory,
+ "Peak memory dump detected", TRACE_EVENT_SCOPE_PROCESS,
ssid 2017/04/05 21:23:46 "Peak memory detected"? Also can we keep this in M
Primiano Tucci (use gerrit) 2017/04/06 20:07:10 Done.
+ "PolledMemoryKB", polled_mem_bytes / 1024);
+ on_peak_detected_callback_.Run();
+ ResetPollHistory();
+ last_dump_memory_total_ = polled_mem_bytes;
+ }
SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
Bind(&MemoryPeakDetector::PollMemoryAndDetectPeak, Unretained(this),
expected_generation),
- TimeDelta::FromMilliseconds(polling_interval_ms_));
+ TimeDelta::FromMilliseconds(config_.polling_interval_ms));
+}
+
+bool MemoryPeakDetector::DetectPeakUsingSlidingWindowStddev(
+ uint64_t polled_mem_bytes) {
+ DCHECK(polled_mem_bytes);
+ samples_bytes_[samples_index_] = polled_mem_bytes;
+ samples_index_ = (samples_index_ + 1) % kSlidingWindowNumSamples;
+ uint64_t mean = 0;
+ for (uint32_t i = 0; i < kSlidingWindowNumSamples; ++i) {
+ if (samples_bytes_[i] == 0)
+ return false; // Not enough samples to detect peaks.
+ mean += samples_bytes_[i];
+ }
+ mean /= kSlidingWindowNumSamples;
+ uint64_t variance = 0;
ssid 2017/04/05 21:23:46 Either make this double or make the sample kb. A s
Primiano Tucci (use gerrit) 2017/04/06 20:07:10 Oh excellent point.
+ for (uint32_t i = 0; i < kSlidingWindowNumSamples; ++i) {
+ const uint64_t deviation = samples_bytes_[i] - mean;
+ variance += deviation * deviation;
+ }
+ variance /= kSlidingWindowNumSamples;
+
+ // If stddev is less than 0.2% then we consider that the process is inactive.
+ if (variance < (mean / 500) * (mean / 500))
+ return false;
+
+ // (mean + 3.69 * stddev) corresponds to a value that is higher than current
+ // sample with 99.99% probability.
+ const uint64_t cur_sample_deviation = polled_mem_bytes - mean;
+ return cur_sample_deviation * cur_sample_deviation > (3.69 * 3.69 * variance);
+}
+
+void MemoryPeakDetector::ResetPollHistory() {
+ skip_polls_ = 0;
+ last_dump_memory_total_ = 0;
ssid 2017/04/05 21:23:46 hm shouldn't this be set to the current value inst
Primiano Tucci (use gerrit) 2017/04/06 20:07:10 Does it make a difference in practice? If I set th
ssid 2017/04/10 20:55:06 The issue is: If we detect peak in browser and ren
Primiano Tucci (use gerrit) 2017/04/11 11:12:09 Okay I changes this, but still not sure this makes
+ memset(samples_bytes_, 0, sizeof(samples_bytes_));
+ samples_index_ = 0;
+ if (config_.polling_interval_ms > 0) {
ssid 2017/04/05 21:23:46 Maybe we should check for state_ == running or ena
Primiano Tucci (use gerrit) 2017/04/06 20:07:10 No I use this also to clearup during Setup().
ssid 2017/04/10 20:55:06 hm that is true. Maybe we could just do it in next
Primiano Tucci (use gerrit) 2017/04/11 11:12:09 I just added a bool bool keep_last_sample here, so
+ skip_polls_ =
+ (config_.min_time_between_peaks_ms + config_.polling_interval_ms - 1) /
+ config_.polling_interval_ms;
+ }
+}
+
+void MemoryPeakDetector::SetStaticThresholdForTesting(
+ uint64_t static_threshold_bytes) {
+ DCHECK_EQ(DISABLED, state_);
+ static_threshold_bytes_ = static_threshold_bytes;
}
} // namespace trace_event

Powered by Google App Engine
This is Rietveld 408576698