Changeset 240944 in webkit

Timestamp:

Feb 4, 2019, 2:30:31 PM (8 years ago)

Author:

benjamin@webkit.org

Message:

Use deferrable timer to restart the Responsiveness Timer on each wheel event
​https://bugs.webkit.org/show_bug.cgi?id=194135
Source/WebCore:

<rdar://problem/47724099>

Reviewed by Simon Fraser.

The original DeferrableOneShotTimer was not really deferrable.
What it allows is to restart the count down from scratch after
firing.

For this optimization, I want to keep the correct timing but avoid
starting a real timer every time.

I renamed DeferrableOneShotTimer to ResettableOneShotTimer and
created a real DeferrableOneShotTimer that support deadlines.

• css/CSSImageGeneratorValue.cpp:
• html/HTMLPlugInImageElement.h:
• loader/cache/CachedResource.h:
• platform/Timer.cpp:

(WebCore::DeferrableOneShotTimer::startOneShot):
(WebCore::DeferrableOneShotTimer::fired):

• platform/Timer.h:

(WebCore::TimerBase::nextFireTime const):
(WebCore::ResettableOneShotTimer::ResettableOneShotTimer):
(WebCore::DeferrableOneShotTimer::DeferrableOneShotTimer):
(WebCore::DeferrableOneShotTimer::stop):
(WebCore::DeferrableOneShotTimer::restart): Deleted.

• platform/graphics/ca/TileController.h:
• platform/graphics/cg/SubimageCacheWithTimer.h:

Source/WebKit:

Reviewed by Simon Fraser.

Simon Fraser suggested a neat improvement over my previous optimization
of ResponsivenessTimer.

Instead of reseting the deadline with every event, we can let the timer
fire and add the missing time from the last start.

I implemented that behavior in the new Deferrable Timer class and use
it from ResponsivenessTimer.

• NetworkProcess/watchos/NetworkProximityAssertion.h:
• UIProcess/ResponsivenessTimer.h:
• WebProcess/Plugins/PluginView.h:

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/css/CSSImageGeneratorValue.cpp (modified) (1 diff)
• WebCore/html/HTMLPlugInImageElement.h (modified) (1 diff)
• WebCore/loader/cache/CachedResource.h (modified) (1 diff)
• WebCore/platform/Timer.cpp (modified) (5 diffs)
• WebCore/platform/Timer.h (modified) (5 diffs)
• WebCore/platform/graphics/ca/TileController.h (modified) (1 diff)
• WebCore/platform/graphics/cg/SubimageCacheWithTimer.h (modified) (1 diff)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/NetworkProcess/watchos/NetworkProximityAssertion.h (modified) (1 diff)
• WebKit/UIProcess/ResponsivenessTimer.cpp (modified) (1 diff)
• WebKit/UIProcess/ResponsivenessTimer.h (modified) (1 diff)
• WebKit/WebProcess/Plugins/PluginView.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-02-04  Benjamin Poulain  <benjamin@webkit.org>
+
+        Use deferrable timer to restart the Responsiveness Timer on each wheel event
+        https://bugs.webkit.org/show_bug.cgi?id=194135
+        <rdar://problem/47724099>
+
+        Reviewed by Simon Fraser.
+
+        The original DeferrableOneShotTimer was not really deferrable.
+        What it allows is to restart the count down from scratch after
+        firing.
+
+        For this optimization, I want to keep the correct timing but avoid
+        starting a real timer every time.
+
+        I renamed DeferrableOneShotTimer to ResettableOneShotTimer and
+        created a real DeferrableOneShotTimer that support deadlines.
+
+        * css/CSSImageGeneratorValue.cpp:
+        * html/HTMLPlugInImageElement.h:
+        * loader/cache/CachedResource.h:
+        * platform/Timer.cpp:
+        (WebCore::DeferrableOneShotTimer::startOneShot):
+        (WebCore::DeferrableOneShotTimer::fired):
+        * platform/Timer.h:
+        (WebCore::TimerBase::nextFireTime const):
+        (WebCore::ResettableOneShotTimer::ResettableOneShotTimer):
+        (WebCore::DeferrableOneShotTimer::DeferrableOneShotTimer):
+        (WebCore::DeferrableOneShotTimer::stop):
+        (WebCore::DeferrableOneShotTimer::restart): Deleted.
+        * platform/graphics/ca/TileController.h:
+        * platform/graphics/cg/SubimageCacheWithTimer.h:
+
 2019-02-04  Antoine Quint  <graouts@apple.com>
 

trunk/Source/WebCore/css/CSSImageGeneratorValue.cpp

@@ -57 +57 @@
     const FloatSize m_size;
     const Ref<GeneratedImage> m_image;
-    DeferrableOneShotTimer m_evictionTimer;
+    ResettableOneShotTimer m_evictionTimer;
 };
 

trunk/Source/WebCore/html/HTMLPlugInImageElement.h

@@ -131 +131 @@
     bool m_needsDocumentActivationCallbacks { false };
     RefPtr<MouseEvent> m_pendingClickEventFromSnapshot;
-    DeferrableOneShotTimer m_simulatedMouseClickTimer;
+    ResettableOneShotTimer m_simulatedMouseClickTimer;
     Timer m_removeSnapshotTimer;
     RefPtr<Image> m_snapshotImage;

trunk/Source/WebCore/loader/cache/CachedResource.h

@@ -312 +312 @@
     ResourceResponse m_response;
 
-    DeferrableOneShotTimer m_decodedDataDeletionTimer;
+    ResettableOneShotTimer m_decodedDataDeletionTimer;
 
     // FIXME: Make the rest of these data members private and use functions in derived classes instead.

trunk/Source/WebCore/platform/Timer.cpp

@@ -261 +261 @@
 TimerBase::~TimerBase()
 {
-    ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
+    assertThreadSafety();
     RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()) || shouldSuppressThreadSafetyCheck());
     stop();
@@ -274 +274 @@
 void TimerBase::start(Seconds nextFireInterval, Seconds repeatInterval)
 {
-    ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
+    assertThreadSafety();
 
     m_repeatInterval = repeatInterval;
@@ -282 +282 @@
 void TimerBase::stop()
 {
-    ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
+    assertThreadSafety();
 
     m_repeatInterval = 0_s;
@@ -466 +466 @@
 void TimerBase::setNextFireTime(MonotonicTime newTime)
 {
-    ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
+    assertThreadSafety();
     RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()) || shouldSuppressThreadSafetyCheck());
     RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!m_wasDeleted);
@@ -521 +521 @@
 }
 
+DeferrableOneShotTimer::DeferrableOneShotTimer(WTF::Function<void()>&& function)
+    : m_function(WTFMove(function))
+{
+}
+
+DeferrableOneShotTimer::~DeferrableOneShotTimer() = default;
+
+void DeferrableOneShotTimer::startOneShot(Seconds interval)
+{
+    assertThreadSafety();
+
+    MonotonicTime oldNextFireTime = TimerBase::nextFireTime();
+    if (!oldNextFireTime) {
+        m_restartFireTime = MonotonicTime();
+        TimerBase::startOneShot(interval);
+        return;
+    }
+
+    MonotonicTime newNextFireTime = MonotonicTime::now() + interval;
+    if (newNextFireTime < oldNextFireTime) {
+        m_restartFireTime = MonotonicTime();
+        TimerBase::setNextFireTime(newNextFireTime);
+        return;
+    }
+
+    m_restartFireTime = newNextFireTime;
+}
+
+void DeferrableOneShotTimer::stop()
+{
+    TimerBase::stop();
+}
+
+void DeferrableOneShotTimer::fired()
+{
+    if (m_restartFireTime) {
+        MonotonicTime now = MonotonicTime::now();
+        MonotonicTime restartFireTime = m_restartFireTime;
+        m_restartFireTime = MonotonicTime();
+        if (now < restartFireTime) {
+            TimerBase::setNextFireTime(restartFireTime);
+            return;
+        }
+    }
+
+    m_function();
+}
+
 } // namespace WebCore
 

trunk/Source/WebCore/platform/Timer.h

@@ -69 +69 @@
     static void fireTimersInNestedEventLoop();
 
+protected:
+    MonotonicTime nextFireTime() const { return m_heapItem ? m_heapItem->time : MonotonicTime { }; }
+    void setNextFireTime(MonotonicTime);
+
+    void assertThreadSafety()
+    {
+        ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
+    }
+
 private:
     virtual void fired() = 0;
@@ -76 +85 @@
     void checkConsistency() const;
     void checkHeapIndex() const;
-
-    void setNextFireTime(MonotonicTime);
 
     bool inHeap() const { return m_heapItem && m_heapItem->isInHeap(); }
@@ -92 +99 @@
     void heapPopMin();
     static void heapDeleteNullMin(ThreadTimerHeap&);
-
-    MonotonicTime nextFireTime() const { return m_heapItem ? m_heapItem->time : MonotonicTime { }; }
 
     MonotonicTime m_unalignedNextFireTime; // m_nextFireTime not considering alignment interval
@@ -142 +147 @@
 }
 
-class DeferrableOneShotTimer : protected TimerBase {
+// ResettableOneShotTimer is a optimization for timers that need to be delayed frequently.
+//
+// Changing the deadline of a timer is not a cheap operation. When it is done frequently, it can
+// affect performance.
+//
+// With ResettableOneShotTimer, calling restart() does not change the underlying timer.
+// Instead, when the underlying timer fires, a new dealine is set for "delay" seconds.
+//
+// When a ResettableOneShotTimer of 5 seconds is restarted before the deadline, the total
+// time before the function is called is 10 seconds.
+//
+// If a timer is unfrequently reset, or if it is usually stopped before the deadline,
+// prefer WebCore::Timer to avoid idle wake-up.
+class ResettableOneShotTimer : protected TimerBase {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     template<typename TimerFiredClass>
-    DeferrableOneShotTimer(TimerFiredClass& object, void (TimerFiredClass::*function)(), Seconds delay)
-        : DeferrableOneShotTimer(std::bind(function, &object), delay)
-    {
-    }
-
-    DeferrableOneShotTimer(WTF::Function<void ()>&& function, Seconds delay)
+    ResettableOneShotTimer(TimerFiredClass& object, void (TimerFiredClass::*function)(), Seconds delay)
+        : ResettableOneShotTimer(std::bind(function, &object), delay)
+    {
+    }
+
+    ResettableOneShotTimer(WTF::Function<void()>&& function, Seconds delay)
         : m_function(WTFMove(function))
         , m_delay(delay)
@@ -197 +215 @@
 };
 
+// DeferrableOneShotTimer is a optimization for timers that need to change the deadline frequently.
+//
+// With DeferrableOneShotTimer, if the new deadline is later than the original deadline, the timer
+// is not reset. Instead, the timer fires and schedule a new timer for the remaining time.
+//
+// DeferrableOneShotTimer supports absolute deadlines.
+// If a timer of 5 seconds is restarted after 2 seconds, the total time will be 7 seconds.
+//
+// Restarting a DeferrableOneShotTimer is more expensive than restarting a ResettableOneShotTimer.
+// If accumulating the delay is not important, prefer ResettableOneShotTimer.
+class WEBCORE_EXPORT DeferrableOneShotTimer : private TimerBase {
+    WTF_MAKE_FAST_ALLOCATED;
+public:
+    DeferrableOneShotTimer(WTF::Function<void()>&&);
+    ~DeferrableOneShotTimer();
+
+    void startOneShot(Seconds interval);
+    void stop();
+
+private:
+    void fired() override;
+
+    WTF::Function<void()> m_function;
+    MonotonicTime m_restartFireTime;
+};
+
 }

trunk/Source/WebCore/platform/graphics/ca/TileController.h

@@ -208 +208 @@
 
     Timer m_tileRevalidationTimer;
-    DeferrableOneShotTimer m_tileSizeChangeTimer;
+    ResettableOneShotTimer m_tileSizeChangeTimer;
 
     TileCoverage m_tileCoverage { CoverageForVisibleArea };

trunk/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h

@@ -95 +95 @@
     HashCountedSet<CGImageRef> m_images;
     SubimageCacheHashSet m_cache;
-    DeferrableOneShotTimer m_timer;
+    ResettableOneShotTimer m_timer;
 
     static SubimageCacheWithTimer& subimageCache();

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-02-04  Benjamin Poulain  <benjamin@webkit.org>
+
+        Use deferrable timer to restart the Responsiveness Timer on each wheel event
+        https://bugs.webkit.org/show_bug.cgi?id=194135
+
+        Reviewed by Simon Fraser.
+
+        Simon Fraser suggested a neat improvement over my previous optimization
+        of ResponsivenessTimer.
+
+        Instead of reseting the deadline with every event, we can let the timer
+        fire and add the missing time from the last start.
+
+        I implemented that behavior in the new Deferrable Timer class and use
+        it from ResponsivenessTimer.
+
+        * NetworkProcess/watchos/NetworkProximityAssertion.h:
+        * UIProcess/ResponsivenessTimer.h:
+        * WebProcess/Plugins/PluginView.h:
+
 2019-02-04  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebKit/NetworkProcess/watchos/NetworkProximityAssertion.h

@@ -88 +88 @@
     uint64_t m_assertionCount { 0 };
     State m_state { State::Suspended };
-    WebCore::DeferrableOneShotTimer m_releaseTimer;
-    WebCore::DeferrableOneShotTimer m_suspendAfterBackgroundingTimer;
+    WebCore::ResettableOneShotTimer m_releaseTimer;
+    WebCore::ResettableOneShotTimer m_suspendAfterBackgroundingTimer;
 };
 

trunk/Source/WebKit/UIProcess/ResponsivenessTimer.cpp

@@ -33 +33 @@
 ResponsivenessTimer::ResponsivenessTimer(ResponsivenessTimer::Client& client)
     : m_client(client)
-    , m_timer(*this, &ResponsivenessTimer::timerFired)
+    , m_timer(std::bind(&ResponsivenessTimer::timerFired, this))
 {
 }

trunk/Source/WebKit/UIProcess/ResponsivenessTimer.h

@@ -72 +72 @@
 
     ResponsivenessTimer::Client& m_client;
-    WebCore::Timer m_timer;
+    WebCore::DeferrableOneShotTimer m_timer;
     bool m_isResponsive { true };
     bool m_waitingForTimer { false };

trunk/Source/WebKit/WebProcess/Plugins/PluginView.h

@@ -285 +285 @@
     RefPtr<ShareableBitmap> m_transientPaintingSnapshot;
     // This timer is used when plugin snapshotting is enabled, to capture a plugin placeholder.
-    WebCore::DeferrableOneShotTimer m_pluginSnapshotTimer;
+    WebCore::ResettableOneShotTimer m_pluginSnapshotTimer;
 #if ENABLE(PRIMARY_SNAPSHOTTED_PLUGIN_HEURISTIC) || PLATFORM(COCOA)
     unsigned m_countSnapshotRetries { 0 };