Commit 87557295 authored by Sebastien Marchand's avatar Sebastien Marchand Committed by Commit Bot

Use system_monitor to gather the System Stats chrome://tracing data

This removes all the PostTask / Timer logic from
trace_event_system_stats_monitor and make it use SystemMonitor instead.

This simplify the trace_event_system_stats_monitor code and will make it
possible to add more system metrics to the system_stats tracing
category (the goal is to replace some of these raw numbers by some more
useful ones as they're currently really easy to misinterpret)

Change-Id: I6ea2ab5c4f12f329b5f28d3b84ab48efb57710d3
Reviewed-on: https://chromium-review.googlesource.com/c/1446772Reviewed-by: default avatarNico Weber <thakis@chromium.org>
Reviewed-by: default avataroysteine <oysteine@chromium.org>
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636297}
parent cee9bb25
...@@ -79,6 +79,7 @@ ...@@ -79,6 +79,7 @@
#include "chrome/browser/metrics/thread_watcher.h" #include "chrome/browser/metrics/thread_watcher.h"
#include "chrome/browser/nacl_host/nacl_browser_delegate_impl.h" #include "chrome/browser/nacl_host/nacl_browser_delegate_impl.h"
#include "chrome/browser/performance_monitor/process_monitor.h" #include "chrome/browser/performance_monitor/process_monitor.h"
#include "chrome/browser/performance_monitor/system_monitor.h"
#include "chrome/browser/plugins/plugin_prefs.h" #include "chrome/browser/plugins/plugin_prefs.h"
#include "chrome/browser/policy/chrome_browser_policy_connector.h" #include "chrome/browser/policy/chrome_browser_policy_connector.h"
#include "chrome/browser/prefs/chrome_command_line_pref_store.h" #include "chrome/browser/prefs/chrome_command_line_pref_store.h"
...@@ -838,6 +839,8 @@ void ChromeBrowserMainParts::PostMainMessageLoopStart() { ...@@ -838,6 +839,8 @@ void ChromeBrowserMainParts::PostMainMessageLoopStart() {
heap_profiler_controller_->StartIfEnabled(); heap_profiler_controller_->StartIfEnabled();
system_monitor_ = performance_monitor::SystemMonitor::Create();
// TODO(sebmarchand): Allow this to be created earlier if startup tracing is // TODO(sebmarchand): Allow this to be created earlier if startup tracing is
// enabled. // enabled.
trace_event_system_stats_monitor_ = trace_event_system_stats_monitor_ =
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#define CHROME_BROWSER_CHROME_BROWSER_MAIN_H_ #define CHROME_BROWSER_CHROME_BROWSER_MAIN_H_
#include <memory> #include <memory>
#include <vector>
#include "base/macros.h" #include "base/macros.h"
#include "build/build_config.h" #include "build/build_config.h"
...@@ -33,6 +34,10 @@ namespace tracing { ...@@ -33,6 +34,10 @@ namespace tracing {
class TraceEventSystemStatsMonitor; class TraceEventSystemStatsMonitor;
} }
namespace performance_monitor {
class SystemMonitor;
}
class ChromeBrowserMainParts : public content::BrowserMainParts { class ChromeBrowserMainParts : public content::BrowserMainParts {
public: public:
~ChromeBrowserMainParts() override; ~ChromeBrowserMainParts() override;
...@@ -158,6 +163,10 @@ class ChromeBrowserMainParts : public content::BrowserMainParts { ...@@ -158,6 +163,10 @@ class ChromeBrowserMainParts : public content::BrowserMainParts {
// the reporting pipeline. // the reporting pipeline.
std::unique_ptr<HeapProfilerController> heap_profiler_controller_; std::unique_ptr<HeapProfilerController> heap_profiler_controller_;
// The system monitor instance, used by some subsystems to collect the system
// metrics they need.
std::unique_ptr<performance_monitor::SystemMonitor> system_monitor_;
// The system stats monitor used by chrome://tracing. This doesn't do anything // The system stats monitor used by chrome://tracing. This doesn't do anything
// until tracing of the |system_stats| category is enabled. // until tracing of the |system_stats| category is enabled.
std::unique_ptr<tracing::TraceEventSystemStatsMonitor> std::unique_ptr<tracing::TraceEventSystemStatsMonitor>
......
...@@ -85,6 +85,11 @@ void SystemMonitor::SystemObserver::OnDiskIdleTimePercent( ...@@ -85,6 +85,11 @@ void SystemMonitor::SystemObserver::OnDiskIdleTimePercent(
NOTREACHED(); NOTREACHED();
} }
void SystemMonitor::SystemObserver::OnSystemMetricsStruct(
const base::SystemMetrics& system_metrics) {
NOTREACHED();
}
void SystemMonitor::AddOrUpdateObserver( void SystemMonitor::AddOrUpdateObserver(
SystemMonitor::SystemObserver* observer, SystemMonitor::SystemObserver* observer,
MetricRefreshFrequencies metrics_and_frequencies) { MetricRefreshFrequencies metrics_and_frequencies) {
...@@ -126,13 +131,12 @@ SystemMonitor::MetricVector SystemMonitor::EvaluateMetrics( ...@@ -126,13 +131,12 @@ SystemMonitor::MetricVector SystemMonitor::EvaluateMetrics(
} }
SystemMonitor::MetricMetadataArray SystemMonitor::CreateMetricMetadataArray() { SystemMonitor::MetricMetadataArray SystemMonitor::CreateMetricMetadataArray() {
#define CREATE_METRIC_METADATA(metric_type, metric_value_type, \ #define CREATE_METRIC_METADATA(metric_type, value_type, helper_function, \
helper_function, notify_function, \ notify_function, metric_freq_field) \
metric_freq_field) \
MetricMetadata( \ MetricMetadata( \
[](MetricEvaluatorsHelper* helper) { \ [](MetricEvaluatorsHelper* helper) { \
std::unique_ptr<MetricEvaluator> metric = \ std::unique_ptr<MetricEvaluator> metric = \
base::WrapUnique(new MetricEvaluatorImpl<metric_value_type>( \ base::WrapUnique(new MetricEvaluatorImpl<value_type>( \
MetricEvaluator::Type::metric_type, \ MetricEvaluator::Type::metric_type, \
base::BindOnce(&MetricEvaluatorsHelper::helper_function, \ base::BindOnce(&MetricEvaluatorsHelper::helper_function, \
base::Unretained(helper)), \ base::Unretained(helper)), \
...@@ -143,6 +147,7 @@ SystemMonitor::MetricMetadataArray SystemMonitor::CreateMetricMetadataArray() { ...@@ -143,6 +147,7 @@ SystemMonitor::MetricMetadataArray SystemMonitor::CreateMetricMetadataArray() {
metric_refresh_frequencies) { \ metric_refresh_frequencies) { \
return metric_refresh_frequencies.metric_freq_field; \ return metric_refresh_frequencies.metric_freq_field; \
}) })
return { return {
CREATE_METRIC_METADATA(kFreeMemoryMb, int, GetFreePhysicalMemoryMb, CREATE_METRIC_METADATA(kFreeMemoryMb, int, GetFreePhysicalMemoryMb,
OnFreePhysicalMemoryMbSample, OnFreePhysicalMemoryMbSample,
...@@ -150,7 +155,11 @@ SystemMonitor::MetricMetadataArray SystemMonitor::CreateMetricMetadataArray() { ...@@ -150,7 +155,11 @@ SystemMonitor::MetricMetadataArray SystemMonitor::CreateMetricMetadataArray() {
CREATE_METRIC_METADATA(kDiskIdleTimePercent, float, CREATE_METRIC_METADATA(kDiskIdleTimePercent, float,
GetDiskIdleTimePercent, OnDiskIdleTimePercent, GetDiskIdleTimePercent, OnDiskIdleTimePercent,
disk_idle_time_percent_frequency), disk_idle_time_percent_frequency),
CREATE_METRIC_METADATA(kSystemMetricsStruct, base::SystemMetrics,
GetSystemMetricsStruct, OnSystemMetricsStruct,
system_metrics_sampling_frequency),
}; };
#undef CREATE_METRIC_METADATA #undef CREATE_METRIC_METADATA
} // namespace performance_monitor } // namespace performance_monitor
...@@ -222,7 +231,7 @@ template <typename T> ...@@ -222,7 +231,7 @@ template <typename T>
SystemMonitor::MetricEvaluatorImpl<T>::MetricEvaluatorImpl( SystemMonitor::MetricEvaluatorImpl<T>::MetricEvaluatorImpl(
Type type, Type type,
base::OnceCallback<base::Optional<T>()> evaluate_function, base::OnceCallback<base::Optional<T>()> evaluate_function,
void (SystemObserver::*notify_function)(T)) void (SystemObserver::*notify_function)(ObserverArgType))
: MetricEvaluator(type), : MetricEvaluator(type),
evaluate_function_(std::move(evaluate_function)), evaluate_function_(std::move(evaluate_function)),
notify_function_(notify_function) {} notify_function_(notify_function) {}
...@@ -251,4 +260,9 @@ void SystemMonitor::MetricEvaluatorImpl<T>::Evaluate() { ...@@ -251,4 +260,9 @@ void SystemMonitor::MetricEvaluatorImpl<T>::Evaluate() {
value_ = std::move(evaluate_function_).Run(); value_ = std::move(evaluate_function_).Run();
} }
base::Optional<base::SystemMetrics>
MetricEvaluatorsHelper::GetSystemMetricsStruct() {
return base::SystemMetrics::Sample();
}
} // namespace performance_monitor } // namespace performance_monitor
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/observer_list.h" #include "base/observer_list.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/process/process_metrics.h"
#include "base/sequence_checker.h" #include "base/sequence_checker.h"
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "base/time/time.h" #include "base/time/time.h"
...@@ -71,6 +72,9 @@ class SystemMonitor { ...@@ -71,6 +72,9 @@ class SystemMonitor {
SamplingFrequency disk_idle_time_percent_frequency = SamplingFrequency disk_idle_time_percent_frequency =
SamplingFrequency::kNoSampling; SamplingFrequency::kNoSampling;
SamplingFrequency system_metrics_sampling_frequency =
SamplingFrequency::kNoSampling;
}; };
~SystemObserver() override = default; ~SystemObserver() override = default;
...@@ -81,6 +85,10 @@ class SystemMonitor { ...@@ -81,6 +85,10 @@ class SystemMonitor {
// Reports the disk idle time during the last observation interval, in // Reports the disk idle time during the last observation interval, in
// percent (between 0.0 and 1.0). // percent (between 0.0 and 1.0).
virtual void OnDiskIdleTimePercent(float disk_idle_time_percent); virtual void OnDiskIdleTimePercent(float disk_idle_time_percent);
// Called when a new |base::SystemMetrics| sample is available.
virtual void OnSystemMetricsStruct(
const base::SystemMetrics& system_metrics);
}; };
using ObserverToFrequenciesMap = using ObserverToFrequenciesMap =
base::flat_map<SystemObserver*, SystemObserver::MetricRefreshFrequencies>; base::flat_map<SystemObserver*, SystemObserver::MetricRefreshFrequencies>;
...@@ -113,8 +121,14 @@ class SystemMonitor { ...@@ -113,8 +121,14 @@ class SystemMonitor {
class MetricEvaluator { class MetricEvaluator {
public: public:
enum class Type : size_t { enum class Type : size_t {
// The amount of free physical memory, in megabytes.
kFreeMemoryMb, kFreeMemoryMb,
// The percentage of time the disk has been idle since the sample.
kDiskIdleTimePercent, kDiskIdleTimePercent,
// A |base::SystemMetrics| instance.
// TODO(sebmarchand): Split this struct into some smaller ones.
kSystemMetricsStruct,
kMax, kMax,
}; };
...@@ -144,10 +158,13 @@ class SystemMonitor { ...@@ -144,10 +158,13 @@ class SystemMonitor {
template <typename T> template <typename T>
class MetricEvaluatorImpl : public MetricEvaluator { class MetricEvaluatorImpl : public MetricEvaluator {
public: public:
using ObserverArgType =
typename std::conditional<std::is_scalar<T>::value, T, const T&>::type;
MetricEvaluatorImpl<T>( MetricEvaluatorImpl<T>(
Type type, Type type,
base::OnceCallback<base::Optional<T>()> evaluate_function, base::OnceCallback<base::Optional<T>()> evaluate_function,
void (SystemObserver::*notify_function)(T)); void (SystemObserver::*notify_function)(ObserverArgType));
virtual ~MetricEvaluatorImpl(); virtual ~MetricEvaluatorImpl();
// Called when the metrics needs to be refreshed. // Called when the metrics needs to be refreshed.
...@@ -167,7 +184,7 @@ class SystemMonitor { ...@@ -167,7 +184,7 @@ class SystemMonitor {
// A function pointer to the SystemObserver function that should be called // A function pointer to the SystemObserver function that should be called
// to notify of a value refresh. // to notify of a value refresh.
void (SystemObserver::*notify_function_)(T); void (SystemObserver::*notify_function_)(ObserverArgType);
// The value, initialized in |Evaluate|. // The value, initialized in |Evaluate|.
base::Optional<T> value_; base::Optional<T> value_;
...@@ -286,6 +303,12 @@ class MetricEvaluatorsHelper { ...@@ -286,6 +303,12 @@ class MetricEvaluatorsHelper {
// this function (returns nullopt on the first call). // this function (returns nullopt on the first call).
virtual base::Optional<float> GetDiskIdleTimePercent() = 0; virtual base::Optional<float> GetDiskIdleTimePercent() = 0;
// Return a |base::SystemMetrics| snapshot.
//
// NOTE: This function doesn't have to be virtual, the base::SystemMetrics
// struct is an abstraction that already has a per-platform definition.
base::Optional<base::SystemMetrics> GetSystemMetricsStruct();
private: private:
DISALLOW_COPY_AND_ASSIGN(MetricEvaluatorsHelper); DISALLOW_COPY_AND_ASSIGN(MetricEvaluatorsHelper);
}; };
......
...@@ -26,6 +26,8 @@ class MockMetricsMonitorObserver : public SystemObserver { ...@@ -26,6 +26,8 @@ class MockMetricsMonitorObserver : public SystemObserver {
~MockMetricsMonitorObserver() override {} ~MockMetricsMonitorObserver() override {}
MOCK_METHOD1(OnFreePhysicalMemoryMbSample, void(int free_phys_memory_mb)); MOCK_METHOD1(OnFreePhysicalMemoryMbSample, void(int free_phys_memory_mb));
MOCK_METHOD1(OnDiskIdleTimePercent, void(float disk_idle_time_percent)); MOCK_METHOD1(OnDiskIdleTimePercent, void(float disk_idle_time_percent));
MOCK_METHOD1(OnSystemMetricsStruct,
void(const base::SystemMetrics& system_metrics));
}; };
// Test version of a MetricEvaluatorsHelper that returns constant values. // Test version of a MetricEvaluatorsHelper that returns constant values.
...@@ -66,18 +68,25 @@ class SystemMonitorTest : public testing::Test { ...@@ -66,18 +68,25 @@ class SystemMonitorTest : public testing::Test {
void TearDown() override { system_monitor_.reset(); } void TearDown() override { system_monitor_.reset(); }
void EnsureMetricsAreObservedAtExpectedFrequency( void EnsureMetricsAreObservedAtExpectedFrequency(
SamplingFrequency expected_free_memory_mb_freq, SamplingFrequency expected_free_memory_mb_freq =
SamplingFrequency expected_disk_idle_time_percent_freq) { SamplingFrequency::kNoSampling,
SamplingFrequency expected_disk_idle_time_percent_freq =
SamplingFrequency::kNoSampling,
SamplingFrequency expected_system_metrics_struct_freq =
SamplingFrequency::kNoSampling) {
const auto& observed_metrics_and_frequencies = const auto& observed_metrics_and_frequencies =
system_monitor_->GetMetricSamplingFrequencyArrayForTesting(); system_monitor_->GetMetricSamplingFrequencyArrayForTesting();
EXPECT_EQ(2U, observed_metrics_and_frequencies.size()); EXPECT_EQ(3U, observed_metrics_and_frequencies.size());
EXPECT_EQ(expected_free_memory_mb_freq, EXPECT_EQ(expected_free_memory_mb_freq,
observed_metrics_and_frequencies[static_cast<size_t>( observed_metrics_and_frequencies[static_cast<size_t>(
SystemMonitor::MetricEvaluator::Type::kFreeMemoryMb)]); SystemMonitor::MetricEvaluator::Type::kFreeMemoryMb)]);
EXPECT_EQ(expected_disk_idle_time_percent_freq, EXPECT_EQ(expected_disk_idle_time_percent_freq,
observed_metrics_and_frequencies[static_cast<size_t>( observed_metrics_and_frequencies[static_cast<size_t>(
SystemMonitor::MetricEvaluator::Type::kDiskIdleTimePercent)]); SystemMonitor::MetricEvaluator::Type::kDiskIdleTimePercent)]);
EXPECT_EQ(expected_system_metrics_struct_freq,
observed_metrics_and_frequencies[static_cast<size_t>(
SystemMonitor::MetricEvaluator::Type::kSystemMetricsStruct)]);
} }
std::unique_ptr<SystemMonitor> system_monitor_; std::unique_ptr<SystemMonitor> system_monitor_;
...@@ -102,8 +111,7 @@ TEST_F(SystemMonitorTest, AddAndUpdateObservers) { ...@@ -102,8 +111,7 @@ TEST_F(SystemMonitorTest, AddAndUpdateObservers) {
// The first observer doesn't observe anything yet. // The first observer doesn't observe anything yet.
MetricsRefreshFrequencies obs1_metrics_frequencies = {}; MetricsRefreshFrequencies obs1_metrics_frequencies = {};
system_monitor_->AddOrUpdateObserver(&obs1, obs1_metrics_frequencies); system_monitor_->AddOrUpdateObserver(&obs1, obs1_metrics_frequencies);
EnsureMetricsAreObservedAtExpectedFrequency(SamplingFrequency::kNoSampling, EnsureMetricsAreObservedAtExpectedFrequency();
SamplingFrequency::kNoSampling);
// Add a second observer that observes the amount of free memory at the // Add a second observer that observes the amount of free memory at the
// default frequency. // default frequency.
...@@ -111,7 +119,7 @@ TEST_F(SystemMonitorTest, AddAndUpdateObservers) { ...@@ -111,7 +119,7 @@ TEST_F(SystemMonitorTest, AddAndUpdateObservers) {
.free_phys_memory_mb_frequency = SamplingFrequency::kDefaultFrequency}; .free_phys_memory_mb_frequency = SamplingFrequency::kDefaultFrequency};
system_monitor_->AddOrUpdateObserver(&obs2, obs2_metrics_frequencies); system_monitor_->AddOrUpdateObserver(&obs2, obs2_metrics_frequencies);
EnsureMetricsAreObservedAtExpectedFrequency( EnsureMetricsAreObservedAtExpectedFrequency(
SamplingFrequency::kDefaultFrequency, SamplingFrequency::kNoSampling); SamplingFrequency::kDefaultFrequency);
// Add a third observer that observes the amount of free memory and the disk // Add a third observer that observes the amount of free memory and the disk
// idle time at the default frequency. // idle time at the default frequency.
...@@ -135,8 +143,7 @@ TEST_F(SystemMonitorTest, AddAndUpdateObservers) { ...@@ -135,8 +143,7 @@ TEST_F(SystemMonitorTest, AddAndUpdateObservers) {
// Remove the third observer, ensure that no metrics are observed anymore. // Remove the third observer, ensure that no metrics are observed anymore.
system_monitor_->RemoveObserver(&obs3); system_monitor_->RemoveObserver(&obs3);
EnsureMetricsAreObservedAtExpectedFrequency(SamplingFrequency::kNoSampling, EnsureMetricsAreObservedAtExpectedFrequency();
SamplingFrequency::kNoSampling);
} }
TEST_F(SystemMonitorTest, ObserverGetsCalled) { TEST_F(SystemMonitorTest, ObserverGetsCalled) {
...@@ -147,8 +154,17 @@ TEST_F(SystemMonitorTest, ObserverGetsCalled) { ...@@ -147,8 +154,17 @@ TEST_F(SystemMonitorTest, ObserverGetsCalled) {
::testing::StrictMock<MockMetricsMonitorObserver> mock_observer_2; ::testing::StrictMock<MockMetricsMonitorObserver> mock_observer_2;
system_monitor_->AddOrUpdateObserver( system_monitor_->AddOrUpdateObserver(
&mock_observer_2, {.disk_idle_time_percent_frequency = &mock_observer_2, {
SamplingFrequency::kDefaultFrequency}); .disk_idle_time_percent_frequency =
SamplingFrequency::kDefaultFrequency,
.system_metrics_sampling_frequency =
SamplingFrequency::kDefaultFrequency,
});
EnsureMetricsAreObservedAtExpectedFrequency(
SamplingFrequency::kDefaultFrequency,
SamplingFrequency::kDefaultFrequency,
SamplingFrequency::kDefaultFrequency);
// Ensure that we get several samples to verify that the timer logic works. // Ensure that we get several samples to verify that the timer logic works.
EXPECT_CALL(mock_observer_1, EXPECT_CALL(mock_observer_1,
...@@ -157,6 +173,7 @@ TEST_F(SystemMonitorTest, ObserverGetsCalled) { ...@@ -157,6 +173,7 @@ TEST_F(SystemMonitorTest, ObserverGetsCalled) {
EXPECT_CALL(mock_observer_2, OnDiskIdleTimePercent(kFakeDiskIdleTimePercent)) EXPECT_CALL(mock_observer_2, OnDiskIdleTimePercent(kFakeDiskIdleTimePercent))
.Times(2); .Times(2);
EXPECT_CALL(mock_observer_2, OnSystemMetricsStruct(::testing::_)).Times(2);
// Fast forward by enough time to get multiple samples and wait for the tasks // Fast forward by enough time to get multiple samples and wait for the tasks
// to complete. // to complete.
......
...@@ -5,85 +5,60 @@ ...@@ -5,85 +5,60 @@
#include "chrome/browser/tracing/trace_event_system_stats_monitor.h" #include "chrome/browser/tracing/trace_event_system_stats_monitor.h"
#include <memory> #include <memory>
#include <string>
#include <utility>
#include "base/bind.h"
#include "base/json/json_writer.h" #include "base/json/json_writer.h"
#include "base/process/process_metrics.h" #include "base/process/process_metrics.h"
#include "base/task/post_task.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/trace_event/trace_event.h" #include "base/trace_event/trace_event.h"
namespace tracing { namespace tracing {
namespace { namespace {
// Length of time interval between stat profiles. using SamplingFrequency = performance_monitor::SystemMonitor::SamplingFrequency;
static const int kSamplingIntervalMilliseconds = 2000;
// Converts system memory profiling stats in |input| to trace event compatible
// JSON and appends to |output|.
void AppendSystemProfileAsTraceFormat(const base::SystemMetrics& system_metrics,
std::string* output) {
std::string tmp;
base::JSONWriter::Write(*system_metrics.ToValue(), &tmp);
*output += tmp;
}
///////////////////////////////////////////////////////////////////////////// /////////////////////////////////////////////////////////////////////////////
// Holds profiled system stats until the tracing system needs to serialize it. // Holds profiled system stats until the tracing system needs to serialize it.
class SystemStatsHolder : public base::trace_event::ConvertableToTraceFormat { class SystemStatsHolder : public base::trace_event::ConvertableToTraceFormat {
public: public:
SystemStatsHolder() = default; explicit SystemStatsHolder(const base::SystemMetrics& system_metrics)
: system_metrics_(system_metrics) {}
~SystemStatsHolder() override = default; ~SystemStatsHolder() override = default;
// Fills system_metrics_ with profiled system memory and disk stats.
// Uses the previous stats to compute rates if this is not the first profile.
void GetSystemProfilingStats();
// base::trace_event::ConvertableToTraceFormat overrides: // base::trace_event::ConvertableToTraceFormat overrides:
void AppendAsTraceFormat(std::string* out) const override { void AppendAsTraceFormat(std::string* out) const override {
AppendSystemProfileAsTraceFormat(system_stats_, out); std::string tmp;
base::JSONWriter::Write(*system_metrics_.ToValue(), &tmp);
*out += tmp;
} }
private: private:
base::SystemMetrics system_stats_; const base::SystemMetrics system_metrics_;
DISALLOW_COPY_AND_ASSIGN(SystemStatsHolder); DISALLOW_COPY_AND_ASSIGN(SystemStatsHolder);
}; };
void SystemStatsHolder::GetSystemProfilingStats() {
system_stats_ = base::SystemMetrics::Sample();
}
void DumpSystemStatsImpl(TraceEventSystemStatsMonitor* stats_monitor) {
std::unique_ptr<SystemStatsHolder> dump_holder =
std::make_unique<SystemStatsHolder>();
dump_holder->GetSystemProfilingStats();
TRACE_EVENT_OBJECT_SNAPSHOT_WITH_ID(
TRACE_DISABLED_BY_DEFAULT("system_stats"),
"base::TraceEventSystemStatsMonitor::SystemStats", stats_monitor,
std::move(dump_holder));
}
} // namespace } // namespace
////////////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////////////
TraceEventSystemStatsMonitor::TraceEventSystemStatsMonitor() TraceEventSystemStatsMonitor::TraceEventSystemStatsMonitor()
: task_runner_(base::ThreadTaskRunnerHandle::Get()), weak_factory_(this) { : weak_factory_(this) {
// Force the "system_stats" category to show up in the trace viewer. // Force the "system_stats" category to show up in the trace viewer.
base::trace_event::TraceLog::GetCategoryGroupEnabled( base::trace_event::TraceLog::GetCategoryGroupEnabled(
TRACE_DISABLED_BY_DEFAULT("system_stats")); TRACE_DISABLED_BY_DEFAULT("system_stats"));
// Allow this to be instantiated on unsupported platforms, but don't run. // Allow this to be instantiated on unsupported platforms, but don't run.
base::trace_event::TraceLog::GetInstance()->AddEnabledStateObserver(this); base::trace_event::TraceLog::GetInstance()->AddAsyncEnabledStateObserver(
weak_factory_.GetWeakPtr());
} }
TraceEventSystemStatsMonitor::~TraceEventSystemStatsMonitor() { TraceEventSystemStatsMonitor::~TraceEventSystemStatsMonitor() {
if (dump_timer_.IsRunning()) if (is_profiling_)
StopProfiling(); StopProfiling();
base::trace_event::TraceLog::GetInstance()->RemoveEnabledStateObserver(this); base::trace_event::TraceLog::GetInstance()->RemoveAsyncEnabledStateObserver(
this);
} }
void TraceEventSystemStatsMonitor::OnTraceLogEnabled() { void TraceEventSystemStatsMonitor::OnTraceLogEnabled() {
...@@ -94,48 +69,43 @@ void TraceEventSystemStatsMonitor::OnTraceLogEnabled() { ...@@ -94,48 +69,43 @@ void TraceEventSystemStatsMonitor::OnTraceLogEnabled() {
&enabled); &enabled);
if (!enabled) if (!enabled)
return; return;
task_runner_->PostTask( StartProfiling();
FROM_HERE, base::BindOnce(&TraceEventSystemStatsMonitor::StartProfiling,
weak_factory_.GetWeakPtr()));
} }
void TraceEventSystemStatsMonitor::OnTraceLogDisabled() { void TraceEventSystemStatsMonitor::OnTraceLogDisabled() {
task_runner_->PostTask( StopProfiling();
FROM_HERE, base::BindOnce(&TraceEventSystemStatsMonitor::StopProfiling,
weak_factory_.GetWeakPtr()));
} }
void TraceEventSystemStatsMonitor::StartProfiling() { void TraceEventSystemStatsMonitor::StartProfiling() {
// Watch for the tracing framework sending enabling more than once. // Watch for the tracing framework sending enabling more than once.
if (dump_timer_.IsRunning()) if (is_profiling_)
return; return;
dump_timer_.Start( is_profiling_ = true;
FROM_HERE, DCHECK(performance_monitor::SystemMonitor::Get());
base::TimeDelta::FromMilliseconds(kSamplingIntervalMilliseconds), performance_monitor::SystemMonitor::Get()->AddOrUpdateObserver(
base::BindRepeating(&TraceEventSystemStatsMonitor::DumpSystemStats, this, {.system_metrics_sampling_frequency =
weak_factory_.GetWeakPtr())); SamplingFrequency::kDefaultFrequency});
} }
void TraceEventSystemStatsMonitor::DumpSystemStats() { void TraceEventSystemStatsMonitor::OnSystemMetricsStruct(
// Calls to |DumpSystemStatsImpl| might be blocking. const base::SystemMetrics& system_metrics) {
// std::unique_ptr<SystemStatsHolder> dump_holder =
// TODO(sebmarchand): Ideally the timer that calls this function should use a std::make_unique<SystemStatsHolder>(system_metrics);
// thread with the MayBlock trait to avoid having to use a trampoline here.
// This isn't currently possible due to https://crbug.com/896990.
base::PostTaskWithTraits(
FROM_HERE,
{base::MayBlock(), base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
base::BindOnce(&DumpSystemStatsImpl, base::Unretained(this)));
}
void TraceEventSystemStatsMonitor::StopProfiling() { TRACE_EVENT_OBJECT_SNAPSHOT_WITH_ID(
dump_timer_.Stop(); TRACE_DISABLED_BY_DEFAULT("system_stats"),
"base::TraceEventSystemStatsMonitor::SystemStats",
static_cast<void*>(this), std::move(dump_holder));
} }
bool TraceEventSystemStatsMonitor::IsTimerRunningForTesting() const { void TraceEventSystemStatsMonitor::StopProfiling() {
return dump_timer_.IsRunning(); // Watch for the tracing framework sending disabling more than once.
if (is_profiling_) {
is_profiling_ = false;
if (auto* sys_monitor = performance_monitor::SystemMonitor::Get())
sys_monitor->RemoveObserver(this);
}
} }
} // namespace tracing } // namespace tracing
...@@ -5,16 +5,11 @@ ...@@ -5,16 +5,11 @@
#ifndef CHROME_BROWSER_TRACING_TRACE_EVENT_SYSTEM_STATS_MONITOR_H_ #ifndef CHROME_BROWSER_TRACING_TRACE_EVENT_SYSTEM_STATS_MONITOR_H_
#define CHROME_BROWSER_TRACING_TRACE_EVENT_SYSTEM_STATS_MONITOR_H_ #define CHROME_BROWSER_TRACING_TRACE_EVENT_SYSTEM_STATS_MONITOR_H_
#include <string>
#include <memory>
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/process/process_metrics.h" #include "base/process/process_metrics.h"
#include "base/timer/timer.h"
#include "base/trace_event/trace_log.h" #include "base/trace_event/trace_log.h"
#include "chrome/browser/performance_monitor/system_monitor.h"
namespace tracing { namespace tracing {
...@@ -22,7 +17,8 @@ namespace tracing { ...@@ -22,7 +17,8 @@ namespace tracing {
// enabled, also enables system events profiling. This class is the preferred // enabled, also enables system events profiling. This class is the preferred
// way to turn system tracing on and off. // way to turn system tracing on and off.
class TraceEventSystemStatsMonitor class TraceEventSystemStatsMonitor
: public base::trace_event::TraceLog::EnabledStateObserver { : public base::trace_event::TraceLog::AsyncEnabledStateObserver,
public performance_monitor::SystemMonitor::SystemObserver {
public: public:
TraceEventSystemStatsMonitor(); TraceEventSystemStatsMonitor();
~TraceEventSystemStatsMonitor() override; ~TraceEventSystemStatsMonitor() override;
...@@ -31,10 +27,7 @@ class TraceEventSystemStatsMonitor ...@@ -31,10 +27,7 @@ class TraceEventSystemStatsMonitor
void OnTraceLogEnabled() override; void OnTraceLogEnabled() override;
void OnTraceLogDisabled() override; void OnTraceLogDisabled() override;
// Retrieves system profiling at the current time. bool is_profiling_for_testing() const { return is_profiling_; }
void DumpSystemStats();
bool IsTimerRunningForTesting() const;
void StartProfilingForTesting() { StartProfiling(); } void StartProfilingForTesting() { StartProfiling(); }
void StopProfilingForTesting() { StopProfiling(); } void StopProfilingForTesting() { StopProfiling(); }
...@@ -44,11 +37,12 @@ class TraceEventSystemStatsMonitor ...@@ -44,11 +37,12 @@ class TraceEventSystemStatsMonitor
void StopProfiling(); void StopProfiling();
// Ensures the observer starts and stops tracing on the primary thread. // performance_monitor::SystemMonitor::SystemObserver:
scoped_refptr<base::SingleThreadTaskRunner> task_runner_; void OnSystemMetricsStruct(
const base::SystemMetrics& system_metrics) override;
// Timer to schedule system profile dumps. // Indicates if profiling has started.
base::RepeatingTimer dump_timer_; bool is_profiling_ = false;
base::WeakPtrFactory<TraceEventSystemStatsMonitor> weak_factory_; base::WeakPtrFactory<TraceEventSystemStatsMonitor> weak_factory_;
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/scoped_task_environment.h" #include "base/test/scoped_task_environment.h"
#include "chrome/browser/performance_monitor/system_monitor.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace tracing { namespace tracing {
...@@ -15,24 +16,25 @@ using TraceSystemStatsMonitorTest = testing::Test; ...@@ -15,24 +16,25 @@ using TraceSystemStatsMonitorTest = testing::Test;
TEST_F(TraceSystemStatsMonitorTest, TraceEventSystemStatsMonitor) { TEST_F(TraceSystemStatsMonitorTest, TraceEventSystemStatsMonitor) {
base::test::ScopedTaskEnvironment scoped_task_environment; base::test::ScopedTaskEnvironment scoped_task_environment;
auto system_monitor = performance_monitor::SystemMonitor::Create();
TraceEventSystemStatsMonitor system_stats_monitor; TraceEventSystemStatsMonitor system_stats_monitor;
EXPECT_TRUE( EXPECT_TRUE(
base::trace_event::TraceLog::GetInstance()->HasEnabledStateObserver( base::trace_event::TraceLog::GetInstance()->HasAsyncEnabledStateObserver(
&system_stats_monitor)); &system_stats_monitor));
// By default the observer isn't dumping memory profiles. // By default the observer isn't dumping memory profiles.
EXPECT_FALSE(system_stats_monitor.IsTimerRunningForTesting()); EXPECT_FALSE(system_stats_monitor.is_profiling_for_testing());
// Simulate enabling tracing. // Simulate enabling tracing.
system_stats_monitor.StartProfilingForTesting(); system_stats_monitor.StartProfilingForTesting();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_TRUE(system_stats_monitor.IsTimerRunningForTesting()); EXPECT_TRUE(system_stats_monitor.is_profiling_for_testing());
// Simulate disabling tracing. // Simulate disabling tracing.
system_stats_monitor.StopProfilingForTesting(); system_stats_monitor.StopProfilingForTesting();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_FALSE(system_stats_monitor.IsTimerRunningForTesting()); EXPECT_FALSE(system_stats_monitor.is_profiling_for_testing());
} }
} // namespace tracing } // namespace tracing
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment