Attachment 357931 Details for Bug 192975 – WIP

[patch] WIP

wip192975.patch (text/plain), 14.15 KB, created by Ryosuke Niwa on 2018-12-20 23:22:48 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-12-20 23:22:48 PST

Size: 14.15 KB

patch

obsolete

>Index: Source/WebCore/platform/ThreadTimers.cpp
>===================================================================
>--- Source/WebCore/platform/ThreadTimers.cpp	(revision 239401)
>+++ Source/WebCore/platform/ThreadTimers.cpp	(working copy)
>@@ -104,8 +104,16 @@
>     MonotonicTime fireTime = MonotonicTime::now();
>     MonotonicTime timeToQuit = fireTime + maxDurationOfFiringTimers;
> 
>-    while (!m_timerHeap.isEmpty() && m_timerHeap.first()->m_nextFireTime <= fireTime) {
>-        TimerBase* timer = m_timerHeap.first();
>+    while (!m_timerHeap.isEmpty()) {
>+        WeakPtr<TimerBase> timer = m_timerHeap.first();
>+        if (!timer) {
>+            TimerBase::heapPopNullMin(m_timerHeap);
>+            continue;
>+        }
>+
>+        if (timer->m_nextFireTime > fireTime)
>+            break;
>+
>         timer->m_nextFireTime = MonotonicTime { };
>         timer->m_unalignedNextFireTime = MonotonicTime { };
>         timer->heapDeleteMin();
>Index: Source/WebCore/platform/ThreadTimers.h
>===================================================================
>--- Source/WebCore/platform/ThreadTimers.h	(revision 239401)
>+++ Source/WebCore/platform/ThreadTimers.h	(working copy)
>@@ -33,32 +33,32 @@
> 
> namespace WebCore {
> 
>-    class SharedTimer;
>-    class TimerBase;
>+class SharedTimer;
>+class TimerBase;
> 
>-    // A collection of timers per thread. Kept in ThreadGlobalData.
>-    class ThreadTimers {
>-        WTF_MAKE_NONCOPYABLE(ThreadTimers); WTF_MAKE_FAST_ALLOCATED;
>-    public:
>-        ThreadTimers();
>+// A collection of timers per thread. Kept in ThreadGlobalData.
>+class ThreadTimers {
>+    WTF_MAKE_NONCOPYABLE(ThreadTimers); WTF_MAKE_FAST_ALLOCATED;
>+public:
>+    ThreadTimers();
> 
>-        // On a thread different then main, we should set the thread's instance of the SharedTimer.
>-        void setSharedTimer(SharedTimer*);
>+    // On a thread different then main, we should set the thread's instance of the SharedTimer.
>+    void setSharedTimer(SharedTimer*);
> 
>-        Vector<TimerBase*>& timerHeap() { return m_timerHeap; }
>+    Vector<WeakPtr<TimerBase>>& timerHeap() { return m_timerHeap; }
> 
>-        void updateSharedTimer();
>-        void fireTimersInNestedEventLoop();
>+    void updateSharedTimer();
>+    void fireTimersInNestedEventLoop();
> 
>-    private:
>-        void sharedTimerFiredInternal();
>-        void fireTimersInNestedEventLoopInternal();
>+private:
>+    void sharedTimerFiredInternal();
>+    void fireTimersInNestedEventLoopInternal();
> 
>-        Vector<TimerBase*> 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.
>-        MonotonicTime m_pendingSharedTimerFireTime;
>-    };
>+    Vector<WeakPtr<TimerBase>> 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.
>+    MonotonicTime m_pendingSharedTimerFireTime;
>+};
> 
> }
> 
>Index: Source/WebCore/platform/Timer.cpp
>===================================================================
>--- Source/WebCore/platform/Timer.cpp	(revision 239401)
>+++ Source/WebCore/platform/Timer.cpp	(working copy)
>@@ -50,7 +50,7 @@
> // Then we set a single shared system timer to fire at that time.
> //
> // When a timer's "next fire time" changes, we need to move it around in the priority queue.
>-static Vector<TimerBase*>& threadGlobalTimerHeap()
>+static Vector<WeakPtr<TimerBase>>& threadGlobalTimerHeap()
> {
>     return threadGlobalData().threadTimers().timerHeap();
> }
>@@ -58,52 +58,68 @@
> 
> class TimerHeapPointer {
> public:
>-    TimerHeapPointer(TimerBase** pointer) : m_pointer(pointer) { }
>+    TimerHeapPointer(WeakPtr<TimerBase>* pointer) : m_pointer(pointer) { }
>     TimerHeapReference operator*() const;
>-    TimerBase* operator->() const { return *m_pointer; }
>+    WeakPtr<TimerBase>& operator->() const { return *m_pointer; }
> private:
>-    TimerBase** m_pointer;
>+    WeakPtr<TimerBase>* m_pointer;
> };
> 
> class TimerHeapReference {
> public:
>-    TimerHeapReference(TimerBase*& reference) : m_reference(reference) { }
>-    operator TimerBase*() const { return m_reference; }
>+    TimerHeapReference(WeakPtr<TimerBase>& reference)
>+        : m_reference(reference)
>+    { }
>+
>+    TimerHeapReference(const TimerHeapReference& other)
>+        : m_reference(other.m_reference)
>+    { }
>+
>+    operator WeakPtr<TimerBase>&() const { return m_reference; }
>     TimerHeapPointer operator&() const { return &m_reference; }
>-    TimerHeapReference& operator=(TimerBase*);
>-    TimerHeapReference& operator=(TimerHeapReference);
>+    TimerHeapReference& operator=(TimerHeapReference&&);
>+    TimerHeapReference& operator=(WeakPtr<TimerBase>&&);
>+
>+    void updateHeapIndex();
>+
> private:
>-    TimerBase*& m_reference;
>+    WeakPtr<TimerBase>& m_reference;
> };
> 
> inline TimerHeapReference TimerHeapPointer::operator*() const
> {
>-    return *m_pointer;
>+    return TimerHeapReference { *m_pointer };
> }
> 
>-inline TimerHeapReference& TimerHeapReference::operator=(TimerBase* timer)
>+inline TimerHeapReference& TimerHeapReference::operator=(TimerHeapReference&& other)
> {
>-    m_reference = timer;
>-    Vector<TimerBase*>& heap = timer->timerHeap();
>-    if (&m_reference >= heap.data() && &m_reference < heap.data() + heap.size())
>-        timer->m_heapIndex = &m_reference - heap.data();
>+    m_reference = WTFMove(other.m_reference);
>+    updateHeapIndex();
>     return *this;
> }
> 
>-inline TimerHeapReference& TimerHeapReference::operator=(TimerHeapReference b)
>+inline TimerHeapReference& TimerHeapReference::operator=(WeakPtr<TimerBase>&& timer)
> {
>-    TimerBase* timer = b;
>-    return *this = timer;
>+    m_reference = WTFMove(timer);
>+    updateHeapIndex();
>+    return *this;
> }
> 
>+inline void TimerHeapReference::updateHeapIndex()
>+{
>+    Vector<WeakPtr<TimerBase>>& heap = m_reference->timerHeap();
>+    if (&m_reference >= heap.data() && &m_reference < heap.data() + heap.size())
>+        m_reference->m_heapIndex = &m_reference - heap.data();
>+}
>+
> inline void swap(TimerHeapReference a, TimerHeapReference b)
> {
>-    TimerBase* timerA = a;
>-    TimerBase* timerB = b;
>+    WeakPtr<TimerBase> timerA = a;
>+    WeakPtr<TimerBase> timerB = b;
> 
>     // Invoke the assignment operator, since that takes care of updating m_heapIndex.
>-    a = timerB;
>-    b = timerA;
>+    a = WTFMove(timerB);
>+    b = WTFMove(timerA);
> }
> 
> // ----------------
>@@ -110,9 +126,9 @@
> 
> // Class to represent iterators in the heap when calling the standard library heap algorithms.
> // Uses a custom pointer and reference type that update indices for pointers in the heap.
>-class TimerHeapIterator : public std::iterator<std::random_access_iterator_tag, TimerBase*, ptrdiff_t, TimerHeapPointer, TimerHeapReference> {
>+class TimerHeapIterator : public std::iterator<std::random_access_iterator_tag, WeakPtr<TimerBase>, ptrdiff_t, TimerHeapPointer, TimerHeapReference> {
> public:
>-    explicit TimerHeapIterator(TimerBase** pointer) : m_pointer(pointer) { checkConsistency(); }
>+    explicit TimerHeapIterator(WeakPtr<TimerBase>* pointer) : m_pointer(pointer) { checkConsistency(); }
> 
>     TimerHeapIterator& operator++() { checkConsistency(); ++m_pointer; checkConsistency(); return *this; }
>     TimerHeapIterator operator++(int) { checkConsistency(1); return TimerHeapIterator(m_pointer++); }
>@@ -125,7 +141,7 @@
> 
>     TimerHeapReference operator*() const { return TimerHeapReference(*m_pointer); }
>     TimerHeapReference operator[](ptrdiff_t i) const { return TimerHeapReference(m_pointer[i]); }
>-    TimerBase* operator->() const { return *m_pointer; }
>+    WeakPtr<TimerBase>& operator->() const { return *m_pointer; }
> 
> private:
>     void checkConsistency(ptrdiff_t offset = 0) const
>@@ -149,7 +165,7 @@
>     friend TimerHeapIterator operator-(TimerHeapIterator, size_t);
>     friend ptrdiff_t operator-(TimerHeapIterator, TimerHeapIterator);
> 
>-    TimerBase** m_pointer;
>+    WeakPtr<TimerBase>* m_pointer;
> };
> 
> inline bool operator==(TimerHeapIterator a, TimerHeapIterator b) { return a.m_pointer == b.m_pointer; }
>@@ -169,20 +185,27 @@
> 
> class TimerHeapLessThanFunction {
> public:
>-    bool operator()(const TimerBase*, const TimerBase*) const;
>+    static bool compare(const TimerBase* a, const TimerBase* b);
>+
>+    bool operator()(const WeakPtr<TimerBase> a, const WeakPtr<TimerBase> b) const
>+    {
>+        return compare(a.get(), b.get());
>+    }
> };
> 
>-inline bool TimerHeapLessThanFunction::operator()(const TimerBase* a, const TimerBase* b) const
>+inline bool TimerHeapLessThanFunction::compare(const TimerBase* a, const TimerBase* b)
> {
>-    // The comparisons below are "backwards" because the heap puts the largest 
>+    ASSERT(!a->m_wasDeleted);
>+    ASSERT(!b->m_wasDeleted);
>+    // The comparisons below are "backwards" because the heap puts the largest
>     // element first and we want the lowest time to be the first one in the heap.
>     MonotonicTime aFireTime = a->m_nextFireTime;
>     MonotonicTime bFireTime = b->m_nextFireTime;
>     if (bFireTime != aFireTime)
>         return bFireTime < aFireTime;
>-    
>-    // We need to look at the difference of the insertion orders instead of comparing the two 
>-    // outright in case of overflow. 
>+
>+    // We need to look at the difference of the insertion orders instead of comparing the two
>+    // outright in case of overflow.
>     unsigned difference = a->m_heapInsertionOrder - b->m_heapInsertionOrder;
>     return difference < std::numeric_limits<unsigned>::max() / 2;
> }
>@@ -251,6 +274,12 @@
>     ASSERT(m_heapIndex >= 0);
>     ASSERT(m_heapIndex < static_cast<int>(timerHeap().size()));
>     ASSERT(timerHeap()[m_heapIndex] == this);
>+#if !ASSERT_DISABLED
>+    ASSERT(timerHeap().size() < INT_MAX);
>+    int size = timerHeap().size();
>+    for (int i = 0; i < size; i++)
>+        ASSERT(timerHeap()[i]->m_heapIndex == i);
>+#endif
> }
> 
> inline void TimerBase::checkConsistency() const
>@@ -265,7 +294,7 @@
> {
>     ASSERT(static_cast<bool>(m_nextFireTime));
>     checkHeapIndex();
>-    TimerBase** heapData = timerHeap().data();
>+    WeakPtr<TimerBase>* heapData = timerHeap().data();
>     push_heap(TimerHeapIterator(heapData), TimerHeapIterator(heapData + m_heapIndex + 1), TimerHeapLessThanFunction());
>     checkHeapIndex();
> }
>@@ -296,7 +325,7 @@
> inline void TimerBase::heapInsert()
> {
>     ASSERT(!inHeap());
>-    timerHeap().append(this);
>+    timerHeap().append(makeWeakPtr(this));
>     m_heapIndex = timerHeap().size() - 1;
>     heapDecreaseKey();
> }
>@@ -315,28 +344,33 @@
> {
>     ASSERT(this == timerHeap().first());
>     checkHeapIndex();
>-    Vector<TimerBase*>& heap = timerHeap();
>-    TimerBase** heapData = heap.data();
>+    auto& heap = timerHeap();
>+    auto* heapData = heap.data();
>     pop_heap(TimerHeapIterator(heapData), TimerHeapIterator(heapData + heap.size()), TimerHeapLessThanFunction());
>     checkHeapIndex();
>     ASSERT(this == timerHeap().last());
> }
> 
>-static inline bool parentHeapPropertyHolds(const TimerBase* current, const Vector<TimerBase*>& heap, unsigned currentIndex)
>+void TimerBase::heapPopNullMin(Vector<WeakPtr<TimerBase>>& heap)
> {
>+    RELEASE_ASSERT(!heap.first());
>+    auto* heapData = heap.data();
>+    pop_heap(TimerHeapIterator(heapData), TimerHeapIterator(heapData + heap.size()), TimerHeapLessThanFunction());
>+}
>+
>+static inline bool parentHeapPropertyHolds(const TimerBase* current, const Vector<WeakPtr<TimerBase>>& heap, unsigned currentIndex)
>+{
>     if (!currentIndex)
>         return true;
>     unsigned parentIndex = (currentIndex - 1) / 2;
>-    TimerHeapLessThanFunction compareHeapPosition;
>-    return compareHeapPosition(current, heap[parentIndex]);
>+    return TimerHeapLessThanFunction::compare(current, heap[parentIndex].get());
> }
> 
>-static inline bool childHeapPropertyHolds(const TimerBase* current, const Vector<TimerBase*>& heap, unsigned childIndex)
>+static inline bool childHeapPropertyHolds(const TimerBase* current, const Vector<WeakPtr<TimerBase>>& heap, unsigned childIndex)
> {
>     if (childIndex >= heap.size())
>         return true;
>-    TimerHeapLessThanFunction compareHeapPosition;
>-    return compareHeapPosition(heap[childIndex], current);
>+    return TimerHeapLessThanFunction::compare(heap[childIndex].get(), current);
> }
> 
> bool TimerBase::hasValidHeapPosition() const
>@@ -347,7 +381,7 @@
>     // Check if the heap property still holds with the new fire time. If it does we don't need to do anything.
>     // This assumes that the STL heap is a standard binary heap. In an unlikely event it is not, the assertions
>     // in updateHeapIfNeeded() will get hit.
>-    const Vector<TimerBase*>& heap = timerHeap();
>+    const auto& heap = timerHeap();
>     if (!parentHeapPropertyHolds(this, heap, m_heapIndex))
>         return false;
>     unsigned childIndex1 = 2 * m_heapIndex + 1;
>Index: Source/WebCore/platform/Timer.h
>===================================================================
>--- Source/WebCore/platform/Timer.h	(revision 239401)
>+++ Source/WebCore/platform/Timer.h	(working copy)
>@@ -33,6 +33,7 @@
> #include <wtf/Seconds.h>
> #include <wtf/Threading.h>
> #include <wtf/Vector.h>
>+#include <wtf/WeakPtr.h>
> 
> #if PLATFORM(IOS_FAMILY)
> #include "WebCoreThread.h"
>@@ -44,7 +45,7 @@
> 
> class TimerHeapElement;
> 
>-class TimerBase {
>+class TimerBase : public CanMakeWeakPtr<TimerBase> {
>     WTF_MAKE_NONCOPYABLE(TimerBase);
>     WTF_MAKE_FAST_ALLOCATED;
> public:
>@@ -92,8 +93,9 @@
>     void heapInsert();
>     void heapPop();
>     void heapPopMin();
>+    static void heapPopNullMin(Vector<WeakPtr<TimerBase>>&);
> 
>-    Vector<TimerBase*>& timerHeap() const { ASSERT(m_cachedThreadGlobalTimerHeap); return *m_cachedThreadGlobalTimerHeap; }
>+    Vector<WeakPtr<TimerBase>>& timerHeap() const { ASSERT(m_cachedThreadGlobalTimerHeap); return *m_cachedThreadGlobalTimerHeap; }
> 
>     MonotonicTime m_nextFireTime; // 0 if inactive
>     MonotonicTime m_unalignedNextFireTime; // m_nextFireTime not considering alignment interval
>@@ -101,7 +103,7 @@
>     signed int m_heapIndex : 31; // -1 if not in heap
>     bool m_wasDeleted : 1;
>     unsigned m_heapInsertionOrder; // Used to keep order among equal-fire-time timers
>-    Vector<TimerBase*>* m_cachedThreadGlobalTimerHeap { nullptr };
>+    Vector<WeakPtr<TimerBase>>* m_cachedThreadGlobalTimerHeap { nullptr };
> 
>     Ref<Thread> m_thread { Thread::current() };
>

Actions: View | Formatted Diff | Diff

Attachments on bug 192975: 357931 | 357980 | 358010 | 358055 | 358766 | 358771