Title: [260477] trunk/Source/WTF
Revision
260477
Author
hironori.fu...@sony.com
Date
2020-04-21 17:29:33 -0700 (Tue, 21 Apr 2020)

Log Message

[Win] Use generic WorkQueue instead of WorkQueueWin.cpp
https://bugs.webkit.org/show_bug.cgi?id=210785

Reviewed by Darin Adler.

WorkQueueWin was using random threads to execute dispatched
functions. This is not desired for IPC::Connection because it
needs to call CancelIo API in the same thread started aync
ReadFile operations.

Implemented RunLoop::dispatchAfter in RunLoopWin.cpp in order to
use generic WorkQueue.

* wtf/PlatformWin.cmake:
* wtf/RunLoop.h: Added DispatchTimer class for USE(WINDOWS_EVENT_LOOP).
(WTF::RunLoop::DispatchTimer::DispatchTimer):
(WTF::RunLoop::DispatchTimer::setFunction):
* wtf/WorkQueue.h: Removed code for USE(WINDOWS_EVENT_LOOP).
* wtf/win/RunLoopWin.cpp:
(WTF::RunLoop::dispatchAfter): Added.
* wtf/win/WorkQueueWin.cpp: Removed.

Modified Paths

Removed Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (260476 => 260477)


--- trunk/Source/WTF/ChangeLog	2020-04-22 00:27:35 UTC (rev 260476)
+++ trunk/Source/WTF/ChangeLog	2020-04-22 00:29:33 UTC (rev 260477)
@@ -1,3 +1,27 @@
+2020-04-21  Fujii Hironori  <hironori.fu...@sony.com>
+
+        [Win] Use generic WorkQueue instead of WorkQueueWin.cpp
+        https://bugs.webkit.org/show_bug.cgi?id=210785
+
+        Reviewed by Darin Adler.
+
+        WorkQueueWin was using random threads to execute dispatched
+        functions. This is not desired for IPC::Connection because it
+        needs to call CancelIo API in the same thread started aync
+        ReadFile operations.
+
+        Implemented RunLoop::dispatchAfter in RunLoopWin.cpp in order to
+        use generic WorkQueue.
+
+        * wtf/PlatformWin.cmake:
+        * wtf/RunLoop.h: Added DispatchTimer class for USE(WINDOWS_EVENT_LOOP).
+        (WTF::RunLoop::DispatchTimer::DispatchTimer):
+        (WTF::RunLoop::DispatchTimer::setFunction):
+        * wtf/WorkQueue.h: Removed code for USE(WINDOWS_EVENT_LOOP).
+        * wtf/win/RunLoopWin.cpp:
+        (WTF::RunLoop::dispatchAfter): Added.
+        * wtf/win/WorkQueueWin.cpp: Removed.
+
 2020-04-21  Charlie Turner  <ctur...@igalia.com>
 
         [Clang 10] Fix warning: definition of implicit copy assignment operator for 'PageBlock' is deprecated because it has a user-declared copy constructor

Modified: trunk/Source/WTF/wtf/PlatformWin.cmake (260476 => 260477)


--- trunk/Source/WTF/wtf/PlatformWin.cmake	2020-04-22 00:27:35 UTC (rev 260476)
+++ trunk/Source/WTF/wtf/PlatformWin.cmake	2020-04-22 00:29:33 UTC (rev 260477)
@@ -8,6 +8,8 @@
 )
 
 list(APPEND WTF_SOURCES
+    generic/WorkQueueGeneric.cpp
+
     text/win/StringWin.cpp
     text/win/TextBreakIteratorInternalICUWin.cpp
 
@@ -22,7 +24,6 @@
     win/PathWalker.cpp
     win/RunLoopWin.cpp
     win/ThreadingWin.cpp
-    win/WorkQueueWin.cpp
 )
 
 list(APPEND WTF_LIBRARIES

Modified: trunk/Source/WTF/wtf/RunLoop.h (260476 => 260477)


--- trunk/Source/WTF/wtf/RunLoop.h	2020-04-22 00:27:35 UTC (rev 260476)
+++ trunk/Source/WTF/wtf/RunLoop.h	2020-04-22 00:29:33 UTC (rev 260477)
@@ -96,7 +96,7 @@
     static void registerRunLoopMessageWindowClass();
 #endif
 
-#if USE(GLIB_EVENT_LOOP) || USE(GENERIC_EVENT_LOOP)
+#if !USE(COCOA_EVENT_LOOP)
     WTF_EXPORT_PRIVATE void dispatchAfter(Seconds, Function<void()>&&);
 #endif
 
@@ -176,6 +176,25 @@
         TimerFiredClass* m_object;
     };
 
+#if USE(WINDOWS_EVENT_LOOP)
+    class DispatchTimer : public TimerBase {
+    public:
+        DispatchTimer(RunLoop& runLoop)
+            : TimerBase(runLoop)
+        {
+        }
+
+        void setFunction(Function<void()>&& function)
+        {
+            m_function = WTFMove(function);
+        }
+    private:
+        void fired() override { m_function(); }
+
+        Function<void()> m_function;
+    };
+#endif
+
     class Holder;
 
 private:

Modified: trunk/Source/WTF/wtf/WorkQueue.h (260476 => 260477)


--- trunk/Source/WTF/wtf/WorkQueue.h	2020-04-22 00:27:35 UTC (rev 260476)
+++ trunk/Source/WTF/wtf/WorkQueue.h	2020-04-22 00:29:33 UTC (rev 260477)
@@ -34,16 +34,7 @@
 
 #if USE(COCOA_EVENT_LOOP)
 #include <dispatch/dispatch.h>
-#endif
-
-#if USE(WINDOWS_EVENT_LOOP)
-#include <wtf/HashMap.h>
-#include <wtf/ThreadingPrimitives.h>
-#include <wtf/Vector.h>
-#endif
-
-#if USE(GLIB_EVENT_LOOP) || USE(GENERIC_EVENT_LOOP)
-#include <wtf/Condition.h>
+#else
 #include <wtf/RunLoop.h>
 #endif
 
@@ -74,7 +65,7 @@
 
 #if USE(COCOA_EVENT_LOOP)
     dispatch_queue_t dispatchQueue() const { return m_dispatchQueue; }
-#elif USE(GLIB_EVENT_LOOP) || USE(GENERIC_EVENT_LOOP)
+#else
     RunLoop& runLoop() const { return *m_runLoop; }
 #endif
 
@@ -84,26 +75,10 @@
     void platformInitialize(const char* name, Type, QOS);
     void platformInvalidate();
 
-#if USE(WINDOWS_EVENT_LOOP)
-    static void CALLBACK timerCallback(void* context, BOOLEAN timerOrWaitFired);
-    static DWORD WINAPI workThreadCallback(void* context);
-
-    bool tryRegisterAsWorkThread();
-    void unregisterAsWorkThread();
-    void performWorkOnRegisteredWorkThread();
-#endif
-
 #if USE(COCOA_EVENT_LOOP)
     static void executeFunction(void*);
     dispatch_queue_t m_dispatchQueue;
-#elif USE(WINDOWS_EVENT_LOOP)
-    volatile LONG m_isWorkThreadRegistered;
-
-    Lock m_functionQueueLock;
-    Vector<Function<void()>> m_functionQueue;
-
-    HANDLE m_timerQueue;
-#elif USE(GLIB_EVENT_LOOP) || USE(GENERIC_EVENT_LOOP)
+#else
     RunLoop* m_runLoop;
 #endif
 };

Modified: trunk/Source/WTF/wtf/win/RunLoopWin.cpp (260476 => 260477)


--- trunk/Source/WTF/wtf/win/RunLoopWin.cpp	2020-04-22 00:27:35 UTC (rev 260476)
+++ trunk/Source/WTF/wtf/win/RunLoopWin.cpp	2020-04-22 00:29:33 UTC (rev 260477)
@@ -140,6 +140,16 @@
     return CycleResult::Continue;
 }
 
+void RunLoop::dispatchAfter(Seconds delay, Function<void()>&& function)
+{
+    auto timer = new DispatchTimer(*this);
+    timer->setFunction([timer, function = WTFMove(function)] {
+        function();
+        delete timer;
+    });
+    timer->startOneShot(delay);
+}
+
 // RunLoop::Timer
 
 void RunLoop::TimerBase::timerFired()

Deleted: trunk/Source/WTF/wtf/win/WorkQueueWin.cpp (260476 => 260477)


--- trunk/Source/WTF/wtf/win/WorkQueueWin.cpp	2020-04-22 00:27:35 UTC (rev 260476)
+++ trunk/Source/WTF/wtf/win/WorkQueueWin.cpp	2020-04-22 00:29:33 UTC (rev 260477)
@@ -1,190 +0,0 @@
-/*
- * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
- * Copyright (C) 2017 Sony Interactive Entertainment Inc.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
- * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
- * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
- * THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include "config.h"
-#include <wtf/WorkQueue.h>
-
-#include <wtf/MathExtras.h>
-#include <wtf/Threading.h>
-
-namespace WTF {
-
-DWORD WorkQueue::workThreadCallback(void* context)
-{
-    ASSERT_ARG(context, context);
-
-    WorkQueue* queue = static_cast<WorkQueue*>(context);
-
-    if (queue->tryRegisterAsWorkThread())
-        queue->performWorkOnRegisteredWorkThread();
-
-    queue->deref();
-
-    return 0;
-}
-
-void WorkQueue::performWorkOnRegisteredWorkThread()
-{
-    ASSERT(m_isWorkThreadRegistered);
-
-    m_functionQueueLock.lock();
-
-    while (!m_functionQueue.isEmpty()) {
-        Vector<Function<void()>> functionQueue;
-        m_functionQueue.swap(functionQueue);
-
-        // Allow more work to be scheduled while we're not using the queue directly.
-        m_functionQueueLock.unlock();
-        for (auto& function : functionQueue)
-            function();
-        m_functionQueueLock.lock();
-    }
-
-    // One invariant we maintain is that any work scheduled while a work thread is registered will
-    // be handled by that work thread. Unregister as the work thread while the queue lock is still
-    // held so that no work can be scheduled while we're still registered.
-    unregisterAsWorkThread();
-
-    m_functionQueueLock.unlock();
-}
-
-void WorkQueue::platformInitialize(const char* name, Type, QOS)
-{
-    m_isWorkThreadRegistered = 0;
-    m_timerQueue = ::CreateTimerQueue();
-    ASSERT_WITH_MESSAGE(m_timerQueue, "::CreateTimerQueue failed with error %lu", ::GetLastError());
-}
-
-bool WorkQueue::tryRegisterAsWorkThread()
-{
-    LONG result = ::InterlockedCompareExchange(&m_isWorkThreadRegistered, 1, 0);
-    ASSERT(!result || result == 1);
-    return !result;
-}
-
-void WorkQueue::unregisterAsWorkThread()
-{
-    LONG result = ::InterlockedCompareExchange(&m_isWorkThreadRegistered, 0, 1);
-    ASSERT_UNUSED(result, result == 1);
-}
-
-void WorkQueue::platformInvalidate()
-{
-    // FIXME: We need to ensure that any timer-queue timers that fire after this point don't try to
-    // access this WorkQueue <http://webkit.org/b/44690>.
-    ::DeleteTimerQueueEx(m_timerQueue, 0);
-}
-
-void WorkQueue::dispatch(Function<void()>&& function)
-{
-    auto locker = holdLock(m_functionQueueLock);
-    m_functionQueue.append(WTFMove(function));
-
-    // Spawn a work thread to perform the work we just added. As an optimization, we avoid
-    // spawning the thread if a work thread is already registered. This prevents multiple work
-    // threads from being spawned in most cases. (Note that when a work thread has been spawned but
-    // hasn't registered itself yet, m_isWorkThreadRegistered will be false and we'll end up
-    // spawning a second work thread here. But work thread registration process will ensure that
-    // only one thread actually ends up performing work.)
-    if (!m_isWorkThreadRegistered) {
-        ref();
-        ::QueueUserWorkItem(workThreadCallback, this, WT_EXECUTEDEFAULT);
-    }
-}
-
-struct TimerContext : public ThreadSafeRefCounted<TimerContext> {
-    static Ref<TimerContext> create() { return adoptRef(*new TimerContext); }
-
-    Lock timerLock;
-    WorkQueue* queue { nullptr };
-    HANDLE timer { nullptr };
-    Function<void()> function;
-
-private:
-    TimerContext() = default;
-};
-
-void WorkQueue::timerCallback(void* context, BOOLEAN timerOrWaitFired)
-{
-    ASSERT_ARG(context, context);
-    ASSERT_UNUSED(timerOrWaitFired, timerOrWaitFired);
-
-    // Balanced by leakRef in scheduleWorkAfterDelay.
-    RefPtr<TimerContext> timerContext = adoptRef(static_cast<TimerContext*>(context));
-
-    timerContext->queue->dispatch(WTFMove(timerContext->function));
-
-    auto locker = holdLock(timerContext->timerLock);
-    ASSERT(timerContext->timer);
-    ASSERT(timerContext->queue->m_timerQueue);
-    if (!::DeleteTimerQueueTimer(timerContext->queue->m_timerQueue, timerContext->timer, 0)) {
-        // Getting ERROR_IO_PENDING here means that the timer will be destroyed once the callback is done executing.
-        ASSERT_WITH_MESSAGE(::GetLastError() == ERROR_IO_PENDING, "::DeleteTimerQueueTimer failed with error %lu", ::GetLastError());
-    }
-}
-
-void WorkQueue::dispatchAfter(Seconds duration, Function<void()>&& function)
-{
-    ASSERT(m_timerQueue);
-    ref();
-
-    Ref<TimerContext> context = TimerContext::create();
-    context->queue = this;
-    context->function = WTFMove(function);
-
-    {
-        // The timer callback could fire before ::CreateTimerQueueTimer even returns, so we protect
-        // context->timer with a mutex to ensure the timer callback doesn't access it before the
-        // timer handle has been stored in it.
-        auto locker = holdLock(context->timerLock);
-
-        int64_t milliseconds = duration.milliseconds();
-
-        // From empirical testing, we've seen CreateTimerQueueTimer() sometimes fire up to 5+ ms early.
-        // This causes havoc for clients of this code that expect to not be called back until the
-        // specified duration has expired. Other folks online have also observed some slop in the
-        // firing times of CreateTimerQuqueTimer(). From the data posted at
-        // http://omeg.pl/blog/2011/11/on-winapi-timers-and-their-resolution, it appears that the slop
-        // can be up to about 10 ms. To ensure that we don't fire the timer early, we'll tack on a
-        // slop adjustment to the duration, and we'll use double the worst amount of slop observed
-        // so far.
-        const int64_t slopAdjustment = 20;
-        if (milliseconds) 
-            milliseconds += slopAdjustment;
-
-        // Since our timer callback is quick, we can execute in the timer thread itself and avoid
-        // an extra thread switch over to a worker thread.
-        if (!::CreateTimerQueueTimer(&context->timer, m_timerQueue, timerCallback, context.ptr(), clampTo<DWORD>(milliseconds), 0, WT_EXECUTEINTIMERTHREAD)) {
-            ASSERT_WITH_MESSAGE(false, "::CreateTimerQueueTimer failed with error %lu", ::GetLastError());
-            return;
-        }
-    }
-
-    // The timer callback will handle destroying context.
-    context.leakRef();
-}
-
-} // namespace WTF
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to