[cc/metrics] Track dropped-frames affecting smoothness separately.

Not all dropped-frames affect smoothness. For example, the compositor
thread may be driving a css animation, while the main-thread drops
frames while doing busy js work (e.g. handling event, etc.). These
dropped frames do not affectthe smoothness of the animation. So count
the dropped frames that specifically affect smoothness separately. To do
this, for each frame, track whether the compositor and/or the main
thread are affecting smoothness.

BUG=1115376

Change-Id: I6cd3e3d2a10ed63c53ab929386abae1d5e29e7bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2366039
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802100}

cc/BUILD.gn

@@ -679,6 +679,7 @@ cc_test("cc_unittests") {
     "metrics/compositor_frame_reporter_unittest.cc",
     "metrics/compositor_frame_reporting_controller_unittest.cc",
     "metrics/compositor_timing_history_unittest.cc",
+    "metrics/dropped_frame_counter_unittest.cc",
     "metrics/events_metrics_manager_unittest.cc",
     "metrics/frame_sequence_metrics_unittest.cc",
     "metrics/frame_sequence_tracker_unittest.cc",

cc/metrics/compositor_frame_reporter.cc

@@ -300,11 +300,13 @@ CompositorFrameReporter::CompositorFrameReporter(
     const ActiveTrackers& active_trackers,
     const viz::BeginFrameArgs& args,
     LatencyUkmReporter* latency_ukm_reporter,
-    bool should_report_metrics)
+    bool should_report_metrics,
+    SmoothThread smooth_thread)
     : should_report_metrics_(should_report_metrics),
       args_(args),
       active_trackers_(active_trackers),
-      latency_ukm_reporter_(latency_ukm_reporter) {}
+      latency_ukm_reporter_(latency_ukm_reporter),
+      smooth_thread_(smooth_thread) {}
 
 std::unique_ptr<CompositorFrameReporter>
 CompositorFrameReporter::CopyReporterAtBeginImplStage() const {
@@ -315,7 +317,8 @@ CompositorFrameReporter::CopyReporterAtBeginImplStage() const {
     return nullptr;
   }
   auto new_reporter = std::make_unique<CompositorFrameReporter>(
-      active_trackers_, args_, latency_ukm_reporter_, should_report_metrics_);
+      active_trackers_, args_, latency_ukm_reporter_, should_report_metrics_,
+      smooth_thread_);
   new_reporter->did_finish_impl_frame_ = did_finish_impl_frame_;
   new_reporter->impl_frame_finish_time_ = impl_frame_finish_time_;
   new_reporter->main_frame_abort_time_ = main_frame_abort_time_;
@@ -484,6 +487,9 @@ void CompositorFrameReporter::TerminateReporter() {
       else
         dropped_frame_counter_->AddGoodFrame();
     }
+
+    if (IsDroppedFrameAffectingSmoothness())
+      dropped_frame_counter_->AddDroppedFrameAffectingSmoothness();
   }
 }
 
@@ -1033,4 +1039,25 @@ base::TimeTicks CompositorFrameReporter::Now() const {
   return tick_clock_->NowTicks();
 }
 
+bool CompositorFrameReporter::IsDroppedFrameAffectingSmoothness() const {
+  // If the frame was not shown, then it hurt smoothness only if either of the
+  // threads is affecting smoothness (e.g. running an animation, scroll, pinch,
+  // etc.).
+  if (TestReportType(FrameReportType::kDroppedFrame)) {
+    return smooth_thread_ != SmoothThread::kSmoothNone;
+  }
+
+  // If the frame was shown, but included only partial updates, then it hurt
+  // smoothness only if the main-thread is affecting smoothness (e.g. running an
+  // animation, or scroll etc.).
+  if (has_partial_update_) {
+    return smooth_thread_ == SmoothThread::kSmoothMain ||
+           smooth_thread_ == SmoothThread::kSmoothBoth;
+  }
+
+  // If the frame was shown, and did not include partial updates, then this
+  // frame did not hurt smoothness.
+  return false;
+}
+
 }  // namespace cc

cc/metrics/compositor_frame_reporter.h

@@ -7,13 +7,13 @@
 
 #include <bitset>
 #include <memory>
+#include <string>
 #include <utility>
 #include <vector>
 
 #include "base/optional.h"
 #include "base/time/default_tick_clock.h"
 #include "base/time/time.h"
-#include "cc/base/base_export.h"
 #include "cc/cc_export.h"
 #include "cc/metrics/begin_main_frame_metrics.h"
 #include "cc/metrics/event_metrics.h"
@@ -133,13 +133,21 @@ class CC_EXPORT CompositorFrameReporter {
     ~StageData();
   };
 
+  enum SmoothThread {
+    kSmoothNone,
+    kSmoothCompositor,
+    kSmoothMain,
+    kSmoothBoth
+  };
+
   using ActiveTrackers =
       std::bitset<static_cast<size_t>(FrameSequenceTrackerType::kMaxType)>;
 
   CompositorFrameReporter(const ActiveTrackers& active_trackers,
                           const viz::BeginFrameArgs& args,
                           LatencyUkmReporter* latency_ukm_reporter,
-                          bool should_report_metrics);
+                          bool should_report_metrics,
+                          SmoothThread smooth_thread);
   ~CompositorFrameReporter();
 
   CompositorFrameReporter(const CompositorFrameReporter& reporter) = delete;
@@ -245,6 +253,8 @@ class CC_EXPORT CompositorFrameReporter {
 
   base::TimeTicks Now() const;
 
+  bool IsDroppedFrameAffectingSmoothness() const;
+
   const bool should_report_metrics_;
   const viz::BeginFrameArgs args_;
 
@@ -296,6 +306,8 @@ class CC_EXPORT CompositorFrameReporter {
 
   DroppedFrameCounter* dropped_frame_counter_ = nullptr;
   bool has_partial_update_ = false;
+
+  const SmoothThread smooth_thread_;
 };
 
 }  // namespace cc

cc/metrics/compositor_frame_reporter_unittest.cc

@@ -32,7 +32,8 @@ class CompositorFrameReporterTest : public testing::Test {
             CompositorFrameReporter::ActiveTrackers(),
             viz::BeginFrameArgs(),
             nullptr,
-            /*should_report_metrics=*/true)) {
+            /*should_report_metrics=*/true,
+            CompositorFrameReporter::SmoothThread::kSmoothBoth)) {
     pipeline_reporter_->set_tick_clock(&test_tick_clock_);
     AdvanceNowByMs(1);
   }

cc/metrics/compositor_frame_reporting_controller.cc

@@ -13,6 +13,7 @@
 
 namespace cc {
 namespace {
+using SmoothThread = CompositorFrameReporter::SmoothThread;
 using StageType = CompositorFrameReporter::StageType;
 using FrameTerminationStatus = CompositorFrameReporter::FrameTerminationStatus;
 }  // namespace
@@ -69,7 +70,7 @@ void CompositorFrameReportingController::WillBeginImplFrame(
   }
   auto reporter = std::make_unique<CompositorFrameReporter>(
       active_trackers_, args, latency_ukm_reporter_.get(),
-      should_report_metrics_);
+      should_report_metrics_, GetSmoothThread());
   reporter->set_tick_clock(tick_clock_);
   reporter->StartStage(StageType::kBeginImplFrameToSendBeginMainFrame,
                        begin_time);
@@ -96,7 +97,7 @@ void CompositorFrameReportingController::WillBeginMainFrame(
     // deadline yet). So will start a new reporter at BeginMainFrame.
     auto reporter = std::make_unique<CompositorFrameReporter>(
         active_trackers_, args, latency_ukm_reporter_.get(),
-        should_report_metrics_);
+        should_report_metrics_, GetSmoothThread());
     reporter->set_tick_clock(tick_clock_);
     reporter->StartStage(StageType::kSendBeginMainFrameToCommit, Now());
     reporter->SetDroppedFrameCounter(dropped_frame_counter_);
@@ -347,6 +348,17 @@ void CompositorFrameReportingController::RemoveActiveTracker(
   active_trackers_.reset(static_cast<size_t>(type));
 }
 
+void CompositorFrameReportingController::SetThreadAffectsSmoothness(
+    FrameSequenceMetrics::ThreadType thread_type,
+    bool affects_smoothness) {
+  if (thread_type == FrameSequenceMetrics::ThreadType::kCompositor) {
+    is_compositor_thread_driving_smoothness_ = affects_smoothness;
+  } else {
+    DCHECK_EQ(thread_type, FrameSequenceMetrics::ThreadType::kMain);
+    is_main_thread_driving_smoothness_ = affects_smoothness;
+  }
+}
+
 void CompositorFrameReportingController::AdvanceReporterStage(
     PipelineStage start,
     PipelineStage target) {
@@ -401,4 +413,16 @@ void CompositorFrameReportingController::SetUkmManager(UkmManager* manager) {
   latency_ukm_reporter_->set_ukm_manager(manager);
 }
 
+CompositorFrameReporter::SmoothThread
+CompositorFrameReportingController::GetSmoothThread() const {
+  if (is_main_thread_driving_smoothness_) {
+    return is_compositor_thread_driving_smoothness_ ? SmoothThread::kSmoothBoth
+                                                    : SmoothThread::kSmoothMain;
+  }
+
+  return is_compositor_thread_driving_smoothness_
+             ? SmoothThread::kSmoothCompositor
+             : SmoothThread::kSmoothNone;
+}
+
 }  // namespace cc

cc/metrics/compositor_frame_reporting_controller.h

@@ -76,8 +76,11 @@ class CC_EXPORT CompositorFrameReportingController {
 
   void SetUkmManager(UkmManager* manager);
 
-  virtual void AddActiveTracker(FrameSequenceTrackerType type);
-  virtual void RemoveActiveTracker(FrameSequenceTrackerType type);
+  void AddActiveTracker(FrameSequenceTrackerType type);
+  void RemoveActiveTracker(FrameSequenceTrackerType type);
+
+  void SetThreadAffectsSmoothness(FrameSequenceMetrics::ThreadType thread_type,
+                                  bool affects_smoothness);
 
   void set_tick_clock(const base::TickClock* tick_clock) {
     DCHECK(tick_clock);
@@ -113,6 +116,7 @@ class CC_EXPORT CompositorFrameReportingController {
   bool CanSubmitMainFrame(const viz::BeginFrameId& id) const;
   std::unique_ptr<CompositorFrameReporter> RestoreReporterAtBeginImpl(
       const viz::BeginFrameId& id);
+  CompositorFrameReporter::SmoothThread GetSmoothThread() const;
 
   const bool should_report_metrics_;
   viz::BeginFrameId last_submitted_frame_id_;
@@ -120,6 +124,9 @@ class CC_EXPORT CompositorFrameReportingController {
   bool next_activate_has_invalidation_ = false;
   CompositorFrameReporter::ActiveTrackers active_trackers_;
 
+  bool is_compositor_thread_driving_smoothness_ = false;
+  bool is_main_thread_driving_smoothness_ = false;
+
   // The latency reporter passed to each CompositorFrameReporter. Owned here
   // because it must be common among all reporters.
   // DO NOT reorder this line and the ones below. The latency_ukm_reporter_ must

cc/metrics/dropped_frame_counter.cc

@@ -41,10 +41,15 @@ void DroppedFrameCounter::AddDroppedFrame() {
   ++total_dropped_;
 }
 
+void DroppedFrameCounter::AddDroppedFrameAffectingSmoothness() {
+  ++total_smoothness_dropped_;
+}
+
 void DroppedFrameCounter::Reset() {
   total_frames_ = 0;
   total_partial_ = 0;
   total_dropped_ = 0;
+  total_smoothness_dropped_ = 0;
   ring_buffer_.Clear();
 }
 

cc/metrics/dropped_frame_counter.h

@@ -10,12 +10,13 @@
 #include <memory>
 
 #include "base/containers/ring_buffer.h"
+#include "cc/cc_export.h"
 
 namespace cc {
 
 // This class maintains a counter for produced/dropped frames, and can be used
 // to estimate the recent throughput.
-class DroppedFrameCounter {
+class CC_EXPORT DroppedFrameCounter {
  public:
   enum FrameState {
     kFrameStateDropped,
@@ -32,6 +33,7 @@ class DroppedFrameCounter {
   size_t total_frames() const { return total_frames_; }
   size_t total_compositor_dropped() const { return total_dropped_; }
   size_t total_main_dropped() const { return total_partial_; }
+  size_t total_smoothness_dropped() const { return total_smoothness_dropped_; }
 
   uint32_t GetAverageThroughput() const;
 
@@ -43,6 +45,8 @@ class DroppedFrameCounter {
   void AddPartialFrame();
   void AddDroppedFrame();
 
+  void AddDroppedFrameAffectingSmoothness();
+
   void Reset();
 
  private:
@@ -50,6 +54,7 @@ class DroppedFrameCounter {
   size_t total_frames_ = 0;
   size_t total_partial_ = 0;
   size_t total_dropped_ = 0;
+  size_t total_smoothness_dropped_ = 0;
 };
 
 }  // namespace cc

cc/metrics/dropped_frame_counter_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "cc/metrics/dropped_frame_counter.h"
+
+#include "base/synchronization/lock.h"
+#include "base/synchronization/waitable_event.h"
+#include "cc/animation/animation_host.h"
+#include "cc/test/layer_tree_test.h"
+
+namespace cc {
+namespace {
+
+class DroppedFrameCounterTestBase : public LayerTreeTest {
+ public:
+  DroppedFrameCounterTestBase() = default;
+  ~DroppedFrameCounterTestBase() override = default;
+
+  virtual void SetUpTestConfigAndExpectations() = 0;
+
+  void InitializeSettings(LayerTreeSettings* settings) override {
+    settings->commit_to_active_tree = false;
+  }
+
+  void RunTest(CompositorMode mode) override {
+    SetUpTestConfigAndExpectations();
+    LayerTreeTest::RunTest(mode);
+  }
+
+  void BeginTest() override {
+    ASSERT_GT(config_.animation_frames, 0u);
+
+    // Start with requesting main-frames.
+    PostSetNeedsCommitToMainThread();
+  }
+
+  void AfterTest() override {
+    EXPECT_GE(total_frames_, config_.animation_frames);
+    // It is possible to drop even more frame than what the test expects (e.g.
+    // in slower machines, slower builds such as asan/tsan builds, etc.), since
+    // the test does not strictly control both threads and deadlines. Therefore,
+    // it is not possible to check for strict equality here.
+    EXPECT_LE(expect_.min_dropped_main, dropped_main_);
+    EXPECT_LE(expect_.min_dropped_compositor, dropped_compositor_);
+    EXPECT_LE(expect_.min_dropped_smoothness, dropped_smoothness_);
+  }
+
+  // Compositor thread function overrides:
+  void WillBeginImplFrameOnThread(LayerTreeHostImpl* host_impl,
+                                  const viz::BeginFrameArgs& args) override {
+    // Request a re-draw, and set a non-empty damage region (otherwise the
+    // draw is aborted with 'no damage').
+    host_impl->SetNeedsRedraw();
+    host_impl->SetViewportDamage(gfx::Rect(0, 0, 10, 20));
+
+    // Request update from the main-thread too.
+    PostSetNeedsCommitToMainThread();
+  }
+
+  void DrawLayersOnThread(LayerTreeHostImpl* host_impl) override {
+    // If the main-thread is blocked, then unblock it once the compositor thread
+    // has already drawn a frame.
+    base::WaitableEvent* wait = nullptr;
+    {
+      base::AutoLock lock(wait_lock_);
+      wait = wait_;
+    }
+
+    if (wait) {
+      wait->Signal();
+    }
+  }
+
+  void DidReceivePresentationTimeOnThread(
+      LayerTreeHostImpl* host_impl,
+      uint32_t frame_token,
+      const gfx::PresentationFeedback& feedback) override {
+    ++presented_frames_;
+    if (presented_frames_ < config_.animation_frames)
+      return;
+
+    auto* dropped_frame_counter = host_impl->dropped_frame_counter();
+    DCHECK(dropped_frame_counter);
+
+    total_frames_ = dropped_frame_counter->total_frames();
+    dropped_main_ = dropped_frame_counter->total_main_dropped();
+    dropped_compositor_ = dropped_frame_counter->total_compositor_dropped();
+    dropped_smoothness_ = dropped_frame_counter->total_smoothness_dropped();
+    EndTest();
+  }
+
+  // Main-thread function overrides:
+  void BeginMainFrame(const viz::BeginFrameArgs& args) override {
+    bool should_wait = false;
+    if (config_.should_drop_main_every > 0) {
+      should_wait =
+          args.frame_id.sequence_number % config_.should_drop_main_every == 0;
+    }
+
+    if (should_wait) {
+      base::WaitableEvent wait{base::WaitableEvent::ResetPolicy::MANUAL,
+                               base::WaitableEvent::InitialState::NOT_SIGNALED};
+      {
+        base::AutoLock lock(wait_lock_);
+        wait_ = &wait;
+      }
+      wait.Wait();
+      {
+        base::AutoLock lock(wait_lock_);
+        wait_ = nullptr;
+      }
+    } else if (!TestEnded()) {
+      // Make sure BeginMainFrame keeps being issued.
+      layer_tree_host()->SetNeedsAnimate();
+      if (config_.should_register_main_thread_animation) {
+        animation_host()->SetAnimationCounts(1, true, true);
+      }
+    }
+  }
+
+ protected:
+  // The test configuration options. This is set before the test starts, and
+  // remains unchanged after that. So it is safe to read these fields from
+  // either threads.
+  struct TestConfig {
+    uint32_t should_drop_main_every = 0;
+    uint32_t animation_frames = 0;
+    bool should_register_main_thread_animation = false;
+  } config_;
+
+  // The test expectations. This is set before the test starts, and
+  // remains unchanged after that. So it is safe to read these fields from
+  // either threads.
+  struct TestExpectation {
+    uint32_t min_dropped_main = 0;
+    uint32_t min_dropped_compositor = 0;
+    uint32_t min_dropped_smoothness = 0;
+  } expect_;
+
+ private:
+  // This field is used only on the compositor thread to track how many frames
+  // have been processed.
+  uint32_t presented_frames_ = 0;
+
+  // The |wait_| event is used when the test wants to deliberately force the
+  // main-thread to block while processing begin-main-frames.
+  base::Lock wait_lock_;
+  base::WaitableEvent* wait_ = nullptr;
+
+  // These fields are populated in the compositor thread when the desired number
+  // of frames have been processed. These fields are subsequently compared
+  // against the expectation after the test ends.
+  uint32_t total_frames_ = 0;
+  uint32_t dropped_main_ = 0;
+  uint32_t dropped_compositor_ = 0;
+  uint32_t dropped_smoothness_ = 0;
+};
+
+class DroppedFrameCounterNoDropTest : public DroppedFrameCounterTestBase {
+ public:
+  ~DroppedFrameCounterNoDropTest() override = default;
+
+  void SetUpTestConfigAndExpectations() override {
+    config_.animation_frames = 28;
+    config_.should_register_main_thread_animation = false;
+
+    expect_.min_dropped_main = 0;
+    expect_.min_dropped_compositor = 0;
+    expect_.min_dropped_smoothness = 0;
+  }
+};
+
+MULTI_THREAD_TEST_F(DroppedFrameCounterNoDropTest);
+
+class DroppedFrameCounterMainDropsNoSmoothness
+    : public DroppedFrameCounterTestBase {
+ public:
+  ~DroppedFrameCounterMainDropsNoSmoothness() override = default;
+
+  void SetUpTestConfigAndExpectations() override {
+    config_.animation_frames = 28;
+    config_.should_drop_main_every = 5;
+    config_.should_register_main_thread_animation = false;
+
+    expect_.min_dropped_main = 5;
+    expect_.min_dropped_smoothness = 0;
+  }
+};
+
+MULTI_THREAD_TEST_F(DroppedFrameCounterMainDropsNoSmoothness);
+
+class DroppedFrameCounterMainDropsSmoothnessTest
+    : public DroppedFrameCounterTestBase {
+ public:
+  ~DroppedFrameCounterMainDropsSmoothnessTest() override = default;
+
+  void SetUpTestConfigAndExpectations() override {
+    config_.animation_frames = 28;
+    config_.should_drop_main_every = 5;
+    config_.should_register_main_thread_animation = true;
+
+    expect_.min_dropped_main = 5;
+    expect_.min_dropped_smoothness = 5;
+  }
+};
+
+MULTI_THREAD_TEST_F(DroppedFrameCounterMainDropsSmoothnessTest);
+
+}  // namespace
+}  // namespace cc

cc/metrics/frame_sequence_tracker_collection.cc

@@ -66,6 +66,25 @@ FrameSequenceTracker* FrameSequenceTrackerCollection::StartSequenceInternal(
     DCHECK_NE(scrolling_thread, ThreadType::kUnknown);
     metrics->SetScrollingThread(scrolling_thread);
   }
+
+  if (type != FrameSequenceTrackerType::kUniversal) {
+    if (metrics->GetEffectiveThread() == ThreadType::kCompositor) {
+      if (compositor_frame_reporting_controller_ &&
+          compositor_thread_driving_smoothness_ == 0) {
+        compositor_frame_reporting_controller_->SetThreadAffectsSmoothness(
+            ThreadType::kCompositor, true);
+      }
+      ++compositor_thread_driving_smoothness_;
+    } else {
+      DCHECK_EQ(metrics->GetEffectiveThread(), ThreadType::kMain);
+      if (compositor_frame_reporting_controller_ &&
+          main_thread_driving_smoothness_ == 0) {
+        compositor_frame_reporting_controller_->SetThreadAffectsSmoothness(
+            ThreadType::kMain, true);
+      }
+      ++main_thread_driving_smoothness_;
+    }
+  }
   return frame_trackers_[key].get();
 }
 
@@ -108,12 +127,31 @@ void FrameSequenceTrackerCollection::StopSequence(
   if (!frame_trackers_.contains(key))
     return;
 
-  std::unique_ptr<FrameSequenceTracker> tracker =
-      std::move(frame_trackers_[key]);
-
-  if (compositor_frame_reporting_controller_)
+  auto tracker = std::move(frame_trackers_[key]);
+  if (compositor_frame_reporting_controller_) {
     compositor_frame_reporting_controller_->RemoveActiveTracker(
         tracker->type());
+  }
+
+  if (type != FrameSequenceTrackerType::kUniversal) {
+    if (tracker->metrics()->GetEffectiveThread() == ThreadType::kCompositor) {
+      DCHECK_GT(compositor_thread_driving_smoothness_, 0u);
+      --compositor_thread_driving_smoothness_;
+      if (compositor_frame_reporting_controller_ &&
+          compositor_thread_driving_smoothness_ == 0) {
+        compositor_frame_reporting_controller_->SetThreadAffectsSmoothness(
+            ThreadType::kCompositor, false);
+      }
+    } else {
+      DCHECK_GT(main_thread_driving_smoothness_, 0u);
+      --main_thread_driving_smoothness_;
+      if (compositor_frame_reporting_controller_ &&
+          main_thread_driving_smoothness_ == 0) {
+        compositor_frame_reporting_controller_->SetThreadAffectsSmoothness(
+            ThreadType::kMain, false);
+      }
+    }
+  }
 
   frame_trackers_.erase(key);
   tracker->ScheduleTerminate();

cc/metrics/frame_sequence_tracker_collection.h

@@ -173,6 +173,10 @@ class CC_EXPORT FrameSequenceTrackerCollection {
       std::pair<FrameSequenceTrackerType, FrameSequenceMetrics::ThreadType>,
       std::unique_ptr<FrameSequenceMetrics>>
       accumulated_metrics_;
+
+  // Tracks how many smoothness effects are driven by each thread.
+  size_t main_thread_driving_smoothness_ = 0;
+  size_t compositor_thread_driving_smoothness_ = 0;
 };
 
 }  // namespace cc