Modified: trunk/Source/WebCore/ChangeLog (103069 => 103070)
--- trunk/Source/WebCore/ChangeLog 2011-12-16 14:58:49 UTC (rev 103069)
+++ trunk/Source/WebCore/ChangeLog 2011-12-16 16:31:34 UTC (rev 103070)
@@ -1,3 +1,20 @@
+2011-12-14 Nat Duca <nd...@chromium.org>
+
+ [chromium] DelayBasedTimeSource should not change its timebase on late ticks
+ https://bugs.webkit.org/show_bug.cgi?id=74573
+
+ The original DelayBasedTimeSource was designed to shift its timebase
+ to the tick time when a tick came back "late." The rationale was that it is
+ better to just "start fresh" after a stutter. After profiling this,
+ this time-rebasing just destabilizes frame rate anytime the thread gets
+ loaded. This patch keeps the timebase stationary, leading to vastly
+ smoother framerates when the message loop is under load.
+
+ Reviewed by James Robinson.
+
+ * platform/graphics/chromium/cc/CCDelayBasedTimeSource.cpp:
+ (WebCore::CCDelayBasedTimeSource::updateState):
+
2011-12-16 Sheriff Bot <webkit.review....@gmail.com>
Unreviewed, rolling out r103062.
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCDelayBasedTimeSource.cpp (103069 => 103070)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCDelayBasedTimeSource.cpp 2011-12-16 14:58:49 UTC (rev 103069)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCDelayBasedTimeSource.cpp 2011-12-16 16:31:34 UTC (rev 103070)
@@ -29,6 +29,7 @@
#include "cc/CCThread.h"
#include "cc/CCThreadTask.h"
#include <wtf/CurrentTime.h>
+#include <wtf/MathExtras.h>
namespace WebCore {
@@ -121,11 +122,8 @@
// now=18 tickTarget=16.667 newTarget=33.333 --> tick(), postDelayedTask(floor(33.333-18)) --> postDelayedTask(15)
// This brings us back to 18+15 = 33, which was where we would have been if the task hadn't been late.
//
-// For the really late delay, we fire a tick immediately reset the timebase from the current tim. E.g.:
-// now=37 tickTarget=16.667 newTarget=53.6667 --> tick(), postDelayedTask(floor(53.667-37)) --> postDelayedTask(16)
-//
-// Here we realize we're more than a tick late. We adjust newTarget to be 16.667 from now, and post a task off that new
-// target.
+// For the really late delay, we we move to the next logical tick. The timebase is not reset.
+// now=37 tickTarget=16.667 newTarget=50.000 --> tick(), postDelayedTask(floor(50.000-37)) --> postDelayedTask(13)
void CCDelayBasedTimeSource::updateState()
{
if (m_state == STATE_INACTIVE)
@@ -138,18 +136,19 @@
m_state = STATE_ACTIVE;
}
- const double maxLateBeforeResettingTimebase = 5.0;
+ int numIntervalsElapsed = static_cast<int>(floor((now - m_tickTarget) / m_intervalMs));
+ double lastEffectiveTick = m_tickTarget + m_intervalMs * numIntervalsElapsed;
+ double newTickTarget = lastEffectiveTick + m_intervalMs;
- double newTickTarget = 0;
- double amountLate = now - m_tickTarget;
- if (amountLate <= maxLateBeforeResettingTimebase)
- newTickTarget = m_tickTarget + m_intervalMs;
- else
- newTickTarget = now + m_intervalMs;
+ long long delay = static_cast<long long>(newTickTarget - now);
+ if (!delay) {
+ newTickTarget = newTickTarget + m_intervalMs;
+ delay = static_cast<long long>(newTickTarget - now);
+ }
// Post another task *before* the tick and update state
ASSERT(newTickTarget > now);
- postTickTask(static_cast<long long>(newTickTarget - now));
+ postTickTask(delay);
m_tickTarget = newTickTarget;
// Fire the tick
Modified: trunk/Source/WebKit/chromium/tests/CCDelayBasedTimeSourceTest.cpp (103069 => 103070)
--- trunk/Source/WebKit/chromium/tests/CCDelayBasedTimeSourceTest.cpp 2011-12-16 14:58:49 UTC (rev 103069)
+++ trunk/Source/WebKit/chromium/tests/CCDelayBasedTimeSourceTest.cpp 2011-12-16 16:31:34 UTC (rev 103070)
@@ -92,6 +92,112 @@
EXPECT_FALSE(thread.hasPendingTask());
}
+// At 60Hz, when the tick returns at exactly the requested next time, make sure
+// a 16ms next delay is posted.
+TEST(CCDelayBasedTimeSource, NextDelaySaneWhenExactlyOnRequestedTime)
+{
+ FakeCCThread thread;
+ FakeCCTimeSourceClient client;
+ double interval = 1000.0 / 60.0;
+ RefPtr<FakeCCDelayBasedTimeSource> timer = FakeCCDelayBasedTimeSource::create(interval, &thread);
+ timer->setClient(&client);
+ timer->setActive(true);
+ // Run the first task, as that activates the timer and picks up a timebase.
+ thread.runPendingTask();
+
+ EXPECT_EQ(16, thread.pendingDelay());
+
+ timer->setMonotonicallyIncreasingTimeMs(interval);
+ thread.runPendingTask();
+
+ EXPECT_EQ(16, thread.pendingDelay());
+}
+
+// At 60Hz, when the tick returns at slightly after the requested next time, make sure
+// a 16ms next delay is posted.
+TEST(CCDelayBasedTimeSource, NextDelaySaneWhenSlightlyAfterRequestedTime)
+{
+ FakeCCThread thread;
+ FakeCCTimeSourceClient client;
+ double interval = 1000.0 / 60.0;
+ RefPtr<FakeCCDelayBasedTimeSource> timer = FakeCCDelayBasedTimeSource::create(interval, &thread);
+ timer->setClient(&client);
+ timer->setActive(true);
+ // Run the first task, as that activates the timer and picks up a timebase.
+ thread.runPendingTask();
+
+ EXPECT_EQ(16, thread.pendingDelay());
+
+ timer->setMonotonicallyIncreasingTimeMs(interval + 0.0001);
+ thread.runPendingTask();
+
+ EXPECT_EQ(16, thread.pendingDelay());
+}
+
+// At 60Hz, when the tick returns at exactly 2*interval after the requested next time, make sure
+// a 16ms next delay is posted.
+TEST(CCDelayBasedTimeSource, NextDelaySaneWhenExactlyTwiceAfterRequestedTime)
+{
+ FakeCCThread thread;
+ FakeCCTimeSourceClient client;
+ double interval = 1000.0 / 60.0;
+ RefPtr<FakeCCDelayBasedTimeSource> timer = FakeCCDelayBasedTimeSource::create(interval, &thread);
+ timer->setClient(&client);
+ timer->setActive(true);
+ // Run the first task, as that activates the timer and picks up a timebase.
+ thread.runPendingTask();
+
+ EXPECT_EQ(16, thread.pendingDelay());
+
+ timer->setMonotonicallyIncreasingTimeMs(2*interval);
+ thread.runPendingTask();
+
+ EXPECT_EQ(16, thread.pendingDelay());
+}
+
+// At 60Hz, when the tick returns at 2*interval and a bit after the requested next time, make sure
+// a 16ms next delay is posted.
+TEST(CCDelayBasedTimeSource, NextDelaySaneWhenSlightlyAfterTwiceRequestedTime)
+{
+ FakeCCThread thread;
+ FakeCCTimeSourceClient client;
+ double interval = 1000.0 / 60.0;
+ RefPtr<FakeCCDelayBasedTimeSource> timer = FakeCCDelayBasedTimeSource::create(interval, &thread);
+ timer->setClient(&client);
+ timer->setActive(true);
+ // Run the first task, as that activates the timer and picks up a timebase.
+ thread.runPendingTask();
+
+ EXPECT_EQ(16, thread.pendingDelay());
+
+ timer->setMonotonicallyIncreasingTimeMs(2*interval + 0.0001);
+ thread.runPendingTask();
+
+ EXPECT_EQ(16, thread.pendingDelay());
+}
+
+// At 60Hz, when the tick returns halfway to the next frame time, make sure
+// a correct next delay value is posted.
+TEST(CCDelayBasedTimeSource, NextDelaySaneWhenHalfAfterRequestedTime)
+{
+ FakeCCThread thread;
+ FakeCCTimeSourceClient client;
+ double interval = 1000.0 / 60.0;
+ RefPtr<FakeCCDelayBasedTimeSource> timer = FakeCCDelayBasedTimeSource::create(interval, &thread);
+ timer->setClient(&client);
+ timer->setActive(true);
+ // Run the first task, as that activates the timer and picks up a timebase.
+ thread.runPendingTask();
+
+ EXPECT_EQ(16, thread.pendingDelay());
+
+ timer->setMonotonicallyIncreasingTimeMs(interval + interval * 0.5);
+ thread.runPendingTask();
+
+ EXPECT_EQ(8, thread.pendingDelay());
+}
+
+
TEST(CCDelayBasedTimeSourceTest, AchievesTargetRateWithNoNoise)
{
int numIterations = 1000;
