Attachment 360946 Details for Bug 194135 – Patch

[patch] Patch

bug-194135-20190201190200.patch (text/plain), 15.68 KB, created by Benjamin Poulain on 2019-02-01 19:02:01 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Benjamin Poulain

Created: 2019-02-01 19:02:01 PST

Size: 15.68 KB

patch

obsolete

>Subversion Revision: 240791
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 38edd5d0c1a4be34e9e6f5e6c4a300c3163679b4..71808f946043a15feaa21e84373f75002af042f4 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,36 @@
>+2019-01-31  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 NOBODY (OOPS!).
>+
>+        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-01-31  Alicia Boya GarcÃa  <aboya@igalia.com>
> 
>         [MSE][GStreamer] Use reference instead of pointer in m_playerPrivate
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index dd09e1bcd77c13904f0edef65253f119395e0a05..3695620fe1b819c57f5e62133f308a29d618bc34 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,23 @@
>+2019-01-31  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 NOBODY (OOPS!).
>+
>+        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-01-30  Simon Fraser  <simon.fraser@apple.com>
> 
>         [Mac] Implement basic hit testing in the scrolling tree
>diff --git a/Source/WebCore/css/CSSImageGeneratorValue.cpp b/Source/WebCore/css/CSSImageGeneratorValue.cpp
>index ffdd3d0aa657f25eaf31d570c5868ed5309e2efd..ba822e0c24dd461ebf7210121ed5619c55513a97 100644
>--- a/Source/WebCore/css/CSSImageGeneratorValue.cpp
>+++ b/Source/WebCore/css/CSSImageGeneratorValue.cpp
>@@ -56,7 +56,7 @@ private:
>     CSSImageGeneratorValue& m_owner;
>     const FloatSize m_size;
>     const Ref<GeneratedImage> m_image;
>-    DeferrableOneShotTimer m_evictionTimer;
>+    ResettableOneShotTimer m_evictionTimer;
> };
> 
> CSSImageGeneratorValue::CSSImageGeneratorValue(ClassType classType)
>diff --git a/Source/WebCore/html/HTMLPlugInImageElement.h b/Source/WebCore/html/HTMLPlugInImageElement.h
>index ce0ea361cb4d50e6132dd6f8ace6f5b6cdf88f8f..1179620f96aa5a2aec0779e1d8bd73c8d2a4dcf0 100644
>--- a/Source/WebCore/html/HTMLPlugInImageElement.h
>+++ b/Source/WebCore/html/HTMLPlugInImageElement.h
>@@ -130,7 +130,7 @@ private:
>     bool m_needsWidgetUpdate { false };
>     bool m_needsDocumentActivationCallbacks { false };
>     RefPtr<MouseEvent> m_pendingClickEventFromSnapshot;
>-    DeferrableOneShotTimer m_simulatedMouseClickTimer;
>+    ResettableOneShotTimer m_simulatedMouseClickTimer;
>     Timer m_removeSnapshotTimer;
>     RefPtr<Image> m_snapshotImage;
>     bool m_createdDuringUserGesture { false };
>diff --git a/Source/WebCore/loader/cache/CachedResource.h b/Source/WebCore/loader/cache/CachedResource.h
>index 9b83848ea2249d587a3ed81aff25e7864b0290f2..c8f78805a84933c936ed7b2e37f1d954be095a2b 100644
>--- a/Source/WebCore/loader/cache/CachedResource.h
>+++ b/Source/WebCore/loader/cache/CachedResource.h
>@@ -311,7 +311,7 @@ protected:
>     ResourceRequest m_resourceRequest;
>     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.
>     HashCountedSet<CachedResourceClient*> m_clients;
>diff --git a/Source/WebCore/platform/Timer.cpp b/Source/WebCore/platform/Timer.cpp
>index fd82402235f3e72e699459a6021183858af7cb02..bd5b59ee321e2aac2764fe837bd10f6412b91931 100644
>--- a/Source/WebCore/platform/Timer.cpp
>+++ b/Source/WebCore/platform/Timer.cpp
>@@ -260,7 +260,7 @@ TimerBase::TimerBase()
> 
> TimerBase::~TimerBase()
> {
>-    ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
>+    assertThreadSafety();
>     RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()) || shouldSuppressThreadSafetyCheck());
>     stop();
>     ASSERT(!inHeap());
>@@ -273,7 +273,7 @@ TimerBase::~TimerBase()
> 
> void TimerBase::start(Seconds nextFireInterval, Seconds repeatInterval)
> {
>-    ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
>+    assertThreadSafety();
> 
>     m_repeatInterval = repeatInterval;
>     setNextFireTime(MonotonicTime::now() + nextFireInterval);
>@@ -281,7 +281,7 @@ void TimerBase::start(Seconds nextFireInterval, Seconds repeatInterval)
> 
> void TimerBase::stop()
> {
>-    ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
>+    assertThreadSafety();
> 
>     m_repeatInterval = 0_s;
>     setNextFireTime(MonotonicTime { });
>@@ -465,7 +465,7 @@ void TimerBase::updateHeapIfNeeded(MonotonicTime oldTime)
> 
> 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);
> 
>@@ -520,5 +520,53 @@ Seconds TimerBase::nextUnalignedFireInterval() const
>     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
> 
>diff --git a/Source/WebCore/platform/Timer.h b/Source/WebCore/platform/Timer.h
>index 6601675fd6d9c288d6ea1a493e5eeae8fbc2be9b..728db091f0a6c4c34782b5f9fa4b944ca17f6b4c 100644
>--- a/Source/WebCore/platform/Timer.h
>+++ b/Source/WebCore/platform/Timer.h
>@@ -68,6 +68,15 @@ public:
> 
>     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,8 +85,6 @@ private:
>     void checkConsistency() const;
>     void checkHeapIndex() const;
> 
>-    void setNextFireTime(MonotonicTime);
>-
>     bool inHeap() const { return m_heapItem && m_heapItem->isInHeap(); }
> 
>     bool hasValidHeapPosition() const;
>@@ -92,8 +99,6 @@ private:
>     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 };
>@@ -141,16 +146,29 @@ inline bool TimerBase::isActive() const
>     return static_cast<bool>(nextFireTime());
> }
> 
>-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)
>+    ResettableOneShotTimer(TimerFiredClass& object, void (TimerFiredClass::*function)(), Seconds delay)
>+        : ResettableOneShotTimer(std::bind(function, &object), delay)
>     {
>     }
> 
>-    DeferrableOneShotTimer(WTF::Function<void ()>&& function, Seconds delay)
>+    ResettableOneShotTimer(WTF::Function<void()>&& function, Seconds delay)
>         : m_function(WTFMove(function))
>         , m_delay(delay)
>         , m_shouldRestartWhenTimerFires(false)
>@@ -196,4 +214,30 @@ private:
>     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;
>+};
>+
> }
>diff --git a/Source/WebCore/platform/graphics/ca/TileController.h b/Source/WebCore/platform/graphics/ca/TileController.h
>index 5797e8644080af1f064d06d3c8ae68718920d173..4b93add4b597ac2d211e8a8295728543b8b00142 100644
>--- a/Source/WebCore/platform/graphics/ca/TileController.h
>+++ b/Source/WebCore/platform/graphics/ca/TileController.h
>@@ -207,7 +207,7 @@ private:
>     IntRect m_boundsAtLastRevalidate;
> 
>     Timer m_tileRevalidationTimer;
>-    DeferrableOneShotTimer m_tileSizeChangeTimer;
>+    ResettableOneShotTimer m_tileSizeChangeTimer;
> 
>     TileCoverage m_tileCoverage { CoverageForVisibleArea };
>     
>diff --git a/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h b/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h
>index 1a461ca13d9f079d499800127059f9bd0b59703d..d0a1a02c94ae6a595b9f8e271926aaf120baedf2 100644
>--- a/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h
>+++ b/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h
>@@ -94,7 +94,7 @@ private:
> 
>     HashCountedSet<CGImageRef> m_images;
>     SubimageCacheHashSet m_cache;
>-    DeferrableOneShotTimer m_timer;
>+    ResettableOneShotTimer m_timer;
> 
>     static SubimageCacheWithTimer& subimageCache();
>     static bool subimageCacheExists();
>diff --git a/Source/WebKit/NetworkProcess/watchos/NetworkProximityAssertion.h b/Source/WebKit/NetworkProcess/watchos/NetworkProximityAssertion.h
>index 91107124f506aa7950540ba1e9ce970213816f09..caa55fc9d3bc582439bac02ced7f68fb26f3758e 100644
>--- a/Source/WebKit/NetworkProcess/watchos/NetworkProximityAssertion.h
>+++ b/Source/WebKit/NetworkProcess/watchos/NetworkProximityAssertion.h
>@@ -87,8 +87,8 @@ private:
> 
>     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;
> };
> 
> class BluetoothProximityAssertion final : public NetworkProximityAssertion {
>diff --git a/Source/WebKit/UIProcess/ResponsivenessTimer.cpp b/Source/WebKit/UIProcess/ResponsivenessTimer.cpp
>index 9002ba1e2ce6f57d94d68a7b11d9c87ab9216aa1..4a29daa9217597d303a00bb90c4630a2286b836f 100644
>--- a/Source/WebKit/UIProcess/ResponsivenessTimer.cpp
>+++ b/Source/WebKit/UIProcess/ResponsivenessTimer.cpp
>@@ -32,7 +32,7 @@ static const Seconds responsivenessTimeout { 3_s };
> 
> ResponsivenessTimer::ResponsivenessTimer(ResponsivenessTimer::Client& client)
>     : m_client(client)
>-    , m_timer(*this, &ResponsivenessTimer::timerFired)
>+    , m_timer(std::bind(&ResponsivenessTimer::timerFired, this))
> {
> }
> 
>diff --git a/Source/WebKit/UIProcess/ResponsivenessTimer.h b/Source/WebKit/UIProcess/ResponsivenessTimer.h
>index d2df806027080014a9c481474c0f288b134f50fe..2a7095d550e68c8195c4edecf65a95abdedf4f22 100644
>--- a/Source/WebKit/UIProcess/ResponsivenessTimer.h
>+++ b/Source/WebKit/UIProcess/ResponsivenessTimer.h
>@@ -71,7 +71,7 @@ private:
>     void timerFired();
> 
>     ResponsivenessTimer::Client& m_client;
>-    WebCore::Timer m_timer;
>+    WebCore::DeferrableOneShotTimer m_timer;
>     bool m_isResponsive { true };
>     bool m_waitingForTimer { false };
>     bool m_useLazyStop { false };
>diff --git a/Source/WebKit/WebProcess/Plugins/PluginView.h b/Source/WebKit/WebProcess/Plugins/PluginView.h
>index 84d4264ce0d06886c399c85648d25a9ca1eb8166..de0994182f4c406218187e54d1a28896840750e2 100644
>--- a/Source/WebKit/WebProcess/Plugins/PluginView.h
>+++ b/Source/WebKit/WebProcess/Plugins/PluginView.h
>@@ -284,7 +284,7 @@ private:
>     // 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::DeferrableOneShotTimer m_pluginSnapshotTimer;
>+    WebCore::ResettableOneShotTimer m_pluginSnapshotTimer;
> #if ENABLE(PRIMARY_SNAPSHOTTED_PLUGIN_HEURISTIC) || PLATFORM(COCOA)
>     unsigned m_countSnapshotRetries { 0 };
> #endif

Actions: View | Formatted Diff | Diff

Attachments on bug 194135: 360824 | 360922 | 360944 | 360946