[cc/metrics] Remove code for aggregated throughput

Now that we have deprecated the Universal throughput tracker,
the aggregated throughput is no longer useful and this CL
removes related code.

Bug: 1128993
Change-Id: I96f95a9cba1f99d8e42ccdbc1c266c41b721d08d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2421760Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809007}

cc/metrics/frame_sequence_metrics.cc

@@ -175,7 +175,6 @@ void FrameSequenceMetrics::Merge(
   DCHECK_EQ(GetEffectiveThread(), metrics->GetEffectiveThread());
   impl_throughput_.Merge(metrics->impl_throughput_);
   main_throughput_.Merge(metrics->main_throughput_);
-  aggregated_throughput_.Merge(metrics->aggregated_throughput_);
   frames_checkerboarded_ += metrics->frames_checkerboarded_;
 
   if (jank_reporter_)
@@ -185,7 +184,6 @@ void FrameSequenceMetrics::Merge(
   // up reporting the metrics.
   metrics->impl_throughput_ = {};
   metrics->main_throughput_ = {};
-  metrics->aggregated_throughput_ = {};
   metrics->frames_checkerboarded_ = 0;
 }
 
@@ -212,12 +210,8 @@ void FrameSequenceMetrics::AdvanceTrace(base::TimeTicks timestamp) {
 void FrameSequenceMetrics::ReportMetrics() {
   DCHECK_LE(impl_throughput_.frames_produced, impl_throughput_.frames_expected);
   DCHECK_LE(main_throughput_.frames_produced, main_throughput_.frames_expected);
-  DCHECK_LE(aggregated_throughput_.frames_produced,
-            aggregated_throughput_.frames_expected);
   DCHECK_LE(impl_throughput_.frames_ontime, impl_throughput_.frames_expected);
   DCHECK_LE(main_throughput_.frames_ontime, main_throughput_.frames_expected);
-  DCHECK_LE(aggregated_throughput_.frames_ontime,
-            aggregated_throughput_.frames_expected);
 
   // Terminates |trace_data_| for all types of FrameSequenceTracker.
   trace_data_.Terminate();
@@ -228,7 +222,6 @@ void FrameSequenceMetrics::ReportMetrics() {
 
     main_throughput_ = {};
     impl_throughput_ = {};
-    aggregated_throughput_ = {};
     frames_checkerboarded_ = 0;
     return;
   }
@@ -352,10 +345,8 @@ void FrameSequenceMetrics::ReportMetrics() {
   }
 
   // Reset the metrics that reach reporting threshold.
-  if (impl_throughput_.frames_expected >= kMinFramesForThroughputMetric) {
+  if (impl_throughput_.frames_expected >= kMinFramesForThroughputMetric)
     impl_throughput_ = {};
-    aggregated_throughput_ = {};
-  }
   if (main_throughput_.frames_expected >= kMinFramesForThroughputMetric)
     main_throughput_ = {};
 }

cc/metrics/frame_sequence_metrics.h

@@ -132,7 +132,6 @@ class CC_EXPORT FrameSequenceMetrics {
 
   ThroughputData& impl_throughput() { return impl_throughput_; }
   ThroughputData& main_throughput() { return main_throughput_; }
-  ThroughputData& aggregated_throughput() { return aggregated_throughput_; }
   void add_checkerboarded_frames(int64_t frames) {
     frames_checkerboarded_ += frames;
   }
@@ -175,8 +174,6 @@ class CC_EXPORT FrameSequenceMetrics {
 
   ThroughputData impl_throughput_;
   ThroughputData main_throughput_;
-  // The aggregated throughput for the main/compositor thread.
-  ThroughputData aggregated_throughput_;
 
   ThreadType scrolling_thread_ = ThreadType::kUnknown;
 

cc/metrics/frame_sequence_metrics_unittest.cc

@@ -14,22 +14,6 @@
 
 namespace cc {
 
-TEST(FrameSequenceMetricsTest, AggregatedThroughputClearedAfterReport) {
-  FrameSequenceMetrics first(FrameSequenceTrackerType::kCompositorAnimation,
-                             nullptr);
-  first.impl_throughput().frames_expected = 200u;
-  first.impl_throughput().frames_produced = 190u;
-  first.impl_throughput().frames_ontime = 120u;
-  first.aggregated_throughput().frames_expected = 170u;
-  first.aggregated_throughput().frames_produced = 150u;
-  first.aggregated_throughput().frames_ontime = 100u;
-
-  first.ReportMetrics();
-  EXPECT_EQ(first.aggregated_throughput().frames_expected, 0u);
-  EXPECT_EQ(first.aggregated_throughput().frames_produced, 0u);
-  EXPECT_EQ(first.aggregated_throughput().frames_ontime, 0u);
-}
-
 TEST(FrameSequenceMetricsTest, MergeMetrics) {
   // Create a metric with only a small number of frames. It shouldn't report any
   // metrics.

cc/metrics/frame_sequence_tracker.cc

@@ -135,8 +135,6 @@ void FrameSequenceTracker::ReportBeginImplFrame(
                          args.frame_id.sequence_number);
   impl_throughput().frames_expected +=
       begin_impl_frame_data_.previous_sequence_delta;
-  aggregated_throughput().frames_expected +=
-      begin_impl_frame_data_.previous_sequence_delta;
 #if DCHECK_IS_ON()
   ++impl_throughput().frames_received;
 #endif
@@ -189,7 +187,6 @@ void FrameSequenceTracker::ReportBeginMainFrame(
       begin_main_frame_data_.previous_sequence_delta;
   previous_begin_main_sequence_ = current_begin_main_sequence_;
   current_begin_main_sequence_ = args.frame_id.sequence_number;
-  no_damage_impl_frames_while_expecting_main_ = 0;
 }
 
 void FrameSequenceTracker::ReportMainFrameProcessed(
@@ -304,14 +301,6 @@ void FrameSequenceTracker::ReportSubmitFrame(
       main_frames_.push_back(frame_token);
       DCHECK_GE(main_throughput().frames_expected, main_frames_.size())
           << TRACKER_DCHECK_MSG;
-    } else {
-      // If we have sent a BeginMainFrame which hasn't yet been submitted, or
-      // confirmed that it has no damage (previous_sequence is set to 0), then
-      // we are currently expecting a main frame.
-      const bool expecting_main = begin_main_frame_data_.previous_sequence >
-                                  last_submitted_main_sequence_;
-      if (expecting_main)
-        expecting_main_when_submit_impl_.push_back(frame_token);
     }
   }
 
@@ -378,10 +367,6 @@ void FrameSequenceTracker::ReportFrameEnd(
       NOTREACHED() << TRACKER_DCHECK_MSG;
 #endif
     begin_impl_frame_data_.previous_sequence = 0;
-    if (!IsExpectingMainFrame())
-      --aggregated_throughput().frames_expected;
-    else
-      ++no_damage_impl_frames_while_expecting_main_;
   }
   // last_submitted_frame_ == 0 means the last impl frame has been presented.
   if (termination_status_ == TerminationStatus::kScheduledForTermination &&
@@ -508,48 +493,6 @@ void FrameSequenceTracker::ReportFramePresented(
         ++main_frames_ontime;
       }
     }
-
-    if (impl_frames_produced > 0) {
-      // If there is no main frame presented, then we need to see whether or not
-      // we are expecting main frames to be presented or not.
-      if (main_frames_produced == 0) {
-        // Only need to check the first element in the deque because the
-        // elements are in order.
-        bool expecting_main_frames =
-            !expecting_main_when_submit_impl_.empty() &&
-            !viz::FrameTokenGT(expecting_main_when_submit_impl_[0],
-                               frame_token);
-        if (expecting_main_frames) {
-          // We are expecting a main frame to be processed, the main frame
-          // should either report no-damage or be submitted to GPU. Since we
-          // don't know which case it would be, we accumulate the number of impl
-          // frames produced so that we can apply that to aggregated throughput
-          // if the main frame reports no-damage later on.
-          impl_frames_produced_while_expecting_main_ += impl_frames_produced;
-          impl_frames_ontime_while_expecting_main_ += impl_frames_ontime;
-        } else {
-          // TODO(https://crbug.com/1066455): Determine why this DCHECK is
-          // causing PageLoadMetricsBrowserTests to flake, and re-enable.
-          // DCHECK_EQ(impl_frames_produced_while_expecting_main_, 0u)
-          //    << TRACKER_DCHECK_MSG;
-          aggregated_throughput().frames_produced += impl_frames_produced;
-          impl_frames_produced_while_expecting_main_ = 0;
-          aggregated_throughput().frames_ontime += impl_frames_ontime;
-          impl_frames_ontime_while_expecting_main_ = 0;
-        }
-      } else {
-        aggregated_throughput().frames_produced += main_frames_produced;
-        impl_frames_produced_while_expecting_main_ = 0;
-        aggregated_throughput().frames_ontime += main_frames_ontime;
-        impl_frames_ontime_while_expecting_main_ = 0;
-        while (!expecting_main_when_submit_impl_.empty() &&
-               !viz::FrameTokenGT(expecting_main_when_submit_impl_.front(),
-                                  frame_token)) {
-          expecting_main_when_submit_impl_.pop_front();
-        }
-      }
-    }
-
     last_frame_presentation_timestamp_ = feedback.timestamp;
 
     if (checkerboarding_.last_frame_had_checkerboarding) {
@@ -653,16 +596,6 @@ void FrameSequenceTracker::ReportMainFrameCausedNoDamage(
       << TRACKER_DCHECK_MSG;
   last_no_main_damage_sequence_ = args.frame_id.sequence_number;
   --main_throughput().frames_expected;
-  // Compute the number of actually expected compositor frames during this main
-  // frame, which produced no damage..
-  DCHECK_GE(aggregated_throughput().frames_expected,
-            no_damage_impl_frames_while_expecting_main_)
-      << TRACKER_DCHECK_MSG;
-  aggregated_throughput().frames_expected -=
-      no_damage_impl_frames_while_expecting_main_;
-  DCHECK_GE(aggregated_throughput().frames_expected,
-            aggregated_throughput().frames_produced)
-      << TRACKER_DCHECK_MSG;
   DCHECK_GE(main_throughput().frames_expected, main_frames_.size())
       << TRACKER_DCHECK_MSG;
 
@@ -673,14 +606,6 @@ void FrameSequenceTracker::ReportMainFrameCausedNoDamage(
         << TRACKER_DCHECK_MSG;
   }
   begin_main_frame_data_.previous_sequence = 0;
-
-  aggregated_throughput().frames_produced +=
-      impl_frames_produced_while_expecting_main_;
-  impl_frames_produced_while_expecting_main_ = 0;
-  aggregated_throughput().frames_ontime +=
-      impl_frames_ontime_while_expecting_main_;
-  impl_frames_ontime_while_expecting_main_ = 0;
-  expecting_main_when_submit_impl_.clear();
 }
 
 void FrameSequenceTracker::PauseFrameProduction() {
@@ -728,13 +653,6 @@ bool FrameSequenceTracker::ShouldIgnoreSequence(
   return sequence_number != begin_impl_frame_data_.previous_sequence;
 }
 
-bool FrameSequenceTracker::IsExpectingMainFrame() const {
-  bool last_main_not_processed =
-      begin_main_frame_data_.previous_sequence != 0 &&
-      begin_main_frame_data_.previous_sequence != last_processed_main_sequence_;
-  return !main_frames_.empty() || last_main_not_processed;
-}
-
 bool FrameSequenceTracker::ShouldReportMetricsNow(
     const viz::BeginFrameArgs& args) const {
   return metrics_->HasEnoughDataForReporting() &&

cc/metrics/frame_sequence_tracker.h

@@ -125,9 +125,6 @@ class CC_EXPORT FrameSequenceTracker {
   FrameSequenceMetrics::ThroughputData& main_throughput() {
     return metrics_->main_throughput();
   }
-  FrameSequenceMetrics::ThroughputData& aggregated_throughput() {
-    return metrics_->aggregated_throughput();
-  }
 
   void ScheduleTerminate();
 
@@ -165,8 +162,6 @@ class CC_EXPORT FrameSequenceTracker {
 
   bool ShouldIgnoreSequence(uint64_t sequence_number) const;
 
-  bool IsExpectingMainFrame() const;
-
   const int custom_sequence_id_;
 
   TerminationStatus termination_status_ = TerminationStatus::kActive;
@@ -242,15 +237,6 @@ class CC_EXPORT FrameSequenceTracker {
   uint64_t last_processed_main_sequence_ = 0;
   uint64_t last_processed_main_sequence_latency_ = 0;
 
-  // Used to compute aggregated throughput.
-  // When expecting a main frame, we accumulate the number of impl frames
-  // presented because if that main frame ends up with no-damage, then we should
-  // count the impl frames that were produced in the meantime.
-  uint32_t impl_frames_produced_while_expecting_main_ = 0;
-  uint32_t impl_frames_ontime_while_expecting_main_ = 0;
-  // Each entry is a frame token, inserted at ReportSubmitFrame.
-  base::circular_deque<uint32_t> expecting_main_when_submit_impl_;
-
   // Handle off-screen main damage case. In this case, the sequence is typically
   // like: b(1)B(0,1)E(1)n(1)e(1)b(2)n(2)e(2)...b(10)E(2)B(10,10)n(10)e(10).
   // Note that between two 'E's, all the impl frames caused no damage, and
@@ -264,10 +250,6 @@ class CC_EXPORT FrameSequenceTracker {
   // only when the last impl-frame is ended (ReportFrameEnd).
   bool is_inside_frame_ = false;
 
-  // The number of no damage impl frames accumulated while expecting main. This
-  // main frame could report no damage eventually, then we need to account for
-  // that in the aggregated throughput.
-  uint32_t no_damage_impl_frames_while_expecting_main_ = 0;
 
 #if DCHECK_IS_ON()
   // This stringstream represents a sequence of frame reporting activities on