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

Unified Diff: cc/scheduler/delay_based_time_source.cc

Issue 18589002: cc: Fix and simplify DelayBasedTimeSource (Closed) Base URL: http://git.chromium.org/chromium/src.git@master
Patch Set: Use instant trace events Created 7 years, 6 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
« no previous file with comments | « cc/scheduler/delay_based_time_source.h ('k') | cc/scheduler/delay_based_time_source_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: cc/scheduler/delay_based_time_source.cc
diff --git a/cc/scheduler/delay_based_time_source.cc b/cc/scheduler/delay_based_time_source.cc
index 5d2f20bee6fd41695b9bcf84d85bd2ca50138163..6038a3bfc29b3142f6d079811bc4473d78f53dee 100644
--- a/cc/scheduler/delay_based_time_source.cc
+++ b/cc/scheduler/delay_based_time_source.cc
@@ -15,10 +15,10 @@ namespace cc {
namespace {
-// kDoubleTickThreshold prevents ticks from running within the specified
+// kDoubleTickDivisor prevents ticks from running within the specified
// fraction of an interval. This helps account for jitter in the timebase as
// well as quick timer reactivation.
-static const double kDoubleTickThreshold = 0.25;
+static const int kDoubleTickDivisor = 4;
// kIntervalChangeThreshold is the fraction of the interval that will trigger an
// immediate interval change. kPhaseChangeThreshold is the fraction of the
@@ -40,10 +40,10 @@ scoped_refptr<DelayBasedTimeSource> DelayBasedTimeSource::Create(
DelayBasedTimeSource::DelayBasedTimeSource(
base::TimeDelta interval, base::SingleThreadTaskRunner* task_runner)
: client_(NULL),
- has_tick_target_(false),
+ last_tick_time_(base::TimeTicks() - interval),
current_parameters_(interval, base::TimeTicks()),
next_parameters_(interval, base::TimeTicks()),
- state_(STATE_INACTIVE),
+ active_(false),
task_runner_(task_runner),
weak_factory_(this) {}
@@ -51,32 +51,19 @@ DelayBasedTimeSource::~DelayBasedTimeSource() {}
void DelayBasedTimeSource::SetActive(bool active) {
TRACE_EVENT1("cc", "DelayBasedTimeSource::SetActive", "active", active);
- if (!active) {
- state_ = STATE_INACTIVE;
- weak_factory_.InvalidateWeakPtrs();
+ if (active == active_)
return;
- }
+ active_ = active;
- if (state_ == STATE_STARTING || state_ == STATE_ACTIVE)
- return;
-
- if (!has_tick_target_) {
- // Becoming active the first time is deferred: we post a 0-delay task.
- // When it runs, we use that to establish the timebase, become truly
- // active, and fire the first tick.
- state_ = STATE_STARTING;
- task_runner_->PostTask(FROM_HERE,
- base::Bind(&DelayBasedTimeSource::OnTimerFired,
- weak_factory_.GetWeakPtr()));
+ if (!active_) {
+ weak_factory_.InvalidateWeakPtrs();
return;
}
- state_ = STATE_ACTIVE;
-
PostNextTickTask(Now());
}
-bool DelayBasedTimeSource::Active() const { return state_ != STATE_INACTIVE; }
+bool DelayBasedTimeSource::Active() const { return active_; }
base::TimeTicks DelayBasedTimeSource::LastTickTime() { return last_tick_time_; }
@@ -85,17 +72,11 @@ base::TimeTicks DelayBasedTimeSource::NextTickTime() {
}
void DelayBasedTimeSource::OnTimerFired() {
- DCHECK(state_ != STATE_INACTIVE);
-
- base::TimeTicks now = this->Now();
- last_tick_time_ = now;
+ DCHECK(active_);
- if (state_ == STATE_STARTING) {
- SetTimebaseAndInterval(now, current_parameters_.interval);
- state_ = STATE_ACTIVE;
- }
+ last_tick_time_ = current_parameters_.tick_target;
- PostNextTickTask(now);
+ PostNextTickTask(Now());
// Fire the tick.
if (client_)
@@ -110,9 +91,8 @@ void DelayBasedTimeSource::SetTimebaseAndInterval(base::TimeTicks timebase,
base::TimeDelta interval) {
next_parameters_.interval = interval;
next_parameters_.tick_target = timebase;
- has_tick_target_ = true;
- if (state_ != STATE_ACTIVE) {
+ if (!active_) {
// If we aren't active, there's no need to reset the timer.
return;
}
@@ -123,6 +103,8 @@ void DelayBasedTimeSource::SetTimebaseAndInterval(base::TimeTicks timebase,
std::abs((interval - current_parameters_.interval).InSecondsF());
double interval_change = interval_delta / interval.InSecondsF();
if (interval_change > kIntervalChangeThreshold) {
+ TRACE_EVENT_INSTANT0("cc", "DelayBasedTimeSource::IntervalChanged",
+ TRACE_EVENT_SCOPE_THREAD);
SetActive(false);
SetActive(true);
return;
@@ -140,6 +122,8 @@ void DelayBasedTimeSource::SetTimebaseAndInterval(base::TimeTicks timebase,
fmod(target_delta, interval.InSecondsF()) / interval.InSecondsF();
if (phase_change > kPhaseChangeThreshold &&
phase_change < (1.0 - kPhaseChangeThreshold)) {
+ TRACE_EVENT_INSTANT0("cc", "DelayBasedTimeSource::PhaseChanged",
+ TRACE_EVENT_SCOPE_THREAD);
SetActive(false);
SetActive(true);
return;
@@ -205,26 +189,24 @@ base::TimeTicks DelayBasedTimeSource::Now() const {
// now=37 tick_target=16.667 new_target=50.000 -->
// tick(), PostDelayedTask(floor(50.000-37)) --> PostDelayedTask(13)
base::TimeTicks DelayBasedTimeSource::NextTickTarget(base::TimeTicks now) {
+ const base::TimeDelta epsilon(base::TimeDelta::FromMicroseconds(1));
base::TimeDelta new_interval = next_parameters_.interval;
int intervals_elapsed =
- static_cast<int>(floor((now - next_parameters_.tick_target).InSecondsF() /
- new_interval.InSecondsF()));
- base::TimeTicks last_effective_tick =
+ (now - next_parameters_.tick_target + new_interval - epsilon) /
+ new_interval;
+ base::TimeTicks new_tick_target =
next_parameters_.tick_target + new_interval * intervals_elapsed;
- base::TimeTicks new_tick_target = last_effective_tick + new_interval;
- DCHECK(now < new_tick_target)
+ DCHECK(now <= new_tick_target)
<< "now = " << now.ToInternalValue()
<< "; new_tick_target = " << new_tick_target.ToInternalValue()
<< "; new_interval = " << new_interval.InMicroseconds()
<< "; tick_target = " << next_parameters_.tick_target.ToInternalValue()
- << "; intervals_elapsed = " << intervals_elapsed
- << "; last_effective_tick = " << last_effective_tick.ToInternalValue();
+ << "; intervals_elapsed = " << intervals_elapsed;
// Avoid double ticks when:
// 1) Turning off the timer and turning it right back on.
// 2) Jittery data is passed to SetTimebaseAndInterval().
- if (new_tick_target - last_tick_time_ <=
- new_interval / static_cast<int>(1.0 / kDoubleTickThreshold))
+ if (new_tick_target - last_tick_time_ <= new_interval / kDoubleTickDivisor)
new_tick_target += new_interval;
return new_tick_target;
@@ -234,10 +216,9 @@ void DelayBasedTimeSource::PostNextTickTask(base::TimeTicks now) {
base::TimeTicks new_tick_target = NextTickTarget(now);
// Post another task *before* the tick and update state
- base::TimeDelta delay = new_tick_target - now;
- DCHECK(delay.InMillisecondsF() <=
- next_parameters_.interval.InMillisecondsF() *
- (1.0 + kDoubleTickThreshold));
+ base::TimeDelta delay;
+ if (now <= new_tick_target)
+ delay = new_tick_target - now;
task_runner_->PostDelayedTask(FROM_HERE,
base::Bind(&DelayBasedTimeSource::OnTimerFired,
weak_factory_.GetWeakPtr()),
« no previous file with comments | « cc/scheduler/delay_based_time_source.h ('k') | cc/scheduler/delay_based_time_source_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698