Move tracking and computation of timer heap current insertion order to ThreadTimers
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Oct 2019 22:15:45 +0000 (22:15 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Oct 2019 22:15:45 +0000 (22:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=203519

Reviewed by Jer Noble.

Each thread maintains its own heap of timers. So, they should also maintain the running insertion count.
The running insertion count is used to ensure timers with the same firing time are ordered by when
they were inserted into the heap. This is important to ensure that code like:

        window.setTimeout(a, 0)
        window.setTimeout(b, 0)

schedule a() to be called before b() even though both has the same firing time.

Currently the insertion count is process-wide. That is, it is globally unique across all threads. The
current width of the count is sizeof(unsigned) and so the more threads that increment it the faster it
approaches the point of wrapping around. The current code can only ensure correct timer ordering in a
window of sizeof(unsigned) / 2 timers (see TimerHeapLessThanFunction::compare(MonotonicTime, unsigned, MonotonicTime, unsigned)).
We could simply leave it process-wide and increases the width to 64-bits, but I felt it made more
sense conceptually to move the count to the thread local storage and be with the timer heap itself
despite the extra 4 bytes per thread that it adds.

* dom/ActiveDOMObject.h:
* platform/ThreadTimers.h:
(WebCore::ThreadTimers::nextHeapInsertionCount): Added.
* platform/Timer.cpp:
(WebCore::TimerBase::setNextFireTime):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@251810 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/ThreadTimers.h
Source/WebCore/platform/Timer.cpp

index caeb2e5..c72ea8c 100644 (file)
@@ -1,3 +1,33 @@
+2019-10-30  Daniel Bates  <dabates@apple.com>
+
+        Move tracking and computation of timer heap current insertion order to ThreadTimers
+        https://bugs.webkit.org/show_bug.cgi?id=203519
+
+        Reviewed by Jer Noble.
+
+        Each thread maintains its own heap of timers. So, they should also maintain the running insertion count.
+        The running insertion count is used to ensure timers with the same firing time are ordered by when
+        they were inserted into the heap. This is important to ensure that code like:
+
+                window.setTimeout(a, 0)
+                window.setTimeout(b, 0)
+
+        schedule a() to be called before b() even though both has the same firing time.
+
+        Currently the insertion count is process-wide. That is, it is globally unique across all threads. The
+        current width of the count is sizeof(unsigned) and so the more threads that increment it the faster it
+        approaches the point of wrapping around. The current code can only ensure correct timer ordering in a
+        window of sizeof(unsigned) / 2 timers (see TimerHeapLessThanFunction::compare(MonotonicTime, unsigned, MonotonicTime, unsigned)).
+        We could simply leave it process-wide and increases the width to 64-bits, but I felt it made more
+        sense conceptually to move the count to the thread local storage and be with the timer heap itself
+        despite the extra 4 bytes per thread that it adds.
+
+        * dom/ActiveDOMObject.h:
+        * platform/ThreadTimers.h:
+        (WebCore::ThreadTimers::nextHeapInsertionCount): Added.
+        * platform/Timer.cpp:
+        (WebCore::TimerBase::setNextFireTime):
+
 2019-10-30  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Remove the argument 'resultElement' from the SVG animation functions
index 7a11138..2c342fa 100644 (file)
@@ -56,6 +56,8 @@ public:
     void updateSharedTimer();
     void fireTimersInNestedEventLoop();
 
+    unsigned nextHeapInsertionCount() { return m_currentHeapInsertionOrder++; }
+
 private:
     void sharedTimerFiredInternal();
     void fireTimersInNestedEventLoopInternal();
@@ -63,6 +65,7 @@ private:
     ThreadTimerHeap m_timerHeap;
     SharedTimer* m_sharedTimer { nullptr }; // External object, can be a run loop on a worker thread. Normally set/reset by worker thread.
     bool m_firingTimers { false }; // Reentrancy guard.
+    unsigned m_currentHeapInsertionOrder { 0 };
     MonotonicTime m_pendingSharedTimerFireTime;
 };
 
index aa5e0da..add48e6 100644 (file)
@@ -480,9 +480,7 @@ void TimerBase::setNextFireTime(MonotonicTime newTime)
     }
 
     if (oldTime != newTime) {
-        // FIXME: This should be part of ThreadTimers, or another per-thread structure.
-        static std::atomic<unsigned> currentHeapInsertionOrder;
-        auto newOrder = currentHeapInsertionOrder++;
+        auto newOrder = threadGlobalData().threadTimers().nextHeapInsertionCount();
 
         if (!m_heapItem)
             m_heapItem = ThreadTimerHeapItem::create(*this, newTime, 0);