Commit 46edf214 authored by Jonathan Ross's avatar Jonathan Ross Committed by Commit Bot

Calculate Histogram diffs rather reporting all to Traces

We currently measure Histograms in perf benchmarks. This is done by
emitting a trace of all histograms from startup. Then emitting another
at the end of tracing.

Then in Catapult we calculate differences in uma_metric.html

However this has led to issues in the past with mis-calculating buckets
of histograms. It also currently breaks down when histogram emitting
changes occur near the startup.

To help clear this, we want to not record the initial histograms. But
instead calculate the differences. Thus emitting only one sample for
Catapult to process. We will then reduce the complexity of uma_metric's
processing in a follow up change in Catapult.

Bug: 1145731

Change-Id: Ia6452984a16914aaf19e66c22d583592835f46df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2521106Reviewed-by: default avataroysteine <oysteine@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827962}
parent 9e0d0984
...@@ -958,7 +958,17 @@ void TraceEventDataSource::LogHistogram(base::HistogramBase* histogram) { ...@@ -958,7 +958,17 @@ void TraceEventDataSource::LogHistogram(base::HistogramBase* histogram) {
if (!histogram) { if (!histogram) {
return; return;
} }
// For the purpose of calculating metrics from histograms we only want the
// delta of the events.
auto samples = histogram->SnapshotSamples(); auto samples = histogram->SnapshotSamples();
// If there were HistogramSamples recorded during startup, then those should
// be subtracted from the overall set. This way we only report the samples
// that occured during the run.
auto it = startup_histogram_samples_.find(histogram->histogram_name());
if (it != startup_histogram_samples_.end()) {
samples->Subtract(*it->second.get());
}
base::Pickle pickle; base::Pickle pickle;
samples->Serialize(&pickle); samples->Serialize(&pickle);
std::string buckets; std::string buckets;
...@@ -972,9 +982,22 @@ void TraceEventDataSource::LogHistogram(base::HistogramBase* histogram) { ...@@ -972,9 +982,22 @@ void TraceEventDataSource::LogHistogram(base::HistogramBase* histogram) {
void TraceEventDataSource::ResetHistograms(const TraceConfig& trace_config) { void TraceEventDataSource::ResetHistograms(const TraceConfig& trace_config) {
histograms_.clear(); histograms_.clear();
startup_histogram_samples_.clear();
for (const std::string& histogram_name : trace_config.histogram_names()) { for (const std::string& histogram_name : trace_config.histogram_names()) {
histograms_.push_back(histogram_name); histograms_.push_back(histogram_name);
LogHistogram(base::StatisticsRecorder::FindHistogram(histogram_name)); auto* histogram = base::StatisticsRecorder::FindHistogram(histogram_name);
if (!histogram) {
continue;
}
// For the purpose of calculating metrics from histograms we only want the
// delta of the events. However we do not want to emit the results when
// resetting. This will allow LogHistogram to emit one UMAHistogramSamples
// which encompasses only the histograms recorded during the trace. We
// cache the initial HistogramSamples so that they can be subtracted from
// the full snapshot at the end.
startup_histogram_samples_.emplace(histogram_name,
histogram->SnapshotSamples());
} }
} }
......
...@@ -27,11 +27,15 @@ ...@@ -27,11 +27,15 @@
#include "third_party/perfetto/protos/perfetto/trace/chrome/chrome_trace_event.pbzero.h" #include "third_party/perfetto/protos/perfetto/trace/chrome/chrome_trace_event.pbzero.h"
namespace base { namespace base {
class HistogramSamples;
namespace trace_event { namespace trace_event {
class ThreadInstructionCount; class ThreadInstructionCount;
class TraceEvent; class TraceEvent;
struct TraceEventHandle; struct TraceEventHandle;
} // namespace trace_event } // namespace trace_event
} // namespace base } // namespace base
namespace perfetto { namespace perfetto {
...@@ -280,6 +284,11 @@ class COMPONENT_EXPORT(TRACING_CPP) TraceEventDataSource ...@@ -280,6 +284,11 @@ class COMPONENT_EXPORT(TRACING_CPP) TraceEventDataSource
bool flushing_trace_log_ = false; bool flushing_trace_log_ = false;
base::OnceClosure flush_complete_task_; base::OnceClosure flush_complete_task_;
std::vector<std::string> histograms_; std::vector<std::string> histograms_;
// For each of the Histogram that we are tracking, cache the snapshot of their
// HistogramSamples from before tracing began. So that we can calculate the
// delta when we go to LogHistograms.
std::map<std::string, std::unique_ptr<base::HistogramSamples>>
startup_histogram_samples_;
// Stores all histogram names for which OnMetricsSampleCallback was set as an // Stores all histogram names for which OnMetricsSampleCallback was set as an
// OnSampleCallback. This is done in order to avoid clearing callbacks for the // OnSampleCallback. This is done in order to avoid clearing callbacks for the
// other histograms. // other histograms.
......
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