[cc/metrics] Include information about main-thread update.
In each FrameInfo, include information about whether it contains update
from the main-thread or not. Use this information to determine whether
the compositor-thread update or main-thread update (or neither, or both)
were dropped for that frame.
BUG=1276983, 1256879
Change-Id: Iccfe7cb2d45d2927a8f16eb6da1719f59a7441e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3320476
Reviewed-by: Behdad Bakhshinategh <behdadb@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/main@{#949919}
diff --git a/cc/BUILD.gn b/cc/BUILD.gn
index f795eb8..60572d95 100644
--- a/cc/BUILD.gn
+++ b/cc/BUILD.gn
@@ -484,6 +484,8 @@
"test/fake_compositor_frame_reporting_controller.h",
"test/fake_content_layer_client.cc",
"test/fake_content_layer_client.h",
+ "test/fake_frame_info.cc",
+ "test/fake_frame_info.h",
"test/fake_impl_task_runner_provider.h",
"test/fake_layer_tree_frame_sink.cc",
"test/fake_layer_tree_frame_sink.h",
diff --git a/cc/metrics/compositor_frame_reporter.cc b/cc/metrics/compositor_frame_reporter.cc
index e3f6fb42..b6ea0646 100644
--- a/cc/metrics/compositor_frame_reporter.cc
+++ b/cc/metrics/compositor_frame_reporter.cc
@@ -1422,8 +1422,30 @@
FrameInfo info;
info.final_state = final_state;
info.smooth_thread = smooth_thread_;
+ info.scroll_thread = scrolling_thread_;
info.has_missing_content = has_missing_content_;
+ if (frame_skip_reason_.has_value() &&
+ frame_skip_reason() == FrameSkippedReason::kNoDamage) {
+ // If the frame was explicitly skipped because of 'no damage', then that
+ // means this frame contains the response ('no damage') from the
+ // main-thread.
+ info.main_thread_response = FrameInfo::MainThreadResponse::kIncluded;
+ } else if (partial_update_dependents_.size() > 0) {
+ // Only a frame containing a response from the main-thread can have
+ // dependent reporters.
+ info.main_thread_response = FrameInfo::MainThreadResponse::kIncluded;
+ } else if (begin_main_frame_start_.is_null() ||
+ (frame_skip_reason_.has_value() &&
+ frame_skip_reason() == FrameSkippedReason::kWaitingOnMain)) {
+ // If 'begin main frame' never started, or if it started, but it
+ // had to be skipped because it was waiting on the main-thread,
+ // then the main-thread update is missing from this reporter.
+ info.main_thread_response = FrameInfo::MainThreadResponse::kMissing;
+ } else {
+ info.main_thread_response = FrameInfo::MainThreadResponse::kIncluded;
+ }
+
if (!stage_history_.empty()) {
const auto& stage = stage_history_.back();
if (stage.stage_type == StageType::kTotalLatency) {
@@ -1433,8 +1455,6 @@
}
}
- info.scroll_thread = scrolling_thread_;
-
return info;
}
diff --git a/cc/metrics/compositor_frame_reporting_controller_unittest.cc b/cc/metrics/compositor_frame_reporting_controller_unittest.cc
index e0c8522..01d9953 100644
--- a/cc/metrics/compositor_frame_reporting_controller_unittest.cc
+++ b/cc/metrics/compositor_frame_reporting_controller_unittest.cc
@@ -1848,12 +1848,12 @@
reporting_controller_.DidPresentCompositorFrame(1u, details);
// Starts a new frame and submit it prior to commit
- reporting_controller_.WillCommit();
- reporting_controller_.DidCommit();
+ SimulateCommit(nullptr);
const auto previous_id = current_id_;
SimulateBeginMainFrame();
+ DCHECK_NE(previous_id, current_id_);
reporting_controller_.OnFinishImplFrame(current_id_);
// Starts a new frame and submit it prior to its commit, but the older frame
@@ -1862,13 +1862,11 @@
reporting_controller_.DidActivate();
reporting_controller_.DidSubmitCompositorFrame(
- 1u, current_id_, previous_id, {}, /*has_missing_content=*/false);
+ 2u, current_id_, previous_id, {}, /*has_missing_content=*/false);
details.presentation_feedback.timestamp = AdvanceNowByMs(10);
- reporting_controller_.DidPresentCompositorFrame(1u, details);
+ reporting_controller_.DidPresentCompositorFrame(2u, details);
- reporting_controller_.WillCommit();
- reporting_controller_.DidCommit();
-
+ SimulateCommit(nullptr);
SimulatePresentCompositorFrame();
// There are two frames with partial updates
diff --git a/cc/metrics/dropped_frame_counter.cc b/cc/metrics/dropped_frame_counter.cc
index 17766b02..f3f8b593 100644
--- a/cc/metrics/dropped_frame_counter.cc
+++ b/cc/metrics/dropped_frame_counter.cc
@@ -9,7 +9,6 @@
#include <iterator>
#include "base/bind.h"
-#include "base/logging.h"
#include "base/metrics/histogram.h"
#include "base/metrics/histogram_macros.h"
#include "base/trace_event/trace_event.h"
diff --git a/cc/metrics/dropped_frame_counter_unittest.cc b/cc/metrics/dropped_frame_counter_unittest.cc
index 57ee9da2..c0cd00b 100644
--- a/cc/metrics/dropped_frame_counter_unittest.cc
+++ b/cc/metrics/dropped_frame_counter_unittest.cc
@@ -14,6 +14,7 @@
#include "build/chromeos_buildflags.h"
#include "cc/animation/animation_host.h"
#include "cc/test/fake_content_layer_client.h"
+#include "cc/test/fake_frame_info.h"
#include "cc/test/fake_picture_layer.h"
#include "cc/test/layer_tree_test.h"
@@ -21,9 +22,9 @@
namespace {
FrameInfo CreateStubFrameInfo(bool is_dropped) {
- return {is_dropped ? FrameInfo::FrameFinalState::kDropped
- : FrameInfo::FrameFinalState::kPresentedAll,
- FrameInfo::SmoothThread::kSmoothBoth};
+ return CreateFakeFrameInfo(is_dropped
+ ? FrameInfo::FrameFinalState::kDropped
+ : FrameInfo::FrameFinalState::kPresentedAll);
}
class DroppedFrameCounterTestBase : public LayerTreeTest {
@@ -310,8 +311,17 @@
viz::BeginFrameArgs args_ = SimulateBeginFrameArgs();
dropped_frame_counter_.OnBeginFrame(args_, /*is_scroll_active=*/false);
dropped_frame_counter_.OnBeginFrame(args_, /*is_scroll_active=*/false);
- dropped_frame_counter_.OnEndFrame(args_, CreateStubFrameInfo(main_dropped));
- dropped_frame_counter_.OnEndFrame(args_, CreateStubFrameInfo(impl_dropped));
+
+ // End the 'main thread' arm of the fork.
+ auto main_info = CreateStubFrameInfo(main_dropped);
+ main_info.main_thread_response = FrameInfo::MainThreadResponse::kIncluded;
+ dropped_frame_counter_.OnEndFrame(args_, main_info);
+
+ // End the 'compositor thread' arm of the fork.
+ auto impl_info = CreateStubFrameInfo(impl_dropped);
+ impl_info.main_thread_response = FrameInfo::MainThreadResponse::kMissing;
+ dropped_frame_counter_.OnEndFrame(args_, impl_info);
+
sequence_number_++;
frame_time_ += interval_;
}
diff --git a/cc/metrics/frame_info.cc b/cc/metrics/frame_info.cc
index 7c9703c7..5334f7a 100644
--- a/cc/metrics/frame_info.cc
+++ b/cc/metrics/frame_info.cc
@@ -6,6 +6,8 @@
#include <algorithm>
+#include "build/build_config.h"
+
namespace cc {
namespace {
@@ -20,6 +22,21 @@
thread == FrameInfo::SmoothThread::kSmoothBoth;
}
+bool ValidateFinalStateIsForMainThread(FrameInfo::FrameFinalState state) {
+ switch (state) {
+ case FrameInfo::FrameFinalState::kPresentedPartialOldMain:
+ case FrameInfo::FrameFinalState::kPresentedPartialNewMain:
+ // Frames that contain main-thread update cannot have a 'partial update'
+ // state.
+ return false;
+
+ case FrameInfo::FrameFinalState::kPresentedAll:
+ case FrameInfo::FrameFinalState::kNoUpdateDesired:
+ case FrameInfo::FrameFinalState::kDropped:
+ return true;
+ }
+}
+
} // namespace
bool FrameInfo::IsDroppedAffectingSmoothness() const {
@@ -28,42 +45,67 @@
if (smooth_thread == SmoothThread::kSmoothNone)
return false;
- switch (final_state) {
- case FrameFinalState::kDropped:
- return true;
-
- case FrameFinalState::kPresentedAll:
- case FrameFinalState::kPresentedPartialNewMain:
- // If the frame includes new main-thread update, even if it's for an
- // earlier begin-frame, then do not count it as a dropped frame affecting
- // smoothness.
- return false;
-
- case FrameFinalState::kPresentedPartialOldMain:
- // Partial-update frames without new updates from the main-thread affect
- // smoothness if the main-thread is expected to be smooth.
- return smooth_thread == SmoothThread::kSmoothBoth ||
- smooth_thread == SmoothThread::kSmoothMain;
-
- case FrameFinalState::kNoUpdateDesired:
- return false;
+ if (IsMainSmooth(smooth_thread) && WasMainUpdateDropped()) {
+ return true;
}
+
+ if (IsCompositorSmooth(smooth_thread) && WasCompositorUpdateDropped()) {
+ return true;
+ }
+
+ return false;
}
-void FrameInfo::MergeWith(const FrameInfo& info) {
+void FrameInfo::MergeWith(const FrameInfo& other) {
+#if defined(OS_ANDROID)
+ // TODO(1278168): on android-webview, multiple frames can be submitted against
+ // the same BeginFrameArgs. This can trip the DCHECK()s in this function.
+ if (was_merged)
+ return;
+#endif
+ DCHECK(!was_merged);
+ DCHECK(!other.was_merged);
+ DCHECK(Validate());
+ DCHECK(other.Validate());
+
+ if (main_thread_response == MainThreadResponse::kIncluded) {
+ // |this| includes the main-thread updates. Therefore:
+ // - |other| must not also include main-thread updates.
+ // - |this| must have a valid final-state.
+ DCHECK_EQ(MainThreadResponse::kMissing, other.main_thread_response);
+ DCHECK(ValidateFinalStateIsForMainThread(final_state));
+
+ main_update_was_dropped = final_state == FrameFinalState::kDropped;
+ compositor_update_was_dropped =
+ other.final_state == FrameFinalState::kDropped;
+ } else {
+ // |this| does not include main-thread updates. Therefore:
+ // - |other| must include main-thread updates.
+ // - |other| must have a valid final-state.
+ DCHECK_EQ(MainThreadResponse::kIncluded, other.main_thread_response);
+ DCHECK(ValidateFinalStateIsForMainThread(other.final_state));
+
+ main_update_was_dropped = other.final_state == FrameFinalState::kDropped;
+ compositor_update_was_dropped = final_state == FrameFinalState::kDropped;
+ }
+
+ was_merged = true;
+ main_thread_response = MainThreadResponse::kIncluded;
+
// The |scroll_thread| information cannot change once the frame starts. So
// it should not need to be updated during merge.
- DCHECK_EQ(scroll_thread, info.scroll_thread);
+ DCHECK_EQ(scroll_thread, other.scroll_thread);
- if (info.has_missing_content)
+ if (other.has_missing_content)
has_missing_content = true;
- if (info.final_state == FrameFinalState::kDropped)
+
+ if (other.final_state == FrameFinalState::kDropped)
final_state = FrameFinalState::kDropped;
const bool is_compositor_smooth = IsCompositorSmooth(smooth_thread) ||
- IsCompositorSmooth(info.smooth_thread);
+ IsCompositorSmooth(other.smooth_thread);
const bool is_main_smooth =
- IsMainSmooth(smooth_thread) || IsMainSmooth(info.smooth_thread);
+ IsMainSmooth(smooth_thread) || IsMainSmooth(other.smooth_thread);
if (is_compositor_smooth && is_main_smooth) {
smooth_thread = SmoothThread::kSmoothBoth;
} else if (is_compositor_smooth) {
@@ -74,7 +116,51 @@
smooth_thread = SmoothThread::kSmoothNone;
}
- total_latency = std::max(total_latency, info.total_latency);
+ total_latency = std::max(total_latency, other.total_latency);
+
+ // Validate the state after the merge.
+ DCHECK(Validate());
+}
+
+bool FrameInfo::Validate() const {
+ // If |scroll_thread| is set, then the |smooth_thread| must include that
+ // thread.
+ if (scroll_thread == SmoothEffectDrivingThread::kCompositor) {
+ DCHECK(IsCompositorSmooth(smooth_thread));
+ } else if (scroll_thread == SmoothEffectDrivingThread::kMain) {
+ DCHECK(IsMainSmooth(smooth_thread));
+ }
+
+ return true;
+}
+
+bool FrameInfo::WasCompositorUpdateDropped() const {
+ if (was_merged)
+ return compositor_update_was_dropped;
+ return final_state == FrameFinalState::kDropped;
+}
+
+bool FrameInfo::WasMainUpdateDropped() const {
+ if (was_merged)
+ return main_update_was_dropped;
+
+ switch (final_state) {
+ case FrameFinalState::kDropped:
+ case FrameFinalState::kPresentedPartialOldMain:
+ return true;
+
+ case FrameFinalState::kPresentedPartialNewMain:
+ // Although this frame dropped the main-thread updates for this particular
+ // frame, it did include new main-thread update. So do not treat this as a
+ // dropped frame.
+ return false;
+
+ case FrameFinalState::kNoUpdateDesired:
+ case FrameFinalState::kPresentedAll:
+ return false;
+ }
+
+ return false;
}
} // namespace cc
diff --git a/cc/metrics/frame_info.h b/cc/metrics/frame_info.h
index 36721c86..db93d10 100644
--- a/cc/metrics/frame_info.h
+++ b/cc/metrics/frame_info.h
@@ -45,6 +45,12 @@
};
SmoothThread smooth_thread = SmoothThread::kSmoothNone;
+ enum class MainThreadResponse {
+ kIncluded,
+ kMissing,
+ };
+ MainThreadResponse main_thread_response = MainThreadResponse::kIncluded;
+
enum class SmoothEffectDrivingThread { kMain, kCompositor, kUnknown };
SmoothEffectDrivingThread scroll_thread = SmoothEffectDrivingThread::kUnknown;
@@ -57,6 +63,16 @@
bool IsDroppedAffectingSmoothness() const;
void MergeWith(const FrameInfo& info);
+
+ bool Validate() const;
+
+ bool WasCompositorUpdateDropped() const;
+ bool WasMainUpdateDropped() const;
+
+ private:
+ bool was_merged = false;
+ bool compositor_update_was_dropped = false;
+ bool main_update_was_dropped = false;
};
} // namespace cc
diff --git a/cc/metrics/frame_sorter.cc b/cc/metrics/frame_sorter.cc
index f14d4c84..a74d723 100644
--- a/cc/metrics/frame_sorter.cc
+++ b/cc/metrics/frame_sorter.cc
@@ -64,6 +64,7 @@
while (pending_frames_.size() > kPendingFramesMaxSize) {
const auto& first = pending_frames_.front();
frame_states_.erase(first.frame_id);
+ frame_infos_.erase(first.frame_id);
pending_frames_.pop_front();
}
}
@@ -98,6 +99,7 @@
if (frame_state.should_ignore()) {
// The associated frame in pending_frames_ was already removed in Reset().
frame_states_.erase(args.frame_id);
+ frame_infos_.erase(args.frame_id);
return;
}
diff --git a/cc/metrics/frame_sorter_unittest.cc b/cc/metrics/frame_sorter_unittest.cc
index bbe7ece..0672e6f 100644
--- a/cc/metrics/frame_sorter_unittest.cc
+++ b/cc/metrics/frame_sorter_unittest.cc
@@ -11,6 +11,7 @@
#include "base/bind.h"
#include "base/strings/string_number_conversions.h"
#include "cc/metrics/frame_info.h"
+#include "cc/test/fake_frame_info.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace cc {
@@ -51,6 +52,9 @@
// "R": Reset the frame sorter.
// Method expects the start of frames to be in order starting with 1.
void SimulateQueries(std::vector<std::string> queries) {
+ // Keeps track of how many times a frame is terminated.
+ std::map<int, int> end_counters;
+
for (auto& query : queries) {
int id;
base::StringToInt(query.substr(1), &id);
@@ -63,13 +67,35 @@
frame_sorter_.AddNewFrame(args_[id]);
break;
case 'D': {
- FrameInfo info = {FrameInfo::FrameFinalState::kDropped,
- FrameInfo::SmoothThread::kSmoothBoth};
+ ++end_counters[id];
+ FrameInfo info =
+ CreateFakeFrameInfo(FrameInfo::FrameFinalState::kDropped);
+ if (end_counters[id] == 1) {
+ // For the first response to the frame, mark it as not including
+ // update from the main-thread.
+ info.main_thread_response = FrameInfo::MainThreadResponse::kMissing;
+ } else {
+ DCHECK_EQ(2, end_counters[id]);
+ info.main_thread_response =
+ FrameInfo::MainThreadResponse::kIncluded;
+ }
frame_sorter_.AddFrameResult(args_[id], info);
break;
}
case 'P': {
- frame_sorter_.AddFrameResult(args_[id], {});
+ ++end_counters[id];
+ FrameInfo info =
+ CreateFakeFrameInfo(FrameInfo::FrameFinalState::kPresentedAll);
+ if (end_counters[id] == 1) {
+ // For the first response to the frame, mark it as not including
+ // update from the main-thread.
+ info.main_thread_response = FrameInfo::MainThreadResponse::kMissing;
+ } else {
+ DCHECK_EQ(2, end_counters[id]);
+ info.main_thread_response =
+ FrameInfo::MainThreadResponse::kIncluded;
+ }
+ frame_sorter_.AddFrameResult(args_[id], info);
break;
}
case 'I':
diff --git a/cc/test/fake_frame_info.cc b/cc/test/fake_frame_info.cc
new file mode 100644
index 0000000..9640323
--- /dev/null
+++ b/cc/test/fake_frame_info.cc
@@ -0,0 +1,16 @@
+// Copyright 2021 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/test/fake_frame_info.h"
+
+namespace cc {
+
+FrameInfo CreateFakeFrameInfo(FrameInfo::FrameFinalState state) {
+ FrameInfo info;
+ info.final_state = state;
+ info.smooth_thread = FrameInfo::SmoothThread::kSmoothBoth;
+ return info;
+}
+
+} // namespace cc
diff --git a/cc/test/fake_frame_info.h b/cc/test/fake_frame_info.h
new file mode 100644
index 0000000..86ff412
--- /dev/null
+++ b/cc/test/fake_frame_info.h
@@ -0,0 +1,17 @@
+// Copyright 2021 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.
+
+#ifndef CC_TEST_FAKE_FRAME_INFO_H_
+#define CC_TEST_FAKE_FRAME_INFO_H_
+
+#include "cc/metrics/frame_info.h"
+
+namespace cc {
+
+// Creates and returns a FrameInfo instance with the desired |state|.
+FrameInfo CreateFakeFrameInfo(FrameInfo::FrameFinalState state);
+
+} // namespace cc
+
+#endif // CC_TEST_FAKE_FRAME_INFO_H_
diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
index 44cebdc..7bb7d0d 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -50,6 +50,7 @@
#include "cc/resources/ui_resource_bitmap.h"
#include "cc/resources/ui_resource_manager.h"
#include "cc/test/animation_test_common.h"
+#include "cc/test/fake_frame_info.h"
#include "cc/test/fake_impl_task_runner_provider.h"
#include "cc/test/fake_layer_tree_frame_sink.h"
#include "cc/test/fake_layer_tree_host_impl.h"
@@ -14048,9 +14049,8 @@
BEGINFRAME_FROM_HERE, 1u /*source_id*/, 2u /*sequence_number*/, now,
deadline, interval, viz::BeginFrameArgs::NORMAL);
- dropped_frame_counter->OnEndFrame(args,
- {FrameInfo::FrameFinalState::kDropped,
- FrameInfo::SmoothThread::kSmoothBoth});
+ dropped_frame_counter->OnEndFrame(
+ args, CreateFakeFrameInfo(FrameInfo::FrameFinalState::kDropped));
// FCP not received, so the total_smoothness_dropped_ won't increase.
EXPECT_EQ(dropped_frame_counter->total_smoothness_dropped(), 0u);
@@ -14058,9 +14058,8 @@
begin_frame_metrics.should_measure_smoothness = true;
host_impl_->ReadyToCommit(args, &begin_frame_metrics);
dropped_frame_counter->SetTimeFcpReceivedForTesting(args.frame_time);
- dropped_frame_counter->OnEndFrame(args,
- {FrameInfo::FrameFinalState::kDropped,
- FrameInfo::SmoothThread::kSmoothBoth});
+ dropped_frame_counter->OnEndFrame(
+ args, CreateFakeFrameInfo(FrameInfo::FrameFinalState::kDropped));
EXPECT_EQ(dropped_frame_counter->total_smoothness_dropped(), 1u);
total_frame_counter->set_total_frames_for_testing(1u);
diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc
index dcdf461..42dadea3 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -39,6 +39,7 @@
#include "cc/paint/image_animation_count.h"
#include "cc/resources/ui_resource_manager.h"
#include "cc/test/fake_content_layer_client.h"
+#include "cc/test/fake_frame_info.h"
#include "cc/test/fake_layer_tree_host_client.h"
#include "cc/test/fake_paint_image_generator.h"
#include "cc/test/fake_painted_scrollbar_layer.h"
@@ -118,6 +119,12 @@
return node->subtree_has_copy_request;
}
+FrameInfo CreateFakeImplDroppedFrameInfo() {
+ auto info = CreateFakeFrameInfo(FrameInfo::FrameFinalState::kDropped);
+ info.main_thread_response = FrameInfo::MainThreadResponse::kMissing;
+ return info;
+}
+
using LayerTreeHostTest = LayerTreeTest;
class LayerTreeHostTestHasImplThreadTest : public LayerTreeHostTest {
@@ -9950,8 +9957,7 @@
// Mark every frame as a dropped frame affecting smoothness.
host_impl->dropped_frame_counter()->OnEndFrame(
- last_args_, {FrameInfo::FrameFinalState::kDropped,
- FrameInfo::SmoothThread::kSmoothBoth});
+ last_args_, CreateFakeImplDroppedFrameInfo());
host_impl->SetNeedsRedraw();
--frames_counter_;
}
@@ -10003,10 +10009,11 @@
return;
}
- // Mark every frame as a dropped frame affecting smoothness.
+ // Mark every frame as a dropped frame affecting smoothness. This happens
+ // entirely on the compositor thread, so mark it as not including
+ // main-thread update.
host_impl->dropped_frame_counter()->OnEndFrame(
- last_args_, {FrameInfo::FrameFinalState::kDropped,
- FrameInfo::SmoothThread::kSmoothBoth});
+ last_args_, CreateFakeImplDroppedFrameInfo());
host_impl->SetNeedsRedraw();
--frames_counter_;
}