Title: [241113] trunk/Source
Revision
241113
Author
benja...@webkit.org
Date
2019-02-06 19:13:11 -0800 (Wed, 06 Feb 2019)

Log Message

Unreviewed, rolling out r240759 and r240944.

Some timer uses are done off the main thread, WebCore::Timer
cannot be used

Reverted changesets:

"<rdar://problem/47570443> Responsiveness timers are too
expensive for frequent events"
https://bugs.webkit.org/show_bug.cgi?id=194003
https://trac.webkit.org/changeset/240759

"Use deferrable timer to restart the Responsiveness Timer on
each wheel event"
https://bugs.webkit.org/show_bug.cgi?id=194135
https://trac.webkit.org/changeset/240944

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (241112 => 241113)


--- trunk/Source/WebCore/ChangeLog	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebCore/ChangeLog	2019-02-07 03:13:11 UTC (rev 241113)
@@ -1,3 +1,22 @@
+2019-02-06  Benjamin Poulain  <benja...@webkit.org>
+
+        Unreviewed, rolling out r240759 and r240944.
+
+        Some timer uses are done off the main thread, WebCore::Timer
+        cannot be used
+
+        Reverted changesets:
+
+        "<rdar://problem/47570443> Responsiveness timers are too
+        expensive for frequent events"
+        https://bugs.webkit.org/show_bug.cgi?id=194003
+        https://trac.webkit.org/changeset/240759
+
+        "Use deferrable timer to restart the Responsiveness Timer on
+        each wheel event"
+        https://bugs.webkit.org/show_bug.cgi?id=194135
+        https://trac.webkit.org/changeset/240944
+
 2019-02-06  Keith Rollin  <krol...@apple.com>
 
         Update .xcfilelist files

Modified: trunk/Source/WebCore/css/CSSImageGeneratorValue.cpp (241112 => 241113)


--- trunk/Source/WebCore/css/CSSImageGeneratorValue.cpp	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebCore/css/CSSImageGeneratorValue.cpp	2019-02-07 03:13:11 UTC (rev 241113)
@@ -56,7 +56,7 @@
     CSSImageGeneratorValue& m_owner;
     const FloatSize m_size;
     const Ref<GeneratedImage> m_image;
-    ResettableOneShotTimer m_evictionTimer;
+    DeferrableOneShotTimer m_evictionTimer;
 };
 
 CSSImageGeneratorValue::CSSImageGeneratorValue(ClassType classType)

Modified: trunk/Source/WebCore/html/HTMLPlugInImageElement.h (241112 => 241113)


--- trunk/Source/WebCore/html/HTMLPlugInImageElement.h	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebCore/html/HTMLPlugInImageElement.h	2019-02-07 03:13:11 UTC (rev 241113)
@@ -130,7 +130,7 @@
     bool m_needsWidgetUpdate { false };
     bool m_needsDocumentActivationCallbacks { false };
     RefPtr<MouseEvent> m_pendingClickEventFromSnapshot;
-    ResettableOneShotTimer m_simulatedMouseClickTimer;
+    DeferrableOneShotTimer m_simulatedMouseClickTimer;
     Timer m_removeSnapshotTimer;
     RefPtr<Image> m_snapshotImage;
     bool m_createdDuringUserGesture { false };

Modified: trunk/Source/WebCore/loader/cache/CachedResource.h (241112 => 241113)


--- trunk/Source/WebCore/loader/cache/CachedResource.h	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebCore/loader/cache/CachedResource.h	2019-02-07 03:13:11 UTC (rev 241113)
@@ -311,7 +311,7 @@
     ResourceRequest m_resourceRequest;
     ResourceResponse m_response;
 
-    ResettableOneShotTimer m_decodedDataDeletionTimer;
+    DeferrableOneShotTimer m_decodedDataDeletionTimer;
 
     // FIXME: Make the rest of these data members private and use functions in derived classes instead.
     HashCountedSet<CachedResourceClient*> m_clients;

Modified: trunk/Source/WebCore/platform/Timer.cpp (241112 => 241113)


--- trunk/Source/WebCore/platform/Timer.cpp	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebCore/platform/Timer.cpp	2019-02-07 03:13:11 UTC (rev 241113)
@@ -260,7 +260,7 @@
 
 TimerBase::~TimerBase()
 {
-    assertThreadSafety();
+    ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
     RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()) || shouldSuppressThreadSafetyCheck());
     stop();
     ASSERT(!inHeap());
@@ -273,7 +273,7 @@
 
 void TimerBase::start(Seconds nextFireInterval, Seconds repeatInterval)
 {
-    assertThreadSafety();
+    ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
 
     m_repeatInterval = repeatInterval;
     setNextFireTime(MonotonicTime::now() + nextFireInterval);
@@ -281,7 +281,7 @@
 
 void TimerBase::stop()
 {
-    assertThreadSafety();
+    ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
 
     m_repeatInterval = 0_s;
     setNextFireTime(MonotonicTime { });
@@ -465,7 +465,7 @@
 
 void TimerBase::setNextFireTime(MonotonicTime newTime)
 {
-    assertThreadSafety();
+    ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
     RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()) || shouldSuppressThreadSafetyCheck());
     RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!m_wasDeleted);
 
@@ -520,53 +520,5 @@
     return std::max(m_unalignedNextFireTime - MonotonicTime::now(), 0_s);
 }
 
-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
 

Modified: trunk/Source/WebCore/platform/Timer.h (241112 => 241113)


--- trunk/Source/WebCore/platform/Timer.h	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebCore/platform/Timer.h	2019-02-07 03:13:11 UTC (rev 241113)
@@ -68,15 +68,6 @@
 
     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;
 
@@ -85,6 +76,8 @@
     void checkConsistency() const;
     void checkHeapIndex() const;
 
+    void setNextFireTime(MonotonicTime);
+
     bool inHeap() const { return m_heapItem && m_heapItem->isInHeap(); }
 
     bool hasValidHeapPosition() const;
@@ -99,6 +92,8 @@
     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
     Seconds m_repeatInterval; // 0 if not repeating
     bool m_wasDeleted { false };
@@ -146,29 +141,16 @@
     return static_cast<bool>(nextFireTime());
 }
 
-// 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 {
+class DeferrableOneShotTimer : protected TimerBase {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     template<typename TimerFiredClass>
-    ResettableOneShotTimer(TimerFiredClass& object, void (TimerFiredClass::*function)(), Seconds delay)
-        : ResettableOneShotTimer(std::bind(function, &object), delay)
+    DeferrableOneShotTimer(TimerFiredClass& object, void (TimerFiredClass::*function)(), Seconds delay)
+        : DeferrableOneShotTimer(std::bind(function, &object), delay)
     {
     }
 
-    ResettableOneShotTimer(WTF::Function<void()>&& function, Seconds delay)
+    DeferrableOneShotTimer(WTF::Function<void ()>&& function, Seconds delay)
         : m_function(WTFMove(function))
         , m_delay(delay)
         , m_shouldRestartWhenTimerFires(false)
@@ -214,30 +196,4 @@
     bool m_shouldRestartWhenTimerFires;
 };
 
-// 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;
-};
-
 }

Modified: trunk/Source/WebCore/platform/graphics/ca/TileController.h (241112 => 241113)


--- trunk/Source/WebCore/platform/graphics/ca/TileController.h	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebCore/platform/graphics/ca/TileController.h	2019-02-07 03:13:11 UTC (rev 241113)
@@ -207,7 +207,7 @@
     IntRect m_boundsAtLastRevalidate;
 
     Timer m_tileRevalidationTimer;
-    ResettableOneShotTimer m_tileSizeChangeTimer;
+    DeferrableOneShotTimer m_tileSizeChangeTimer;
 
     TileCoverage m_tileCoverage { CoverageForVisibleArea };
     

Modified: trunk/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h (241112 => 241113)


--- trunk/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h	2019-02-07 03:13:11 UTC (rev 241113)
@@ -94,7 +94,7 @@
 
     HashCountedSet<CGImageRef> m_images;
     SubimageCacheHashSet m_cache;
-    ResettableOneShotTimer m_timer;
+    DeferrableOneShotTimer m_timer;
 
     static SubimageCacheWithTimer& subimageCache();
     static bool subimageCacheExists();

Modified: trunk/Source/WebKit/ChangeLog (241112 => 241113)


--- trunk/Source/WebKit/ChangeLog	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebKit/ChangeLog	2019-02-07 03:13:11 UTC (rev 241113)
@@ -1,3 +1,22 @@
+2019-02-06  Benjamin Poulain  <benja...@webkit.org>
+
+        Unreviewed, rolling out r240759 and r240944.
+
+        Some timer uses are done off the main thread, WebCore::Timer
+        cannot be used
+
+        Reverted changesets:
+
+        "<rdar://problem/47570443> Responsiveness timers are too
+        expensive for frequent events"
+        https://bugs.webkit.org/show_bug.cgi?id=194003
+        https://trac.webkit.org/changeset/240759
+
+        "Use deferrable timer to restart the Responsiveness Timer on
+        each wheel event"
+        https://bugs.webkit.org/show_bug.cgi?id=194135
+        https://trac.webkit.org/changeset/240944
+
 2019-02-06  chris fleizach  <cfleiz...@apple.com>
 
         AX:  com.apple.WebKit.WebContent at WebKit: -[WKAccessibilityWebPageObjectBase axObjectCache]

Modified: trunk/Source/WebKit/NetworkProcess/watchos/NetworkProximityAssertion.h (241112 => 241113)


--- trunk/Source/WebKit/NetworkProcess/watchos/NetworkProximityAssertion.h	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebKit/NetworkProcess/watchos/NetworkProximityAssertion.h	2019-02-07 03:13:11 UTC (rev 241113)
@@ -87,8 +87,8 @@
 
     uint64_t m_assertionCount { 0 };
     State m_state { State::Suspended };
-    WebCore::ResettableOneShotTimer m_releaseTimer;
-    WebCore::ResettableOneShotTimer m_suspendAfterBackgroundingTimer;
+    WebCore::DeferrableOneShotTimer m_releaseTimer;
+    WebCore::DeferrableOneShotTimer m_suspendAfterBackgroundingTimer;
 };
 
 class BluetoothProximityAssertion final : public NetworkProximityAssertion {

Modified: trunk/Source/WebKit/UIProcess/ResponsivenessTimer.cpp (241112 => 241113)


--- trunk/Source/WebKit/UIProcess/ResponsivenessTimer.cpp	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebKit/UIProcess/ResponsivenessTimer.cpp	2019-02-07 03:13:11 UTC (rev 241113)
@@ -32,32 +32,27 @@
 
 ResponsivenessTimer::ResponsivenessTimer(ResponsivenessTimer::Client& client)
     : m_client(client)
-    , m_timer(std::bind(&ResponsivenessTimer::timerFired, this))
+    , m_isResponsive(true)
+    , m_timer(RunLoop::main(), this, &ResponsivenessTimer::timerFired)
 {
 }
 
-ResponsivenessTimer::~ResponsivenessTimer() = default;
+ResponsivenessTimer::~ResponsivenessTimer()
+{
+    m_timer.stop();
+}
 
 void ResponsivenessTimer::invalidate()
 {
-    m_waitingForTimer = false;
-    m_useLazyStop = false;
     m_timer.stop();
 }
 
 void ResponsivenessTimer::timerFired()
 {
-    if (!m_waitingForTimer)
-        return;
-
-    m_waitingForTimer = false;
-    m_useLazyStop = false;
-
     if (!m_isResponsive)
         return;
 
     if (!m_client.mayBecomeUnresponsive()) {
-        m_waitingForTimer = true;
         m_timer.startOneShot(responsivenessTimeout);
         return;
     }
@@ -71,22 +66,12 @@
     
 void ResponsivenessTimer::start()
 {
-    if (m_waitingForTimer)
+    if (m_timer.isActive())
         return;
 
-    m_waitingForTimer = true;
-    m_useLazyStop = false;
     m_timer.startOneShot(responsivenessTimeout);
 }
 
-void ResponsivenessTimer::startWithLazyStop()
-{
-    if (!m_waitingForTimer) {
-        start();
-        m_useLazyStop = true;
-    }
-}
-
 void ResponsivenessTimer::stop()
 {
     if (!m_isResponsive) {
@@ -98,19 +83,13 @@
         m_client.didBecomeResponsive();
     }
 
-    m_waitingForTimer = false;
-
-    if (m_useLazyStop)
-        m_useLazyStop = false;
-    else
-        m_timer.stop();
+    m_timer.stop();
 }
 
 void ResponsivenessTimer::processTerminated()
 {
     // Since there is no web process, we must not be waiting for it anymore.
-    m_waitingForTimer = false;
-    m_timer.stop();
+    stop();
 }
 
 } // namespace WebKit

Modified: trunk/Source/WebKit/UIProcess/ResponsivenessTimer.h (241112 => 241113)


--- trunk/Source/WebKit/UIProcess/ResponsivenessTimer.h	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebKit/UIProcess/ResponsivenessTimer.h	2019-02-07 03:13:11 UTC (rev 241113)
@@ -26,7 +26,7 @@
 #ifndef ResponsivenessTimer_h
 #define ResponsivenessTimer_h
 
-#include <WebCore/Timer.h>
+#include <wtf/RunLoop.h>
 
 namespace WebKit {
 
@@ -48,17 +48,6 @@
     ~ResponsivenessTimer();
     
     void start();
-
-    // A responsiveness timer with lazy stop does not stop the underlying system timer when stopped.
-    // Instead, it ignores the timeout if stop() was already called.
-    //
-    // This exists to reduce the rate at which we reset the timer.
-    //
-    // With a non lazy timer, we may set a timer and reset it soon after because the process is responsive.
-    // For events, this means reseting a timer 120 times/s for a 60 Hz event source.
-    // By not reseting the timer when responsive, we cut that in half to 60 timeout changes.
-    void startWithLazyStop();
-
     void stop();
 
     void invalidate();
@@ -71,10 +60,9 @@
     void timerFired();
 
     ResponsivenessTimer::Client& m_client;
-    WebCore::DeferrableOneShotTimer m_timer;
-    bool m_isResponsive { true };
-    bool m_waitingForTimer { false };
-    bool m_useLazyStop { false };
+    bool m_isResponsive;
+
+    RunLoop::Timer<ResponsivenessTimer> m_timer;
 };
 
 } // namespace WebKit

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (241112 => 241113)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2019-02-07 03:13:11 UTC (rev 241113)
@@ -2357,7 +2357,7 @@
 
     // Manually ping the web process to check for responsiveness since our wheel
     // event will dispatch to a non-main thread, which always responds.
-    m_process->isResponsiveWithLazyStop();
+    m_process->isResponsive(nullptr);
 }
 
 bool WebPageProxy::shouldProcessWheelEventNow(const WebWheelEvent& event) const

Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (241112 => 241113)


--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp	2019-02-07 03:13:11 UTC (rev 241113)
@@ -1120,15 +1120,6 @@
     send(Messages::WebProcess::MainThreadPing(), 0);
 }
 
-void WebProcessProxy::isResponsiveWithLazyStop()
-{
-    if (m_isResponsive == NoOrMaybe::No)
-        return;
-
-    responsivenessTimer().startWithLazyStop();
-    send(Messages::WebProcess::MainThreadPing(), 0);
-}
-
 bool WebProcessProxy::isJITEnabled() const
 {
     return processPool().configuration().isJITEnabled();

Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (241112 => 241113)


--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h	2019-02-07 03:13:11 UTC (rev 241113)
@@ -206,7 +206,6 @@
     ProcessThrottler& throttler() { return m_throttler; }
 
     void isResponsive(WTF::Function<void(bool isWebProcessResponsive)>&&);
-    void isResponsiveWithLazyStop();
     void didReceiveMainThreadPing();
     void didReceiveBackgroundResponsivenessPing();
 

Modified: trunk/Source/WebKit/WebProcess/Plugins/PluginView.h (241112 => 241113)


--- trunk/Source/WebKit/WebProcess/Plugins/PluginView.h	2019-02-07 02:53:16 UTC (rev 241112)
+++ trunk/Source/WebKit/WebProcess/Plugins/PluginView.h	2019-02-07 03:13:11 UTC (rev 241113)
@@ -284,7 +284,7 @@
     // This snapshot is used to avoid side effects should the plugin run JS during painting.
     RefPtr<ShareableBitmap> m_transientPaintingSnapshot;
     // This timer is used when plugin snapshotting is enabled, to capture a plugin placeholder.
-    WebCore::ResettableOneShotTimer m_pluginSnapshotTimer;
+    WebCore::DeferrableOneShotTimer m_pluginSnapshotTimer;
 #if ENABLE(PRIMARY_SNAPSHOTTED_PLUGIN_HEURISTIC) || PLATFORM(COCOA)
     unsigned m_countSnapshotRetries { 0 };
 #endif
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to