Skip to content

Commit

Permalink
Remove PercentDroppedFrames v1
Browse files Browse the repository at this point in the history 
The v1 of this metric is no longer needed. ChromeOS has begun using v3.
Experiments relying on it have wrapped up, or moved on to v3.

With v3 being a guardrail metric this is no longer needed.

Also remove the UKMs for this. We currently do not plan to do further
analysis here. Nor do we have plans to add v3.

Bug: 1405963
Change-Id: Idffacdfb15e3a06237dd88e86c3524e8ac31f00d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4210171
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Mingjing Zhang <mjzhang@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1103450}
  • Loading branch information
Jonathan Ross authored and Chromium LUCI CQ committed Feb 9, 2023
1 parent 4f837fe commit 41b7b34
Show file tree
Hide file tree
Showing 21 changed files with 23 additions and 1,355 deletions.
2 changes: 0 additions & 2 deletions cc/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,6 @@ cc_component("cc") {
"metrics/lcd_text_metrics_reporter.cc",
"metrics/lcd_text_metrics_reporter.h",
"metrics/shared_metrics_buffer.h",
"metrics/throughput_ukm_reporter.cc",
"metrics/throughput_ukm_reporter.h",
"metrics/total_frame_counter.cc",
"metrics/total_frame_counter.h",
"metrics/ukm_smoothness_data.cc",
Expand Down
119 changes: 3 additions & 116 deletions cc/metrics/frame_sequence_metrics.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "base/trace_event/traced_value.h"
#include "cc/metrics/frame_sequence_tracker.h"
#include "cc/metrics/jank_metrics.h"
#include "cc/metrics/throughput_ukm_reporter.h"
#include "components/viz/common/frame_sinks/begin_frame_args.h"

namespace cc {
Expand Down Expand Up @@ -81,13 +80,6 @@ std::string GetCheckerboardingHistogramName(FrameSequenceTrackerType type) {
FrameSequenceTracker::GetFrameSequenceTrackerTypeName(type)});
}

std::string GetThroughputHistogramName(FrameSequenceTrackerType type,
const char* thread_name) {
return base::StrCat(
{"Graphics.Smoothness.PercentDroppedFrames.", thread_name, ".",
FrameSequenceTracker::GetFrameSequenceTrackerTypeName(type)});
}

std::string GetThroughputV3HistogramName(FrameSequenceTrackerType type,
const char* thread_name) {
return base::StrCat(
Expand All @@ -111,9 +103,8 @@ bool IsInteractionType(FrameSequenceTrackerType sequence_type) {

} // namespace

FrameSequenceMetrics::FrameSequenceMetrics(FrameSequenceTrackerType type,
ThroughputUkmReporter* ukm_reporter)
: type_(type), throughput_ukm_reporter_(ukm_reporter) {
FrameSequenceMetrics::FrameSequenceMetrics(FrameSequenceTrackerType type)
: type_(type) {
SmoothEffectDrivingThread thread_type = GetEffectiveThread();

// Only construct |jank_reporter_| if it has a valid tracker and thread type.
Expand Down Expand Up @@ -310,30 +301,18 @@ void FrameSequenceMetrics::ReportMetrics() {
v3_ = {};
}

absl::optional<int> impl_throughput_percent_dropped;
absl::optional<int> impl_throughput_percent_missed;
absl::optional<int> main_throughput_percent_dropped;
absl::optional<int> main_throughput_percent_missed;

// Report the throughput metrics.
if (compositor_report) {
impl_throughput_percent_dropped =
ThroughputData::ReportDroppedFramePercentHistogram(
this, SmoothEffectDrivingThread::kCompositor,
GetIndexForMetric(SmoothEffectDrivingThread::kCompositor, type_),
impl_throughput_);
impl_throughput_percent_missed =
ThroughputData::ReportMissedDeadlineFramePercentHistogram(
this, SmoothEffectDrivingThread::kCompositor,
GetIndexForMetric(SmoothEffectDrivingThread::kCompositor, type_),
impl_throughput_);
}
if (main_report) {
main_throughput_percent_dropped =
ThroughputData::ReportDroppedFramePercentHistogram(
this, SmoothEffectDrivingThread::kMain,
GetIndexForMetric(SmoothEffectDrivingThread::kMain, type_),
main_throughput_);
main_throughput_percent_missed =
ThroughputData::ReportMissedDeadlineFramePercentHistogram(
this, SmoothEffectDrivingThread::kMain,
Expand All @@ -343,50 +322,31 @@ void FrameSequenceMetrics::ReportMetrics() {

// Report for the 'scrolling thread' for the scrolling interactions.
if (scrolling_thread_ != SmoothEffectDrivingThread::kUnknown) {
absl::optional<int> scrolling_thread_throughput_dropped;
absl::optional<int> scrolling_thread_throughput_missed;
switch (scrolling_thread_) {
case SmoothEffectDrivingThread::kCompositor:
scrolling_thread_throughput_dropped = impl_throughput_percent_dropped;
scrolling_thread_throughput_missed = impl_throughput_percent_missed;
break;
case SmoothEffectDrivingThread::kMain:
scrolling_thread_throughput_dropped = main_throughput_percent_dropped;
scrolling_thread_throughput_missed = main_throughput_percent_missed;
break;
case SmoothEffectDrivingThread::kUnknown:
NOTREACHED();
break;
}
// It's OK to use the UMA histogram in the following code while still
// using |GetThroughputHistogramName()| to get the name of the metric,
// since the input-params to the function never change at runtime.
if (scrolling_thread_throughput_dropped.has_value() &&
scrolling_thread_throughput_missed.has_value()) {
if (scrolling_thread_throughput_missed.has_value()) {
if (type_ == FrameSequenceTrackerType::kWheelScroll) {
UMA_HISTOGRAM_PERCENTAGE(
GetThroughputHistogramName(FrameSequenceTrackerType::kWheelScroll,
"ScrollingThread"),
scrolling_thread_throughput_dropped.value());
UMA_HISTOGRAM_PERCENTAGE(
GetMissedDeadlineHistogramName(
FrameSequenceTrackerType::kWheelScroll, "ScrollingThread"),
scrolling_thread_throughput_missed.value());
} else if (type_ == FrameSequenceTrackerType::kTouchScroll) {
UMA_HISTOGRAM_PERCENTAGE(
GetThroughputHistogramName(FrameSequenceTrackerType::kTouchScroll,
"ScrollingThread"),
scrolling_thread_throughput_dropped.value());
UMA_HISTOGRAM_PERCENTAGE(
GetMissedDeadlineHistogramName(
FrameSequenceTrackerType::kTouchScroll, "ScrollingThread"),
scrolling_thread_throughput_missed.value());
} else {
DCHECK_EQ(type_, FrameSequenceTrackerType::kScrollbarScroll);
UMA_HISTOGRAM_PERCENTAGE(
GetThroughputHistogramName(
FrameSequenceTrackerType::kScrollbarScroll, "ScrollingThread"),
scrolling_thread_throughput_dropped.value());
UMA_HISTOGRAM_PERCENTAGE(
GetMissedDeadlineHistogramName(
FrameSequenceTrackerType::kScrollbarScroll, "ScrollingThread"),
Expand Down Expand Up @@ -502,79 +462,6 @@ bool FrameSequenceMetrics::ThroughputData::CanReportHistogram(
sequence_type == FrameSequenceTrackerType::kSETCompositorAnimation;
}

int FrameSequenceMetrics::ThroughputData::ReportDroppedFramePercentHistogram(
FrameSequenceMetrics* metrics,
SmoothEffectDrivingThread thread_type,
int metric_index,
const ThroughputData& data) {
const auto sequence_type = metrics->type();
DCHECK_LT(sequence_type, FrameSequenceTrackerType::kMaxType);
DCHECK(CanReportHistogram(metrics, thread_type, data));

// Throughput means the percent of frames that was expected to show on the
// screen but didn't. In other words, the lower the throughput is, the
// smoother user experience.
const int percent = data.DroppedFramePercent();

const bool is_animation =
ShouldReportForAnimation(sequence_type, thread_type);
const bool is_interaction = ShouldReportForInteraction(
metrics->type(), thread_type, metrics->GetEffectiveThread());

ThroughputUkmReporter* const ukm_reporter = metrics->ukm_reporter();

if (is_animation) {
TRACE_EVENT_INSTANT2("cc,benchmark", "PercentDroppedFrames.AllAnimations",
TRACE_EVENT_SCOPE_THREAD, "frames_expected",
data.frames_expected, "frames_produced",
data.frames_produced);

UMA_HISTOGRAM_PERCENTAGE(
"Graphics.Smoothness.PercentDroppedFrames.AllAnimations", percent);
if (ukm_reporter) {
ukm_reporter->ReportAggregateThroughput(AggregationType::kAllAnimations,
percent);
}
}

if (is_interaction) {
TRACE_EVENT_INSTANT2("cc,benchmark", "PercentDroppedFrames.AllInteractions",
TRACE_EVENT_SCOPE_THREAD, "frames_expected",
data.frames_expected, "frames_produced",
data.frames_produced);
UMA_HISTOGRAM_PERCENTAGE(
"Graphics.Smoothness.PercentDroppedFrames.AllInteractions", percent);
if (ukm_reporter) {
ukm_reporter->ReportAggregateThroughput(AggregationType::kAllInteractions,
percent);
}
}

if (is_animation || is_interaction) {
TRACE_EVENT_INSTANT2("cc,benchmark", "PercentDroppedFrames.AllSequences",
TRACE_EVENT_SCOPE_THREAD, "frames_expected",
data.frames_expected, "frames_produced",
data.frames_produced);
UMA_HISTOGRAM_PERCENTAGE(
"Graphics.Smoothness.PercentDroppedFrames.AllSequences", percent);
if (ukm_reporter) {
ukm_reporter->ReportAggregateThroughput(AggregationType::kAllSequences,
percent);
}
}

const char* thread_name =
thread_type == SmoothEffectDrivingThread::kCompositor ? "CompositorThread"
: "MainThread";
STATIC_HISTOGRAM_POINTER_GROUP(
GetThroughputHistogramName(sequence_type, thread_name), metric_index,
kMaximumHistogramIndex, Add(percent),
base::LinearHistogram::FactoryGet(
GetThroughputHistogramName(sequence_type, thread_name), 1, 100, 101,
base::HistogramBase::kUmaTargetedHistogramFlag));
return percent;
}

int FrameSequenceMetrics::ThroughputData::
ReportMissedDeadlineFramePercentHistogram(
FrameSequenceMetrics* metrics,
Expand Down
38 changes: 1 addition & 37 deletions cc/metrics/frame_sequence_metrics.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ struct BeginFrameArgs;
} // namespace viz

namespace cc {
class ThroughputUkmReporter;
class JankMetrics;
struct FrameInfo;

Expand Down Expand Up @@ -74,8 +73,7 @@ inline bool HasCompositorThreadAnimation(const ActiveTrackers& trackers) {

class CC_EXPORT FrameSequenceMetrics {
public:
FrameSequenceMetrics(FrameSequenceTrackerType type,
ThroughputUkmReporter* ukm_reporter);
explicit FrameSequenceMetrics(FrameSequenceTrackerType type);
~FrameSequenceMetrics();

FrameSequenceMetrics(const FrameSequenceMetrics&) = delete;
Expand All @@ -92,13 +90,6 @@ class CC_EXPORT FrameSequenceMetrics {
FrameInfo::SmoothEffectDrivingThread thread_type,
const ThroughputData& data);

// Returns the dropped throughput in percent
static int ReportDroppedFramePercentHistogram(
FrameSequenceMetrics* metrics,
FrameInfo::SmoothEffectDrivingThread thread_type,
int metric_index,
const ThroughputData& data);

// Returns the missed deadline throughput in percent
static int ReportMissedDeadlineFramePercentHistogram(
FrameSequenceMetrics* metrics,
Expand All @@ -115,17 +106,6 @@ class CC_EXPORT FrameSequenceMetrics {
frames_expected += data.frames_expected;
frames_produced += data.frames_produced;
frames_ontime += data.frames_ontime;
#if DCHECK_IS_ON()
frames_processed += data.frames_processed;
frames_received += data.frames_received;
#endif
}

int DroppedFramePercent() const {
if (frames_expected == 0)
return 0;
return std::ceil(100 * (frames_expected - frames_produced) /
static_cast<double>(frames_expected));
}

int MissedDeadlineFramePercent() const {
Expand All @@ -146,15 +126,6 @@ class CC_EXPORT FrameSequenceMetrics {
// Tracks the number of frames that were actually presented to the user
// that didn't miss the vsync deadline during this frame-sequence.
uint32_t frames_ontime = 0;

#if DCHECK_IS_ON()
// Tracks the number of frames that is either submitted or reported as no
// damage.
uint32_t frames_processed = 0;

// Tracks the number of begin-frames that are received.
uint32_t frames_received = 0;
#endif
};

void SetScrollingThread(FrameInfo::SmoothEffectDrivingThread thread);
Expand Down Expand Up @@ -189,9 +160,6 @@ class CC_EXPORT FrameSequenceMetrics {
uint32_t frames_checkerboarded() const { return frames_checkerboarded_; }

FrameSequenceTrackerType type() const { return type_; }
ThroughputUkmReporter* ukm_reporter() const {
return throughput_ukm_reporter_;
}

// Must be called before destructor.
void ReportLeftoverData();
Expand Down Expand Up @@ -233,10 +201,6 @@ class CC_EXPORT FrameSequenceMetrics {
void Terminate();
} trace_data_{this};

// Pointer to the reporter owned by the FrameSequenceTrackerCollection.
const raw_ptr<ThroughputUkmReporter, DanglingUntriaged>
throughput_ukm_reporter_;

// Track state for measuring the PercentDroppedFrames v3 metrics.
struct {
uint32_t frames_expected = 0;
Expand Down

0 comments on commit 41b7b34

Please sign in to comment.