Implement an unified throughput metric

This CL make the frame sequence tracker reports an aggregated metric
for both compositor + main thread. At this moment, it reports the worse
throughput between the compositor vs main thread. Unit tests are added.

Bug: 1002994
Change-Id: I777c68ea04929e287a91cf82160818be34e36f45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1801921Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699585}

cc/metrics/frame_sequence_tracker.cc

@@ -37,16 +37,19 @@ constexpr int kMinFramesForThroughputMetric = 4;
 enum class ThreadType {
   kMain,
   kCompositor,
+  kSlowerThread,
 };
 
 constexpr int kBuiltinSequenceNum =
     base::size(FrameSequenceTracker::kFrameSequenceTrackerTypeNames);
-constexpr int kMaximumHistogramIndex = 2 * kBuiltinSequenceNum;
+constexpr int kMaximumHistogramIndex = 3 * kBuiltinSequenceNum;
 
 int GetIndexForMetric(ThreadType thread_type, FrameSequenceTrackerType type) {
-  return thread_type == ThreadType::kMain
-             ? static_cast<int>(type)
-             : static_cast<int>(type + kBuiltinSequenceNum);
+  if (thread_type == ThreadType::kMain)
+    return static_cast<int>(type);
+  if (thread_type == ThreadType::kCompositor)
+    return static_cast<int>(type + kBuiltinSequenceNum);
+  return static_cast<int>(type + 2 * kBuiltinSequenceNum);
 }
 
 std::string GetCheckerboardingHistogramName(FrameSequenceTrackerType type) {
@@ -208,14 +211,35 @@ FrameSequenceTracker::~FrameSequenceTracker() {
       "cc,benchmark", "FrameSequenceTracker", this, "args",
       ThroughputData::ToTracedValue(impl_throughput_, main_throughput_),
       "checkerboard", checkerboarding_.frames_checkerboarded);
+  ReportMetrics();
+}
 
+void FrameSequenceTracker::ReportMetrics() {
   // Report the throughput metrics.
-  ThroughputData::ReportHistogram(
+  base::Optional<int> impl_throughput_percent = ThroughputData::ReportHistogram(
       type_, "CompositorThread",
       GetIndexForMetric(ThreadType::kCompositor, type_), impl_throughput_);
-  ThroughputData::ReportHistogram(type_, "MainThread",
-                                  GetIndexForMetric(ThreadType::kMain, type_),
-                                  main_throughput_);
+  base::Optional<int> main_throughput_percent = ThroughputData::ReportHistogram(
+      type_, "MainThread", GetIndexForMetric(ThreadType::kMain, type_),
+      main_throughput_);
+
+  base::Optional<ThroughputData> slower_throughput;
+  if (impl_throughput_percent &&
+      (!main_throughput_percent ||
+       impl_throughput_percent.value() < main_throughput_percent.value())) {
+    slower_throughput = impl_throughput_;
+  }
+  if (main_throughput_percent &&
+      (!impl_throughput_percent ||
+       main_throughput_percent.value() < impl_throughput_percent.value())) {
+    slower_throughput = main_throughput_;
+  }
+  if (slower_throughput.has_value()) {
+    ThroughputData::ReportHistogram(
+        type_, "SlowerThread",
+        GetIndexForMetric(ThreadType::kSlowerThread, type_),
+        slower_throughput.value());
+  }
 
   // Report the checkerboarding metrics.
   if (impl_throughput_.frames_expected >= kMinFramesForThroughputMetric) {
@@ -462,7 +486,7 @@ FrameSequenceTracker::ThroughputData::ToTracedValue(
   return dict;
 }
 
-void FrameSequenceTracker::ThroughputData::ReportHistogram(
+base::Optional<int> FrameSequenceTracker::ThroughputData::ReportHistogram(
     FrameSequenceTrackerType sequence_type,
     const char* thread_name,
     int metric_index,
@@ -477,7 +501,7 @@ void FrameSequenceTracker::ThroughputData::ReportHistogram(
           base::HistogramBase::kUmaTargetedHistogramFlag));
 
   if (data.frames_expected < kMinFramesForThroughputMetric)
-    return;
+    return base::nullopt;
 
   const int percent =
       static_cast<int>(100 * data.frames_produced / data.frames_expected);
@@ -487,6 +511,7 @@ void FrameSequenceTracker::ThroughputData::ReportHistogram(
       base::LinearHistogram::FactoryGet(
           GetThroughputHistogramName(sequence_type, thread_name), 1, 100, 101,
           base::HistogramBase::kUmaTargetedHistogramFlag));
+  return percent;
 }
 
 FrameSequenceTracker::CheckerboardingData::CheckerboardingData() = default;

cc/metrics/frame_sequence_tracker.h

@@ -15,6 +15,7 @@
 #include "base/containers/circular_deque.h"
 #include "base/containers/flat_map.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/trace_event/traced_value.h"
 #include "cc/cc_export.h"
 
@@ -193,10 +194,13 @@ class CC_EXPORT FrameSequenceTracker {
     static std::unique_ptr<base::trace_event::TracedValue> ToTracedValue(
         const ThroughputData& impl,
         const ThroughputData& main);
-    static void ReportHistogram(FrameSequenceTrackerType sequence_type,
-                                const char* thread_name,
-                                int metric_index,
-                                const ThroughputData& data);
+    // Returns the throughput in percent, a return value of base::nullopt
+    // indicates that no throughput metric is reported.
+    static base::Optional<int> ReportHistogram(
+        FrameSequenceTrackerType sequence_type,
+        const char* thread_name,
+        int metric_index,
+        const ThroughputData& data);
     // Tracks the number of frames that were expected to be shown during this
     // frame-sequence.
     uint32_t frames_expected = 0;
@@ -230,6 +234,9 @@ class CC_EXPORT FrameSequenceTracker {
 
   bool ShouldIgnoreBeginFrameSource(uint64_t source_id) const;
 
+  // Report related metrics: throughput, checkboarding...
+  void ReportMetrics();
+
   const FrameSequenceTrackerType type_;
 
   TerminationStatus termination_status_ = TerminationStatus::kActive;

cc/metrics/frame_sequence_tracker_unittest.cc

@@ -7,6 +7,7 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "cc/metrics/compositor_frame_reporting_controller.h"
 #include "components/viz/common/frame_sinks/begin_frame_args.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -111,6 +112,45 @@ class FrameSequenceTrackerTest : public testing::Test {
     EXPECT_TRUE(collection_.removal_trackers_.empty());
   }
 
+  void ReportMetrics() {
+    base::HistogramTester histogram_tester;
+
+    // Test that there is no main thread frames expected.
+    tracker_->impl_throughput_.frames_expected = 100u;
+    tracker_->impl_throughput_.frames_produced = 85u;
+    tracker_->ReportMetrics();
+    histogram_tester.ExpectTotalCount(
+        "Graphics.Smoothness.Throughput.CompositorThread.TouchScroll", 1u);
+    histogram_tester.ExpectTotalCount(
+        "Graphics.Smoothness.Throughput.MainThread.TouchScroll", 0u);
+    histogram_tester.ExpectTotalCount(
+        "Graphics.Smoothness.Throughput.SlowerThread.TouchScroll", 1u);
+
+    // Test that both are reported.
+    tracker_->main_throughput_.frames_expected = 50u;
+    tracker_->main_throughput_.frames_produced = 25u;
+    tracker_->ReportMetrics();
+    histogram_tester.ExpectTotalCount(
+        "Graphics.Smoothness.Throughput.CompositorThread.TouchScroll", 2u);
+    histogram_tester.ExpectTotalCount(
+        "Graphics.Smoothness.Throughput.MainThread.TouchScroll", 1u);
+    histogram_tester.ExpectTotalCount(
+        "Graphics.Smoothness.Throughput.SlowerThread.TouchScroll", 2u);
+
+    // Test that none is reported.
+    tracker_->main_throughput_.frames_expected = 2u;
+    tracker_->main_throughput_.frames_produced = 1u;
+    tracker_->impl_throughput_.frames_expected = 2u;
+    tracker_->impl_throughput_.frames_produced = 1u;
+    tracker_->ReportMetrics();
+    histogram_tester.ExpectTotalCount(
+        "Graphics.Smoothness.Throughput.CompositorThread.TouchScroll", 2u);
+    histogram_tester.ExpectTotalCount(
+        "Graphics.Smoothness.Throughput.MainThread.TouchScroll", 1u);
+    histogram_tester.ExpectTotalCount(
+        "Graphics.Smoothness.Throughput.SlowerThread.TouchScroll", 2u);
+  }
+
  protected:
   uint32_t number_of_frames_checkerboarded() const {
     return tracker_->checkerboarding_.frames_checkerboarded;
@@ -274,4 +314,8 @@ TEST_F(FrameSequenceTrackerTest, MultipleCheckerboardingFrames) {
   EXPECT_EQ(kFrames, number_of_frames_checkerboarded());
 }
 
+TEST_F(FrameSequenceTrackerTest, ReportMetrics) {
+  ReportMetrics();
+}
+
 }  // namespace cc

tools/metrics/histograms/histograms.xml

@@ -176461,6 +176461,7 @@ regressions. -->
   <affected-histogram name="Graphics.Smoothness.FrameSequenceLength"/>
   <affected-histogram name="Graphics.Smoothness.Throughput.CompositorThread"/>
   <affected-histogram name="Graphics.Smoothness.Throughput.MainThread"/>
+  <affected-histogram name="Graphics.Smoothness.Throughput.SlowerThread"/>
   <affected-histogram name="SingleThreadedCompositorLatency"/>
   <affected-histogram name="SingleThreadedCompositorLatency.MissedFrame"/>
   <affected-histogram
@@ -176475,6 +176476,8 @@ regressions. -->
   <suffix name="CompositorThread"
       label="The throughput of the compositor thread"/>
   <suffix name="MainThread" label="The throughput of the main thread"/>
+  <suffix name="SlowerThread"
+      label="The worse throughput of the main and the compositor thread"/>
   <affected-histogram name="Graphics.Smoothness.Throughput"/>
 </histogram_suffixes>