Skip to content

Commit

Permalink
Cap the duration of EventLoop::run in the main thread
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=260112

Reviewed by Yusuke Suzuki.

Cap the duration of EventLoop::run in the main thread using the same time limit
ThreadTimers::sharedTimerFiredInternal() uses.

* LayoutTests/fast/eventloop/queue-task-event-loop-starvation-expected.txt: Added.
* LayoutTests/fast/eventloop/queue-task-event-loop-starvation.html: Added.
* Source/WebCore/dom/EventLoop.cpp:
(WebCore::EventLoop::run):
* Source/WebCore/dom/EventLoop.h:
* Source/WebCore/dom/WindowEventLoop.cpp:
(WebCore::WindowEventLoop::didReachTimeToRun):
* Source/WebCore/platform/ThreadTimers.cpp:
* Source/WebCore/platform/ThreadTimers.h:

Canonical link: https://commits.webkit.org/266896@main
  • Loading branch information
rniwa committed Aug 15, 2023
1 parent f8811c9 commit 9c58823
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
This tests repeated queueing of event loop tasks. Event loop should eventually yield to the run loop.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS executedTaskCount < 100 is true
PASS successfullyParsed is true

TEST COMPLETE

45 changes: 45 additions & 0 deletions LayoutTests/fast/eventloop/queue-task-event-loop-starvation.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<!DOCTYPE html>
<html>
<body>
<script src="../../resources/js-test.js"></script>
<script>

description('This tests repeated queueing of event loop tasks. Event loop should eventually yield to the run loop.');

if (!window.internals)
testFailed('This test relies on window.internals');
else {
jsTestIsAsync = true;
logs = [];
}

function slowFunction(duration)
{
const startTime = performance.now();
while (performance.now() < startTime + duration);
}

let executedTaskCount = 0;
function scheduleSlowTask()
{
internals.queueTask("DOMManipulation", () => {
slowFunction(10);
++executedTaskCount;
});
}

function runTest()
{
setTimeout(() => {
shouldBeTrue('executedTaskCount < 100');
finishJSTest();
}, 100);
for (let i = 0; i < 100; ++i)
scheduleSlowTask();
}

onload = runTest;

</script>
</body>
</html>
9 changes: 7 additions & 2 deletions Source/WebCore/dom/EventLoop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ void EventLoop::scheduleToRunIfNeeded()
scheduleToRun();
}

void EventLoop::run()
void EventLoop::run(std::optional<ApproximateTime> deadline)
{
m_isScheduledToRun = false;
bool didPerformMicrotaskCheckpoint = false;
Expand All @@ -304,12 +304,14 @@ void EventLoop::run()
auto tasks = std::exchange(m_tasks, { });
m_groupsWithSuspendedTasks.clear();
Vector<std::unique_ptr<EventLoopTask>> remainingTasks;
bool hasReachedDeadline = false;
for (auto& task : tasks) {
auto* group = task->group();
if (!group || group->isStoppedPermanently())
continue;

if (group->isSuspended()) {
hasReachedDeadline = hasReachedDeadline || (deadline && ApproximateTime::now() > *deadline);
if (group->isSuspended() || hasReachedDeadline) {
m_groupsWithSuspendedTasks.add(*group);
remainingTasks.append(WTFMove(task));
continue;
Expand All @@ -322,6 +324,9 @@ void EventLoop::run()
for (auto& task : m_tasks)
remainingTasks.append(WTFMove(task));
m_tasks = WTFMove(remainingTasks);

if (!m_tasks.isEmpty() && hasReachedDeadline)
scheduleToRunIfNeeded();
}

// FIXME: Remove this once everything is integrated with the event loop.
Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/dom/EventLoop.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#pragma once

#include "TaskSource.h"
#include <optional>
#include <wtf/ApproximateTime.h>
#include <wtf/Function.h>
#include <wtf/Markable.h>
#include <wtf/MonotonicTime.h>
Expand Down Expand Up @@ -128,7 +130,7 @@ class EventLoop : public RefCounted<EventLoop>, public CanMakeWeakPtr<EventLoop>
protected:
EventLoop();
void scheduleToRunIfNeeded();
void run();
void run(std::optional<ApproximateTime> deadline = std::nullopt);
void clearAllTasks();

// FIXME: Account for fully-activeness of each document.
Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/dom/WindowEventLoop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "Page.h"
#include "SecurityOrigin.h"
#include "ThreadGlobalData.h"
#include "ThreadTimers.h"
#include <wtf/RobinHoodHashMap.h>
#include <wtf/RunLoop.h>

Expand Down Expand Up @@ -179,7 +180,8 @@ MonotonicTime WindowEventLoop::computeIdleDeadline()
void WindowEventLoop::didReachTimeToRun()
{
Ref protectedThis { *this }; // Executing tasks may remove the last reference to this WindowEventLoop.
run();
auto deadline = ApproximateTime::now() + ThreadTimers::maxDurationOfFiringTimers;
run(deadline);

if (hasTasksForFullyActiveDocument() || !microtaskQueue().isEmpty())
return;
Expand Down
3 changes: 0 additions & 3 deletions Source/WebCore/platform/ThreadTimers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@

namespace WebCore {

// Fire timers for this length of time, and then quit to let the run loop process user input events.
static constexpr auto maxDurationOfFiringTimers { 16_ms };

// Timers are created, started and fired on the same thread, and each thread has its own ThreadTimers
// copy to keep the heap and a set of currently firing timers.

Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/platform/ThreadTimers.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ class ThreadTimers {
public:
ThreadTimers();

// Fire timers for this length of time, and then quit to let the run loop process user input events.
static constexpr auto maxDurationOfFiringTimers { 16_ms };

// On a thread different then main, we should set the thread's instance of the SharedTimer.
void setSharedTimer(SharedTimer*);

Expand Down

0 comments on commit 9c58823

Please sign in to comment.