Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3394)

Unified Diff: cc/scheduler/scheduler.cc

Issue 2158023005: cc: Remove retro frame logic from scheduler. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: cc/scheduler/scheduler.cc
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index 4fa133cb3538e608f32053b216d68aef12fdcfc4..8c744074c785e219434ebb68da0d155fbea20c4f 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -64,8 +64,6 @@ Scheduler::Scheduler(
DCHECK(client_);
DCHECK(!state_machine_.BeginFrameNeeded());
- begin_retro_frame_closure_ =
- base::Bind(&Scheduler::BeginRetroFrame, weak_factory_.GetWeakPtr());
begin_impl_frame_deadline_closure_ = base::Bind(
&Scheduler::OnBeginImplFrameDeadline, weak_factory_.GetWeakPtr());
@@ -200,8 +198,6 @@ void Scheduler::DidPrepareTiles() {
void Scheduler::DidLoseOutputSurface() {
TRACE_EVENT0("cc", "Scheduler::DidLoseOutputSurface");
- begin_retro_frame_args_.clear();
- begin_retro_frame_task_.Cancel();
state_machine_.DidLoseOutputSurface();
UpdateCompositorTimingHistoryRecordingEnabled();
ProcessScheduledActions();
@@ -251,6 +247,7 @@ void Scheduler::SetupNextBeginFrameIfNeeded() {
} else if (state_machine_.begin_impl_frame_state() ==
SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE) {
// Call RemoveObserver in between frames only.
+ missed_begin_frame_task_.Cancel();
observing_begin_frame_source_ = false;
if (begin_frame_source_)
begin_frame_source_->RemoveObserver(this);
@@ -259,8 +256,6 @@ void Scheduler::SetupNextBeginFrameIfNeeded() {
false);
}
}
-
- PostBeginRetroFrameIfNeeded();
}
void Scheduler::OnBeginFrameSourcePausedChanged(bool paused) {
@@ -274,11 +269,21 @@ void Scheduler::OnBeginFrameSourcePausedChanged(bool paused) {
// BeginFrame is the mechanism that tells us that now is a good time to start
// making a frame. Usually this means that user input for the frame is complete.
-// If the scheduler is busy, we queue the BeginFrame to be handled later as
-// a BeginRetroFrame.
+// If the scheduler is busy, we finish the previous frame before starting a new
+// one.
bool Scheduler::OnBeginFrameDerivedImpl(const BeginFrameArgs& args) {
TRACE_EVENT1("cc,benchmark", "Scheduler::BeginFrame", "args", args.AsValue());
+ if (state_machine_.begin_impl_frame_state() !=
+ SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE) {
+ OnBeginImplFrameDeadline();
+ }
+
+ if (!observing_begin_frame_source_) {
+ DCHECK(!state_machine_.BeginFrameNeeded());
+ return false;
+ }
+
// Trace this begin frame time through the Chrome stack
TRACE_EVENT_FLOW_BEGIN0(
TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler.frames"), "BeginFrameArgs",
@@ -290,31 +295,12 @@ bool Scheduler::OnBeginFrameDerivedImpl(const BeginFrameArgs& args) {
if (settings_.using_synchronous_renderer_compositor) {
BeginImplFrameSynchronous(adjusted_args);
- return true;
- }
-
- // We have just called SetNeedsBeginFrame(true) and the BeginFrameSource has
- // sent us the last BeginFrame we have missed. As we might not be able to
- // actually make rendering for this call, handle it like a "retro frame".
- // TODO(brainderson): Add a test for this functionality ASAP!
- if (adjusted_args.type == BeginFrameArgs::MISSED) {
- begin_retro_frame_args_.push_back(adjusted_args);
- PostBeginRetroFrameIfNeeded();
- return true;
- }
-
- bool should_defer_begin_frame =
- !begin_retro_frame_args_.empty() ||
- !begin_retro_frame_task_.IsCancelled() ||
- !observing_begin_frame_source_ ||
- (state_machine_.begin_impl_frame_state() !=
- SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE);
-
- if (should_defer_begin_frame) {
- begin_retro_frame_args_.push_back(adjusted_args);
- TRACE_EVENT_INSTANT0("cc", "Scheduler::BeginFrame deferred",
- TRACE_EVENT_SCOPE_THREAD);
- // Queuing the frame counts as "using it", so we need to return true.
+ } else if (args.type == BeginFrameArgs::MISSED) {
+ DCHECK(missed_begin_frame_task_.IsCancelled());
+ missed_begin_frame_task_.Reset(
+ base::Bind(&Scheduler::BeginImplFrameWithDeadline,
+ weak_factory_.GetWeakPtr(), adjusted_args));
+ task_runner_->PostTask(FROM_HERE, missed_begin_frame_task_.callback());
} else {
BeginImplFrameWithDeadline(adjusted_args);
}
@@ -341,78 +327,9 @@ void Scheduler::OnDrawForOutputSurface(bool resourceless_software_draw) {
state_machine_.SetResourcelessSoftwareDraw(false);
}
-// BeginRetroFrame is called for BeginFrames that we've deferred because
-// the scheduler was in the middle of processing a previous BeginFrame.
-void Scheduler::BeginRetroFrame() {
- TRACE_EVENT0("cc,benchmark", "Scheduler::BeginRetroFrame");
- DCHECK(!settings_.using_synchronous_renderer_compositor);
- DCHECK(!begin_retro_frame_args_.empty());
- DCHECK(!begin_retro_frame_task_.IsCancelled());
- DCHECK_EQ(state_machine_.begin_impl_frame_state(),
- SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE);
-
- begin_retro_frame_task_.Cancel();
-
- // Discard expired BeginRetroFrames
- // Today, we should always end up with at most one un-expired BeginRetroFrame
- // because deadlines will not be greater than the next frame time. We don't
- // DCHECK though because some systems don't always have monotonic timestamps.
- // TODO(brianderson): In the future, long deadlines could result in us not
- // draining the queue if we don't catch up. If we consistently can't catch
- // up, our fallback should be to lower our frame rate.
- base::TimeTicks now = Now();
-
- while (!begin_retro_frame_args_.empty()) {
- const BeginFrameArgs& args = begin_retro_frame_args_.front();
- base::TimeTicks expiration_time = args.deadline;
- if (now <= expiration_time)
- break;
- TRACE_EVENT_INSTANT2(
- "cc", "Scheduler::BeginRetroFrame discarding", TRACE_EVENT_SCOPE_THREAD,
- "expiration_time - now", (expiration_time - now).InMillisecondsF(),
- "BeginFrameArgs", begin_retro_frame_args_.front().AsValue());
- begin_retro_frame_args_.pop_front();
- if (begin_frame_source_)
- begin_frame_source_->DidFinishFrame(this, begin_retro_frame_args_.size());
- }
-
- if (begin_retro_frame_args_.empty()) {
- TRACE_EVENT_INSTANT0("cc", "Scheduler::BeginRetroFrames all expired",
- TRACE_EVENT_SCOPE_THREAD);
- } else {
- BeginFrameArgs front = begin_retro_frame_args_.front();
- begin_retro_frame_args_.pop_front();
- BeginImplFrameWithDeadline(front);
- }
-}
-
-// There could be a race between the posted BeginRetroFrame and a new
-// BeginFrame arriving via the normal mechanism. Scheduler::BeginFrame
-// will check if there is a pending BeginRetroFrame to ensure we handle
-// BeginFrames in FIFO order.
-void Scheduler::PostBeginRetroFrameIfNeeded() {
- TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler"),
- "Scheduler::PostBeginRetroFrameIfNeeded", "state", AsValue());
- if (!observing_begin_frame_source_)
- return;
-
- if (begin_retro_frame_args_.empty() || !begin_retro_frame_task_.IsCancelled())
- return;
-
- // begin_retro_frame_args_ should always be empty for the
- // synchronous compositor.
- DCHECK(!settings_.using_synchronous_renderer_compositor);
-
- if (state_machine_.begin_impl_frame_state() !=
- SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_IDLE)
- return;
-
- begin_retro_frame_task_.Reset(begin_retro_frame_closure_);
-
- task_runner_->PostTask(FROM_HERE, begin_retro_frame_task_.callback());
-}
-
void Scheduler::BeginImplFrameWithDeadline(const BeginFrameArgs& args) {
+ missed_begin_frame_task_.Cancel();
+
bool main_thread_is_in_high_latency_mode =
state_machine_.main_thread_missed_last_deadline();
TRACE_EVENT2("cc,benchmark", "Scheduler::BeginImplFrame", "args",
@@ -464,7 +381,7 @@ void Scheduler::BeginImplFrameWithDeadline(const BeginFrameArgs& args) {
TRACE_EVENT_INSTANT0("cc", "SkipBeginImplFrameToReduceLatency",
TRACE_EVENT_SCOPE_THREAD);
if (begin_frame_source_)
- begin_frame_source_->DidFinishFrame(this, begin_retro_frame_args_.size());
+ begin_frame_source_->DidFinishFrame(this, 0);
return;
}
@@ -493,7 +410,7 @@ void Scheduler::FinishImplFrame() {
client_->DidFinishImplFrame();
if (begin_frame_source_)
- begin_frame_source_->DidFinishFrame(this, begin_retro_frame_args_.size());
+ begin_frame_source_->DidFinishFrame(this, 0);
begin_impl_frame_tracker_.Finish();
}
@@ -738,10 +655,6 @@ Scheduler::AsValue() const {
estimated_parent_draw_time_.InMillisecondsF());
state->SetBoolean("observing_begin_frame_source",
observing_begin_frame_source_);
- state->SetInteger("begin_retro_frame_args",
- static_cast<int>(begin_retro_frame_args_.size()));
- state->SetBoolean("begin_retro_frame_task",
- !begin_retro_frame_task_.IsCancelled());
state->SetBoolean("begin_impl_frame_deadline_task",
!begin_impl_frame_deadline_task_.IsCancelled());
state->SetString("inside_action",

Powered by Google App Engine
This is Rietveld 408576698