Commit 642cf5c5 authored by Tim Volodine's avatar Tim Volodine Committed by Commit Bot

Revert "[cc/metric] Some sanity checks in the code."

This reverts commit bed8ddb1.

Reason for revert:
Failing on multiple bots Android WebView O,P (dbg), test-o-phone,
see crbug.com/1073077

Original change's description:
> [cc/metric] Some sanity checks in the code.
>
> Add some DCHECK()s in the code to validate some assumptions (e.g. make
> sure the reporters are terminated predictably), and update tests
> accordingly.
>
> BUG=none
>
> Change-Id: Id01bd571ad1d59b90c297f71097edcf3e6edc59b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2128448
> Auto-Submit: Sadrul Chowdhury <sadrul@chromium.org>
> Reviewed-by: Behdad Bakhshinategh <behdadb@chromium.org>
> Commit-Queue: Behdad Bakhshinategh <behdadb@chromium.org>
> Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#760220}

TBR=sadrul@chromium.org,behdadb@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1073077
Change-Id: Ie97d69b1b176f5048c7b15d5f0d418d0a15346be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2159443
Commit-Queue: Tim Volodine <timvolodine@chromium.org>
Reviewed-by: default avatarTim Volodine <timvolodine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#761070}
parent 196c7017
...@@ -329,7 +329,8 @@ void CompositorFrameReporter::MissedDeadlineFrame() { ...@@ -329,7 +329,8 @@ void CompositorFrameReporter::MissedDeadlineFrame() {
} }
void CompositorFrameReporter::TerminateReporter() { void CompositorFrameReporter::TerminateReporter() {
DCHECK_NE(frame_termination_status_, FrameTerminationStatus::kUnknown); if (frame_termination_status_ == FrameTerminationStatus::kUnknown)
TerminateFrame(FrameTerminationStatus::kUnknown, base::TimeTicks::Now());
DCHECK_EQ(current_stage_.start_time, base::TimeTicks()); DCHECK_EQ(current_stage_.start_time, base::TimeTicks());
bool report_compositor_latency = false; bool report_compositor_latency = false;
bool report_event_latency = false; bool report_event_latency = false;
......
...@@ -79,7 +79,6 @@ void CompositorFrameReportingController::WillBeginImplFrame( ...@@ -79,7 +79,6 @@ void CompositorFrameReportingController::WillBeginImplFrame(
void CompositorFrameReportingController::WillBeginMainFrame( void CompositorFrameReportingController::WillBeginMainFrame(
const viz::BeginFrameArgs& args) { const viz::BeginFrameArgs& args) {
const auto now = Now();
if (reporters_[PipelineStage::kBeginImplFrame]) { if (reporters_[PipelineStage::kBeginImplFrame]) {
// We need to use .get() below because operator<< in std::unique_ptr is a // We need to use .get() below because operator<< in std::unique_ptr is a
// C++20 feature. // C++20 feature.
...@@ -88,22 +87,10 @@ void CompositorFrameReportingController::WillBeginMainFrame( ...@@ -88,22 +87,10 @@ void CompositorFrameReportingController::WillBeginMainFrame(
DCHECK_EQ(reporters_[PipelineStage::kBeginImplFrame]->frame_id_, DCHECK_EQ(reporters_[PipelineStage::kBeginImplFrame]->frame_id_,
args.frame_id); args.frame_id);
reporters_[PipelineStage::kBeginImplFrame]->StartStage( reporters_[PipelineStage::kBeginImplFrame]->StartStage(
StageType::kSendBeginMainFrameToCommit, now); StageType::kSendBeginMainFrameToCommit, Now());
AdvanceReporterStage(PipelineStage::kBeginImplFrame, AdvanceReporterStage(PipelineStage::kBeginImplFrame,
PipelineStage::kBeginMainFrame); PipelineStage::kBeginMainFrame);
} else { } else {
auto& old_reporter = reporters_[PipelineStage::kBeginMainFrame];
if (old_reporter) {
DCHECK(old_reporter->did_abort_main_frame());
if (old_reporter->did_not_produce_frame()) {
old_reporter->TerminateFrame(
FrameTerminationStatus::kDidNotProduceFrame,
old_reporter->did_not_produce_frame_time());
} else {
old_reporter->TerminateFrame(
FrameTerminationStatus::kReplacedByNewReporter, now);
}
}
// In this case we have already submitted the ImplFrame, but we received // In this case we have already submitted the ImplFrame, but we received
// beginMain frame before next BeginImplFrame (Not reached the ImplFrame // beginMain frame before next BeginImplFrame (Not reached the ImplFrame
// deadline yet). So will start a new reporter at BeginMainFrame. // deadline yet). So will start a new reporter at BeginMainFrame.
...@@ -112,7 +99,7 @@ void CompositorFrameReportingController::WillBeginMainFrame( ...@@ -112,7 +99,7 @@ void CompositorFrameReportingController::WillBeginMainFrame(
active_trackers_, args.frame_id, active_trackers_, args.frame_id,
args.frame_time + (args.interval * 1.5), args.frame_time + (args.interval * 1.5),
latency_ukm_reporter_.get(), should_report_metrics_); latency_ukm_reporter_.get(), should_report_metrics_);
reporter->StartStage(StageType::kSendBeginMainFrameToCommit, now); reporter->StartStage(StageType::kSendBeginMainFrameToCommit, Now());
reporters_[PipelineStage::kBeginMainFrame] = std::move(reporter); reporters_[PipelineStage::kBeginMainFrame] = std::move(reporter);
} }
} }
...@@ -369,9 +356,7 @@ bool CompositorFrameReportingController::CanSubmitImplFrame( ...@@ -369,9 +356,7 @@ bool CompositorFrameReportingController::CanSubmitImplFrame(
if (!reporters_[PipelineStage::kBeginImplFrame]) if (!reporters_[PipelineStage::kBeginImplFrame])
return false; return false;
auto& reporter = reporters_[PipelineStage::kBeginImplFrame]; auto& reporter = reporters_[PipelineStage::kBeginImplFrame];
DCHECK_EQ(reporter->frame_id_, id); return (reporter->frame_id_ == id && reporter->did_finish_impl_frame());
DCHECK(reporter->did_finish_impl_frame());
return true;
} }
bool CompositorFrameReportingController::CanSubmitMainFrame( bool CompositorFrameReportingController::CanSubmitMainFrame(
...@@ -379,8 +364,8 @@ bool CompositorFrameReportingController::CanSubmitMainFrame( ...@@ -379,8 +364,8 @@ bool CompositorFrameReportingController::CanSubmitMainFrame(
if (!reporters_[PipelineStage::kBeginMainFrame]) if (!reporters_[PipelineStage::kBeginMainFrame])
return false; return false;
auto& reporter = reporters_[PipelineStage::kBeginMainFrame]; auto& reporter = reporters_[PipelineStage::kBeginMainFrame];
DCHECK(reporter->did_finish_impl_frame()); return (reporter->frame_id_ == id && reporter->did_finish_impl_frame() &&
return reporter->frame_id_ == id && reporter->did_abort_main_frame(); reporter->did_abort_main_frame());
} }
std::unique_ptr<CompositorFrameReporter> std::unique_ptr<CompositorFrameReporter>
......
...@@ -108,12 +108,10 @@ class CompositorFrameReportingControllerTest : public testing::Test { ...@@ -108,12 +108,10 @@ class CompositorFrameReportingControllerTest : public testing::Test {
if (!reporting_controller_.reporters() if (!reporting_controller_.reporters()
[CompositorFrameReportingController::PipelineStage::kActivate]) [CompositorFrameReportingController::PipelineStage::kActivate])
SimulateActivate(); SimulateActivate();
auto& reporter = CHECK(reporting_controller_.reporters()
reporting_controller_.reporters() [CompositorFrameReportingController::PipelineStage::kActivate]);
[CompositorFrameReportingController::PipelineStage::kActivate]; reporting_controller_.DidSubmitCompositorFrame(frame_token, current_id_,
DCHECK(reporter); last_activated_id_,
reporting_controller_.DidSubmitCompositorFrame(
frame_token, reporter->frame_id_, last_activated_id_,
std::move(events_metrics)); std::move(events_metrics));
} }
...@@ -517,6 +515,9 @@ TEST_F(CompositorFrameReportingControllerTest, MainFrameAborted2) { ...@@ -517,6 +515,9 @@ TEST_F(CompositorFrameReportingControllerTest, MainFrameAborted2) {
viz::BeginFrameId current_id_2(1, 2); viz::BeginFrameId current_id_2(1, 2);
viz::BeginFrameArgs args_2 = SimulateBeginFrameArgs(current_id_2); viz::BeginFrameArgs args_2 = SimulateBeginFrameArgs(current_id_2);
viz::BeginFrameId current_id_3(1, 3);
viz::BeginFrameArgs args_3 = SimulateBeginFrameArgs(current_id_3);
reporting_controller_.WillBeginImplFrame(args_1); reporting_controller_.WillBeginImplFrame(args_1);
reporting_controller_.OnFinishImplFrame(current_id_1); reporting_controller_.OnFinishImplFrame(current_id_1);
reporting_controller_.WillBeginMainFrame(args_1); reporting_controller_.WillBeginMainFrame(args_1);
...@@ -547,6 +548,44 @@ TEST_F(CompositorFrameReportingControllerTest, MainFrameAborted2) { ...@@ -547,6 +548,44 @@ TEST_F(CompositorFrameReportingControllerTest, MainFrameAborted2) {
histogram_tester.ExpectTotalCount( histogram_tester.ExpectTotalCount(
"CompositorLatency.SubmitCompositorFrameToPresentationCompositorFrame", "CompositorLatency.SubmitCompositorFrameToPresentationCompositorFrame",
2); 2);
reporting_controller_.DidSubmitCompositorFrame(2, current_id_2, current_id_1,
{});
reporting_controller_.DidPresentCompositorFrame(2, details);
histogram_tester.ExpectTotalCount(
"CompositorLatency.DroppedFrame.BeginImplFrameToSendBeginMainFrame", 0);
histogram_tester.ExpectTotalCount(
"CompositorLatency.BeginImplFrameToSendBeginMainFrame", 2);
histogram_tester.ExpectTotalCount(
"CompositorLatency.SendBeginMainFrameToCommit", 2);
histogram_tester.ExpectTotalCount("CompositorLatency.Commit", 1);
histogram_tester.ExpectTotalCount("CompositorLatency.EndCommitToActivation",
1);
histogram_tester.ExpectTotalCount("CompositorLatency.Activation", 1);
histogram_tester.ExpectTotalCount(
"CompositorLatency.EndActivateToSubmitCompositorFrame", 2);
histogram_tester.ExpectTotalCount(
"CompositorLatency.SubmitCompositorFrameToPresentationCompositorFrame",
2);
reporting_controller_.WillBeginImplFrame(args_3);
reporting_controller_.OnFinishImplFrame(current_id_3);
reporting_controller_.DidSubmitCompositorFrame(3, current_id_3, current_id_1,
{});
reporting_controller_.DidPresentCompositorFrame(3, details);
histogram_tester.ExpectTotalCount(
"CompositorLatency.DroppedFrame.BeginImplFrameToSendBeginMainFrame", 0);
histogram_tester.ExpectTotalCount(
"CompositorLatency.BeginImplFrameToSendBeginMainFrame", 3);
histogram_tester.ExpectTotalCount(
"CompositorLatency.SendBeginMainFrameToCommit", 2);
histogram_tester.ExpectTotalCount("CompositorLatency.Commit", 1);
histogram_tester.ExpectTotalCount("CompositorLatency.EndCommitToActivation",
1);
histogram_tester.ExpectTotalCount("CompositorLatency.Activation", 1);
histogram_tester.ExpectTotalCount(
"CompositorLatency.EndActivateToSubmitCompositorFrame", 3);
histogram_tester.ExpectTotalCount(
"CompositorLatency.SubmitCompositorFrameToPresentationCompositorFrame",
3);
} }
TEST_F(CompositorFrameReportingControllerTest, LongMainFrame) { TEST_F(CompositorFrameReportingControllerTest, LongMainFrame) {
...@@ -710,33 +749,42 @@ TEST_F(CompositorFrameReportingControllerTest, BlinkBreakdown) { ...@@ -710,33 +749,42 @@ TEST_F(CompositorFrameReportingControllerTest, BlinkBreakdown) {
blink_breakdown->composite_commit = base::TimeDelta::FromMicroseconds(2); blink_breakdown->composite_commit = base::TimeDelta::FromMicroseconds(2);
blink_breakdown->update_layers = base::TimeDelta::FromMicroseconds(1); blink_breakdown->update_layers = base::TimeDelta::FromMicroseconds(1);
SimulateCommit(std::move(blink_breakdown));
SimulateActivate(); SimulateActivate();
SimulateCommit(std::move(blink_breakdown));
SimulatePresentCompositorFrame(); SimulatePresentCompositorFrame();
histogram_tester.ExpectTotalCount( histogram_tester.ExpectTotalCount(
"CompositorLatency.SendBeginMainFrameToCommit", 1); "CompositorLatency.SendBeginMainFrameToCommit", 1);
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"CompositorLatency.SendBeginMainFrameToCommit.HandleInputEvents", 10, 1); "CompositorLatency.SendBeginMainFrameToCommit.HandleInputEvents",
base::TimeDelta::FromMicroseconds(10).InMilliseconds(), 1);
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"CompositorLatency.SendBeginMainFrameToCommit.Animate", 9, 1); "CompositorLatency.SendBeginMainFrameToCommit.Animate",
base::TimeDelta::FromMicroseconds(9).InMilliseconds(), 1);
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"CompositorLatency.SendBeginMainFrameToCommit.StyleUpdate", 8, 1); "CompositorLatency.SendBeginMainFrameToCommit.StyleUpdate",
base::TimeDelta::FromMicroseconds(8).InMilliseconds(), 1);
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"CompositorLatency.SendBeginMainFrameToCommit.LayoutUpdate", 7, 1); "CompositorLatency.SendBeginMainFrameToCommit.LayoutUpdate",
base::TimeDelta::FromMicroseconds(7).InMilliseconds(), 1);
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"CompositorLatency.SendBeginMainFrameToCommit.Prepaint", 6, 1); "CompositorLatency.SendBeginMainFrameToCommit.Prepaint",
base::TimeDelta::FromMicroseconds(6).InMilliseconds(), 1);
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"CompositorLatency.SendBeginMainFrameToCommit.Composite", 5, 1); "CompositorLatency.SendBeginMainFrameToCommit.Composite",
base::TimeDelta::FromMicroseconds(5).InMilliseconds(), 1);
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"CompositorLatency.SendBeginMainFrameToCommit.Paint", 4, 1); "CompositorLatency.SendBeginMainFrameToCommit.Paint",
base::TimeDelta::FromMicroseconds(4).InMilliseconds(), 1);
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"CompositorLatency.SendBeginMainFrameToCommit.ScrollingCoordinator", 3, "CompositorLatency.SendBeginMainFrameToCommit.ScrollingCoordinator",
1); base::TimeDelta::FromMicroseconds(3).InMilliseconds(), 1);
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"CompositorLatency.SendBeginMainFrameToCommit.CompositeCommit", 2, 1); "CompositorLatency.SendBeginMainFrameToCommit.CompositeCommit",
base::TimeDelta::FromMicroseconds(2).InMilliseconds(), 1);
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"CompositorLatency.SendBeginMainFrameToCommit.UpdateLayers", 1, 1); "CompositorLatency.SendBeginMainFrameToCommit.UpdateLayers",
base::TimeDelta::FromMicroseconds(1).InMilliseconds(), 1);
histogram_tester.ExpectTotalCount( histogram_tester.ExpectTotalCount(
"CompositorLatency.SendBeginMainFrameToCommit.BeginMainSentToStarted", 1); "CompositorLatency.SendBeginMainFrameToCommit.BeginMainSentToStarted", 1);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment