Changeset 240042 in webkit


Ignore:
Timestamp:
Jan 16, 2019 11:07:22 AM (5 years ago)
Author:
youenn@apple.com
Message:

Prevent WorkerRunLoop::runInMode from spinning in nested cases
https://bugs.webkit.org/show_bug.cgi?id=193359
<rdar://problem/46345353>

Reviewed by Joseph Pecoraro.

Speculative fix for some cases where service worker is spinning and consuming a lot of CPU.
The hypothesis is that:

  • Service Worker is checking for its script freshness through WorkerScriptLoader.

This triggers the worker run loop to be nested.

  • The run loop timer is active and needs to fire immediately.

The hypothesis is that this happens in some cases like restarting a device after sleep mode.

WorkerRunLoop::runInMode will then compute a 0 timeout value for getting a message.
This will trigger a timeout while waiting for the message queue.
Since the run loop is nested, the run loop timer will not be able to fire,
and it will keep ask to fire immediately.
runInMode will return timeout as a result and WorkerRunLoop::run will call it immediately.

The fix is to prevent the shared timer to fire only when the run loop is being debugged through the web inspector.
We compute this by checking the run loop mode as debuggerMode().
Did some refactoring by introducing helper routines for running the loop and posting task in debugger mode.

  • inspector/WorkerScriptDebugServer.cpp:

(WebCore::WorkerScriptDebugServer::runEventLoopWhilePaused):

  • workers/WorkerInspectorProxy.cpp:

(WebCore::WorkerInspectorProxy::resumeWorkerIfPaused):
(WebCore::WorkerInspectorProxy::connectToWorkerInspectorController):
(WebCore::WorkerInspectorProxy::disconnectFromWorkerInspectorController):
(WebCore::WorkerInspectorProxy::sendMessageToWorkerInspectorController):

  • workers/WorkerRunLoop.cpp:

(WebCore::ModePredicate::ModePredicate):
(WebCore::WorkerRunLoop::WorkerRunLoop):
(WebCore::debuggerMode):
(WebCore::RunLoopSetup::RunLoopSetup):
(WebCore::RunLoopSetup::~RunLoopSetup):
(WebCore::WorkerRunLoop::run):
(WebCore::WorkerRunLoop::runInDebuggerMode):
(WebCore::WorkerRunLoop::runInMode):
(WebCore::WorkerRunLoop::Task::performTask):

  • workers/WorkerRunLoop.h:

(WebCore::WorkerRunLoop::isBeingDebugged const):

  • workers/WorkerThread.cpp:

(WebCore::WorkerThread::startRunningDebuggerTasks):

  • workers/service/context/ServiceWorkerInspectorProxy.cpp:

(WebCore::ServiceWorkerInspectorProxy::connectToWorker):
(WebCore::ServiceWorkerInspectorProxy::disconnectFromWorker):
(WebCore::ServiceWorkerInspectorProxy::sendMessageToWorker):

Location:
trunk/Source/WebCore
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r240039 r240042  
     12019-01-16  Youenn Fablet  <youenn@apple.com>
     2
     3        Prevent WorkerRunLoop::runInMode from spinning in nested cases
     4        https://bugs.webkit.org/show_bug.cgi?id=193359
     5        <rdar://problem/46345353>
     6
     7        Reviewed by Joseph Pecoraro.
     8
     9        Speculative fix for some cases where service worker is spinning and consuming a lot of CPU.
     10        The hypothesis is that:
     11        - Service Worker is checking for its script freshness through WorkerScriptLoader.
     12        This triggers the worker run loop to be nested.
     13        - The run loop timer is active and needs to fire immediately.
     14        The hypothesis is that this happens in some cases like restarting a device after sleep mode.
     15
     16        WorkerRunLoop::runInMode will then compute a 0 timeout value for getting a message.
     17        This will trigger a timeout while waiting for the message queue.
     18        Since the run loop is nested,  the run loop timer will not be able to fire,
     19        and it will keep ask to fire immediately.
     20        runInMode will return timeout as a result and WorkerRunLoop::run will call it immediately.
     21
     22        The fix is to prevent the shared timer to fire only when the run loop is being debugged through the web inspector.
     23        We compute this by checking the run loop mode as debuggerMode().
     24        Did some refactoring by introducing helper routines for running the loop and posting task in debugger mode.
     25
     26        * inspector/WorkerScriptDebugServer.cpp:
     27        (WebCore::WorkerScriptDebugServer::runEventLoopWhilePaused):
     28        * workers/WorkerInspectorProxy.cpp:
     29        (WebCore::WorkerInspectorProxy::resumeWorkerIfPaused):
     30        (WebCore::WorkerInspectorProxy::connectToWorkerInspectorController):
     31        (WebCore::WorkerInspectorProxy::disconnectFromWorkerInspectorController):
     32        (WebCore::WorkerInspectorProxy::sendMessageToWorkerInspectorController):
     33        * workers/WorkerRunLoop.cpp:
     34        (WebCore::ModePredicate::ModePredicate):
     35        (WebCore::WorkerRunLoop::WorkerRunLoop):
     36        (WebCore::debuggerMode):
     37        (WebCore::RunLoopSetup::RunLoopSetup):
     38        (WebCore::RunLoopSetup::~RunLoopSetup):
     39        (WebCore::WorkerRunLoop::run):
     40        (WebCore::WorkerRunLoop::runInDebuggerMode):
     41        (WebCore::WorkerRunLoop::runInMode):
     42        (WebCore::WorkerRunLoop::Task::performTask):
     43        * workers/WorkerRunLoop.h:
     44        (WebCore::WorkerRunLoop::isBeingDebugged const):
     45        * workers/WorkerThread.cpp:
     46        (WebCore::WorkerThread::startRunningDebuggerTasks):
     47        * workers/service/context/ServiceWorkerInspectorProxy.cpp:
     48        (WebCore::ServiceWorkerInspectorProxy::connectToWorker):
     49        (WebCore::ServiceWorkerInspectorProxy::disconnectFromWorker):
     50        (WebCore::ServiceWorkerInspectorProxy::sendMessageToWorker):
     51
    1522019-01-16  Sihui Liu  <sihui_liu@apple.com>
    253
  • trunk/Source/WebCore/inspector/WorkerScriptDebugServer.cpp

    r228218 r240042  
    7575    MessageQueueWaitResult result;
    7676    do {
    77         result = m_workerGlobalScope.thread().runLoop().runInMode(&m_workerGlobalScope, WorkerRunLoop::debuggerMode());
     77        result = m_workerGlobalScope.thread().runLoop().runInDebuggerMode(m_workerGlobalScope);
    7878    } while (result != MessageQueueTerminated && !m_doneProcessingDebuggerEvents);
    7979}
  • trunk/Source/WebCore/workers/WorkerInspectorProxy.cpp

    r228218 r240042  
    9191void WorkerInspectorProxy::resumeWorkerIfPaused()
    9292{
    93     m_workerThread->runLoop().postTaskForMode([] (ScriptExecutionContext& context) {
     93    m_workerThread->runLoop().postDebuggerTask([] (ScriptExecutionContext& context) {
    9494        downcast<WorkerGlobalScope>(context).thread().stopRunningDebuggerTasks();
    95     }, WorkerRunLoop::debuggerMode());
     95    });
    9696}
    9797
     
    104104    m_pageChannel = channel;
    105105
    106     m_workerThread->runLoop().postTaskForMode([] (ScriptExecutionContext& context) {
     106    m_workerThread->runLoop().postDebuggerTask([] (ScriptExecutionContext& context) {
    107107        downcast<WorkerGlobalScope>(context).inspectorController().connectFrontend();
    108     }, WorkerRunLoop::debuggerMode());
     108    });
    109109}
    110110
     
    117117    m_pageChannel = nullptr;
    118118
    119     m_workerThread->runLoop().postTaskForMode([] (ScriptExecutionContext& context) {
     119    m_workerThread->runLoop().postDebuggerTask([] (ScriptExecutionContext& context) {
    120120        downcast<WorkerGlobalScope>(context).inspectorController().disconnectFrontend(DisconnectReason::InspectorDestroyed);
    121121
     
    123123        // the pause since this will be the last debugger task we send to the worker.
    124124        downcast<WorkerGlobalScope>(context).thread().stopRunningDebuggerTasks();
    125     }, WorkerRunLoop::debuggerMode());
     125    });
    126126}
    127127
     
    132132        return;
    133133
    134     m_workerThread->runLoop().postTaskForMode([message = message.isolatedCopy()] (ScriptExecutionContext& context) {
     134    m_workerThread->runLoop().postDebuggerTask([message = message.isolatedCopy()] (ScriptExecutionContext& context) {
    135135        downcast<WorkerGlobalScope>(context).inspectorController().dispatchMessageFromFrontend(message);
    136     }, WorkerRunLoop::debuggerMode());
     136    });
    137137}
    138138
  • trunk/Source/WebCore/workers/WorkerRunLoop.cpp

    r233122 r240042  
    6565class ModePredicate {
    6666public:
    67     ModePredicate(const String& mode)
    68         : m_mode(mode)
    69         , m_defaultMode(mode == WorkerRunLoop::defaultMode())
     67    explicit ModePredicate(String&& mode)
     68        : m_mode(WTFMove(mode))
     69        , m_defaultMode(m_mode == WorkerRunLoop::defaultMode())
    7070    {
    7171    }
     
    8888WorkerRunLoop::WorkerRunLoop()
    8989    : m_sharedTimer(std::make_unique<WorkerSharedTimer>())
    90     , m_nestedCount(0)
    91     , m_uniqueId(0)
    9290{
    9391}
     
    103101}
    104102
    105 String WorkerRunLoop::debuggerMode()
     103static String debuggerMode()
    106104{
    107105    return "debugger"_s;
     
    111109    WTF_MAKE_NONCOPYABLE(RunLoopSetup);
    112110public:
    113     RunLoopSetup(WorkerRunLoop& runLoop)
     111    enum class IsForDebugging { No, Yes };
     112    RunLoopSetup(WorkerRunLoop& runLoop, IsForDebugging isForDebugging)
    114113        : m_runLoop(runLoop)
     114        , m_isForDebugging(isForDebugging)
    115115    {
    116116        if (!m_runLoop.m_nestedCount)
    117117            threadGlobalData().threadTimers().setSharedTimer(m_runLoop.m_sharedTimer.get());
    118118        m_runLoop.m_nestedCount++;
     119        if (m_isForDebugging == IsForDebugging::Yes)
     120            m_runLoop.m_debugCount++;
    119121    }
    120122
     
    124126        if (!m_runLoop.m_nestedCount)
    125127            threadGlobalData().threadTimers().setSharedTimer(nullptr);
     128        if (m_isForDebugging == IsForDebugging::Yes)
     129            m_runLoop.m_debugCount--;
    126130    }
    127131private:
    128132    WorkerRunLoop& m_runLoop;
     133    IsForDebugging m_isForDebugging { IsForDebugging::No };
    129134};
    130135
    131136void WorkerRunLoop::run(WorkerGlobalScope* context)
    132137{
    133     RunLoopSetup setup(*this);
     138    RunLoopSetup setup(*this, RunLoopSetup::IsForDebugging::No);
    134139    ModePredicate modePredicate(defaultMode());
    135140    MessageQueueWaitResult result;
     
    140145}
    141146
     147MessageQueueWaitResult WorkerRunLoop::runInDebuggerMode(WorkerGlobalScope& context)
     148{
     149    RunLoopSetup setup(*this, RunLoopSetup::IsForDebugging::Yes);
     150    return runInMode(&context, ModePredicate { debuggerMode() }, WaitForMessage);
     151}
     152
    142153MessageQueueWaitResult WorkerRunLoop::runInMode(WorkerGlobalScope* context, const String& mode, WaitMode waitMode)
    143154{
    144     RunLoopSetup setup(*this);
    145     ModePredicate modePredicate(mode);
     155    ASSERT(mode != debuggerMode());
     156    RunLoopSetup setup(*this, RunLoopSetup::IsForDebugging::No);
     157    ModePredicate modePredicate(String { mode });
    146158    MessageQueueWaitResult result = runInMode(context, modePredicate, waitMode);
    147159    return result;
     
    156168        // We don't actually do anything here, we just want to loop around runInMode
    157169        // to both recalculate our deadline and to potentially run the run loop.
    158         this->postTask([](ScriptExecutionContext&) { }); 
     170        this->postTask([](ScriptExecutionContext&) { });
    159171    });
    160172
     
    203215
    204216    case MessageQueueTimeout:
    205         if (!context->isClosing() && !isNested())
     217        if (!context->isClosing() && !isBeingDebugged())
    206218            m_sharedTimer->fire();
    207219        break;
     
    252264}
    253265
     266void WorkerRunLoop::postDebuggerTask(ScriptExecutionContext::Task&& task)
     267{
     268    postTaskForMode(WTFMove(task), debuggerMode());
     269}
     270
    254271void WorkerRunLoop::Task::performTask(WorkerGlobalScope* context)
    255272{
  • trunk/Source/WebCore/workers/WorkerRunLoop.h

    r216876 r240042  
    5454        // Waits for a single task and returns.
    5555        MessageQueueWaitResult runInMode(WorkerGlobalScope*, const String& mode, WaitMode = WaitForMessage);
     56        MessageQueueWaitResult runInDebuggerMode(WorkerGlobalScope&);
    5657
    5758        void terminate();
     
    6162        void postTaskAndTerminate(ScriptExecutionContext::Task&&);
    6263        void postTaskForMode(ScriptExecutionContext::Task&&, const String& mode);
     64        void postDebuggerTask(ScriptExecutionContext::Task&&);
    6365
    6466        unsigned long createUniqueId() { return ++m_uniqueId; }
    6567
    6668        static String defaultMode();
    67         static String debuggerMode();
    68 
    6969        class Task {
    7070            WTF_MAKE_NONCOPYABLE(Task); WTF_MAKE_FAST_ALLOCATED;
     
    9090        void runCleanupTasks(WorkerGlobalScope*);
    9191
    92         bool isNested() const { return m_nestedCount > 1; }
     92        bool isBeingDebugged() const { return m_debugCount >= 1; }
    9393
    9494        MessageQueue<Task> m_messageQueue;
    9595        std::unique_ptr<WorkerSharedTimer> m_sharedTimer;
    96         int m_nestedCount;
    97         unsigned long m_uniqueId;
     96        int m_nestedCount { 0 };
     97        int m_debugCount { 0 };
     98        unsigned long m_uniqueId { 0 };
    9899    };
    99100
  • trunk/Source/WebCore/workers/WorkerThread.cpp

    r239569 r240042  
    250250    MessageQueueWaitResult result;
    251251    do {
    252         result = m_runLoop.runInMode(m_workerGlobalScope.get(), WorkerRunLoop::debuggerMode());
     252        result = m_runLoop.runInDebuggerMode(*m_workerGlobalScope);
    253253    } while (result != MessageQueueTerminated && m_pausedForDebugger);
    254254}
  • trunk/Source/WebCore/workers/service/context/ServiceWorkerInspectorProxy.cpp

    r238206 r240042  
    6262    m_channel = &channel;
    6363
    64     m_serviceWorkerThreadProxy.thread().runLoop().postTaskForMode([] (ScriptExecutionContext& context) {
     64    m_serviceWorkerThreadProxy.thread().runLoop().postDebuggerTask([] (ScriptExecutionContext& context) {
    6565        downcast<WorkerGlobalScope>(context).inspectorController().connectFrontend();
    66     }, WorkerRunLoop::debuggerMode());
     66    });
    6767}
    6868
     
    7272    m_channel = nullptr;
    7373
    74     m_serviceWorkerThreadProxy.thread().runLoop().postTaskForMode([] (ScriptExecutionContext& context) {
     74    m_serviceWorkerThreadProxy.thread().runLoop().postDebuggerTask([] (ScriptExecutionContext& context) {
    7575        downcast<WorkerGlobalScope>(context).inspectorController().disconnectFrontend(DisconnectReason::InspectorDestroyed);
    7676
     
    7878        // the pause since this will be the last debugger task we send to the worker.
    7979        downcast<WorkerGlobalScope>(context).thread().stopRunningDebuggerTasks();
    80     }, WorkerRunLoop::debuggerMode());
     80    });
    8181}
    8282
    8383void ServiceWorkerInspectorProxy::sendMessageToWorker(const String& message)
    8484{
    85     m_serviceWorkerThreadProxy.thread().runLoop().postTaskForMode([message = message.isolatedCopy()] (ScriptExecutionContext& context) {
     85    m_serviceWorkerThreadProxy.thread().runLoop().postDebuggerTask([message = message.isolatedCopy()] (ScriptExecutionContext& context) {
    8686        downcast<WorkerGlobalScope>(context).inspectorController().dispatchMessageFromFrontend(message);
    87     }, WorkerRunLoop::debuggerMode());
     87    });
    8888}
    8989
Note: See TracChangeset for help on using the changeset viewer.