Commit 35c14b06 authored by Xida Chen's avatar Xida Chen Committed by Commit Bot

[throughput] Ignore the first sample when reporting UKM

Currently we report throughput to UKM at:
  1. 5 seconds after a sequence starts.
  2. Every 500 seconds after that as long as the sequence lasts.
The problem is that in many cases users stays on a page less than 500
seconds, and then we would only get the report for the first 5 seconds,
which can be heavily biased by the page loading time.

This CL is a temp solution to ignore the first 5 seconds, so instead
we will report throughput at:
  1. The second 5 seconds after a sequence starts.
  2. Every 500 seconds after that as long as the sequence lasts.

Bug: 1040634
Change-Id: I808b3ff56acce12c3a81e8fca5b7d472f4fcf4a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2129014
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: default avatarRobert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755463}
parent 080186fb
...@@ -16,19 +16,26 @@ constexpr unsigned kNumberOfSamplesToReport = 100u; ...@@ -16,19 +16,26 @@ constexpr unsigned kNumberOfSamplesToReport = 100u;
ThroughputUkmReporter::ThroughputUkmReporter(UkmManager* ukm_manager) ThroughputUkmReporter::ThroughputUkmReporter(UkmManager* ukm_manager)
: ukm_manager_(ukm_manager) { : ukm_manager_(ukm_manager) {
DCHECK(ukm_manager_); DCHECK(ukm_manager_);
// TODO(crbug.com/1040634): Setting it to 1 such that the first sample is
// ignored. TThis is because the universal tracker is active during the page
// load and the first sample is heavily biased by loading as a result.
samples_to_next_event_[static_cast<int>(
FrameSequenceTrackerType::kUniversal)] = 1;
} }
ThroughputUkmReporter::~ThroughputUkmReporter() = default;
void ThroughputUkmReporter::ReportThroughputUkm( void ThroughputUkmReporter::ReportThroughputUkm(
const base::Optional<int>& slower_throughput_percent, const base::Optional<int>& slower_throughput_percent,
const base::Optional<int>& impl_throughput_percent, const base::Optional<int>& impl_throughput_percent,
const base::Optional<int>& main_throughput_percent, const base::Optional<int>& main_throughput_percent,
FrameSequenceTrackerType type) { FrameSequenceTrackerType type) {
if (samples_to_next_event_ == 0) { if (samples_to_next_event_[static_cast<int>(type)] == 0) {
// Sample every 2000 events. Using the Universal tracker as an example // Sample every 100 events. Using the Universal tracker as an example
// which reports UMA every 5s, then the system collects UKM once per // which reports UMA every 5s, then the system collects UKM once per
// 2000*5 = 10000 seconds, which is about 3 hours. This number may need to // 100*5 = 500 seconds. This number may need to be tuned to not throttle
// be tuned to not throttle the UKM system. // the UKM system.
samples_to_next_event_ = kNumberOfSamplesToReport; samples_to_next_event_[static_cast<int>(type)] = kNumberOfSamplesToReport;
if (impl_throughput_percent) { if (impl_throughput_percent) {
ukm_manager_->RecordThroughputUKM( ukm_manager_->RecordThroughputUKM(
type, FrameSequenceMetrics::ThreadType::kCompositor, type, FrameSequenceMetrics::ThreadType::kCompositor,
...@@ -43,8 +50,8 @@ void ThroughputUkmReporter::ReportThroughputUkm( ...@@ -43,8 +50,8 @@ void ThroughputUkmReporter::ReportThroughputUkm(
FrameSequenceMetrics::ThreadType::kSlower, FrameSequenceMetrics::ThreadType::kSlower,
slower_throughput_percent.value()); slower_throughput_percent.value());
} }
DCHECK_GT(samples_to_next_event_, 0u); DCHECK_GT(samples_to_next_event_[static_cast<int>(type)], 0u);
samples_to_next_event_--; samples_to_next_event_[static_cast<int>(type)]--;
} }
void ThroughputUkmReporter::ReportAggregateThroughput( void ThroughputUkmReporter::ReportAggregateThroughput(
......
...@@ -23,7 +23,7 @@ enum class AggregationType { ...@@ -23,7 +23,7 @@ enum class AggregationType {
class CC_EXPORT ThroughputUkmReporter { class CC_EXPORT ThroughputUkmReporter {
public: public:
explicit ThroughputUkmReporter(UkmManager* ukm_manager); explicit ThroughputUkmReporter(UkmManager* ukm_manager);
~ThroughputUkmReporter() = default; ~ThroughputUkmReporter();
ThroughputUkmReporter(const ThroughputUkmReporter&) = delete; ThroughputUkmReporter(const ThroughputUkmReporter&) = delete;
ThroughputUkmReporter& operator=(const ThroughputUkmReporter&) = delete; ThroughputUkmReporter& operator=(const ThroughputUkmReporter&) = delete;
...@@ -40,7 +40,8 @@ class CC_EXPORT ThroughputUkmReporter { ...@@ -40,7 +40,8 @@ class CC_EXPORT ThroughputUkmReporter {
// Sampling control. We sample the event here to not throttle the UKM system. // Sampling control. We sample the event here to not throttle the UKM system.
// Currently, the same sampling rate is applied to all existing trackers. We // Currently, the same sampling rate is applied to all existing trackers. We
// might want to iterate on this based on the collected data. // might want to iterate on this based on the collected data.
uint32_t samples_to_next_event_ = 0; uint32_t samples_to_next_event_[static_cast<int>(
FrameSequenceTrackerType::kMaxType)] = {0};
uint32_t samples_for_aggregated_report_ = 0; uint32_t samples_for_aggregated_report_ = 0;
// This is pointing to the LayerTreeHostImpl::ukm_manager_, which is // This is pointing to the LayerTreeHostImpl::ukm_manager_, which is
......
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