Skip to content

Commit

Permalink
[Throughput UKM] Correct sampling logic
Browse files Browse the repository at this point in the history
We have identified that the reason of not getting throughput UKM is
due to the sampling logic, via this debug CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1946872

This CL changes the sampling logic.

Bug: 1029964
Change-Id: Icf5fc750102d0e3631047913d77adb4e37589721
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1958195
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723642}
  • Loading branch information
xidachen authored and Commit Bot committed Dec 11, 2019
1 parent cbbe680 commit f6bbaca
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
15 changes: 5 additions & 10 deletions cc/metrics/throughput_ukm_reporter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace cc {

namespace {
// Collect UKM once per kNumberOfSamplesToReport UMA reports.
constexpr unsigned kNumberOfSamplesToReport = 2000u;
constexpr unsigned kNumberOfSamplesToReport = 100u;
} // namespace

void ThroughputUkmReporter::ReportThroughputUkm(
Expand All @@ -19,17 +19,12 @@ void ThroughputUkmReporter::ReportThroughputUkm(
const base::Optional<int>& impl_throughput_percent,
const base::Optional<int>& main_throughput_percent,
FrameSequenceTrackerType type) {
// 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
// might want to iterate on this based on the collected data.
static uint32_t samples_to_next_event = 0;

if (samples_to_next_event == 0) {
if (samples_to_next_event_ == 0) {
// Sample every 2000 events. Using the Universal tracker as an example
// 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
// be tuned to not throttle the UKM system.
samples_to_next_event = kNumberOfSamplesToReport;
samples_to_next_event_ = kNumberOfSamplesToReport;
if (impl_throughput_percent) {
ukm_manager->RecordThroughputUKM(
type, FrameSequenceTracker::ThreadType::kCompositor,
Expand All @@ -44,8 +39,8 @@ void ThroughputUkmReporter::ReportThroughputUkm(
FrameSequenceTracker::ThreadType::kSlower,
slower_throughput_percent.value());
}
DCHECK_GT(samples_to_next_event, 0u);
samples_to_next_event--;
DCHECK_GT(samples_to_next_event_, 0u);
samples_to_next_event_--;
}

} // namespace cc
6 changes: 6 additions & 0 deletions cc/metrics/throughput_ukm_reporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ class CC_EXPORT ThroughputUkmReporter {
const base::Optional<int>& impl_throughput_percent,
const base::Optional<int>& main_throughput_percent,
FrameSequenceTrackerType type);

private:
// 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
// might want to iterate on this based on the collected data.
uint32_t samples_to_next_event_ = 0;
};

} // namespace cc
Expand Down

0 comments on commit f6bbaca

Please sign in to comment.