From 4d27e9aa5e4e44b306b6a492d8cffda912f6381d Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 1 Dec 2025 15:23:27 +0100 Subject: [PATCH] LibWeb: Make setInterval() reuse the timer to reduce drift Instead of creating a new single-shot timer every time a setInterval timer reschedules itself, we now use a repeating Core::Timer internally. This dramatically reduces timer drift, since the next timeout is now based on when the timer fired, rather than when the timer callback completed (which could take arbitrarily long since we run JS). It's not perfect, but it's a huge improvement and allows us to play DiabloWeb at full framerate (20 fps). --- Libraries/LibWeb/HTML/Timer.cpp | 22 +++++++++++-------- Libraries/LibWeb/HTML/Timer.h | 12 +++++++--- .../LibWeb/HTML/WindowOrWorkerGlobalScope.cpp | 22 ++++++++++++++----- .../LibWeb/HTML/WindowOrWorkerGlobalScope.h | 2 +- 4 files changed, 39 insertions(+), 19 deletions(-) diff --git a/Libraries/LibWeb/HTML/Timer.cpp b/Libraries/LibWeb/HTML/Timer.cpp index 7023a00b81d..76b9adf6af0 100644 --- a/Libraries/LibWeb/HTML/Timer.cpp +++ b/Libraries/LibWeb/HTML/Timer.cpp @@ -13,27 +13,26 @@ namespace Web::HTML { GC_DEFINE_ALLOCATOR(Timer); -GC::Ref Timer::create(JS::Object& window_or_worker_global_scope, i32 milliseconds, Function callback, i32 id) +GC::Ref Timer::create(JS::Object& window_or_worker_global_scope, i32 milliseconds, Function callback, i32 id, Repeating repeating) { - auto heap_function_callback = GC::create_function(window_or_worker_global_scope.heap(), move(callback)); - return window_or_worker_global_scope.heap().allocate(window_or_worker_global_scope, milliseconds, heap_function_callback, id); + return window_or_worker_global_scope.heap().allocate(window_or_worker_global_scope, milliseconds, move(callback), id, repeating); } -Timer::Timer(JS::Object& window_or_worker_global_scope, i32 milliseconds, GC::Ref> callback, i32 id) +Timer::Timer(JS::Object& window_or_worker_global_scope, i32 milliseconds, Function callback, i32 id, Repeating repeating) : m_window_or_worker_global_scope(window_or_worker_global_scope) - , m_callback(move(callback)) , m_id(id) { - m_timer = Core::Timer::create_single_shot(milliseconds, [this] { - m_callback->function()(); - }); + if (repeating == Repeating::Yes) + m_timer = Core::Timer::create_repeating(milliseconds, move(callback)); + else + m_timer = Core::Timer::create_single_shot(milliseconds, move(callback)); } void Timer::visit_edges(Cell::Visitor& visitor) { Base::visit_edges(visitor); visitor.visit(m_window_or_worker_global_scope); - visitor.visit(m_callback); + visitor.visit_possible_values(m_timer->on_timeout.raw_capture_range()); } Timer::~Timer() @@ -51,4 +50,9 @@ void Timer::stop() m_timer->stop(); } +void Timer::set_callback(Function callback) +{ + m_timer->on_timeout = move(callback); +} + } diff --git a/Libraries/LibWeb/HTML/Timer.h b/Libraries/LibWeb/HTML/Timer.h index c87aac0f680..97636482fb1 100644 --- a/Libraries/LibWeb/HTML/Timer.h +++ b/Libraries/LibWeb/HTML/Timer.h @@ -22,20 +22,26 @@ class Timer final : public JS::Cell { GC_DECLARE_ALLOCATOR(Timer); public: - static GC::Ref create(JS::Object&, i32 milliseconds, Function callback, i32 id); + enum class Repeating { + No, + Yes, + }; + + static GC::Ref create(JS::Object&, i32 milliseconds, Function callback, i32 id, Repeating); virtual ~Timer() override; void start(); void stop(); + void set_callback(Function); + private: - Timer(JS::Object& window, i32 milliseconds, GC::Ref> callback, i32 id); + Timer(JS::Object& window, i32 milliseconds, Function callback, i32 id, Repeating); virtual void visit_edges(Cell::Visitor&) override; RefPtr m_timer; GC::Ref m_window_or_worker_global_scope; - GC::Ref> m_callback; i32 m_id { 0 }; }; diff --git a/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp b/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp index 458a72ef219..8e48d2e4deb 100644 --- a/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp +++ b/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -672,7 +673,7 @@ i32 WindowOrWorkerGlobalScopeMixin::run_timer_initialization_steps(TimerHandler // 13. Set uniqueHandle to the result of running steps after a timeout given global, "setTimeout/setInterval", // timeout, and completionStep. // FIXME: run_steps_after_a_timeout() needs to be updated to return a unique internal value that can be used here. - run_steps_after_a_timeout_impl(timeout, move(completion_step), id); + run_steps_after_a_timeout_impl(timeout, move(completion_step), id, repeat); // FIXME: 14. Set global's map of setTimeout and setInterval IDs[id] to uniqueHandle. @@ -1074,20 +1075,29 @@ WindowOrWorkerGlobalScopeMixin::AffectedAnyWebSockets WindowOrWorkerGlobalScopeM // https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#run-steps-after-a-timeout void WindowOrWorkerGlobalScopeMixin::run_steps_after_a_timeout(i32 timeout, Function completion_step) { - return run_steps_after_a_timeout_impl(timeout, move(completion_step)); + return run_steps_after_a_timeout_impl(timeout, move(completion_step), {}, Repeat::No); } -void WindowOrWorkerGlobalScopeMixin::run_steps_after_a_timeout_impl(i32 timeout, Function completion_step, Optional timer_key) +void WindowOrWorkerGlobalScopeMixin::run_steps_after_a_timeout_impl(i32 timeout, Function completion_step, Optional timer_key, Repeat repeat) { // 1. Assert: if timerKey is given, then the caller of this algorithm is the timer initialization steps. (Other specifications must not pass timerKey.) // Note: This is enforced by the caller. - // 2. If timerKey is not given, then set it to a new unique non-numeric value. - if (!timer_key.has_value()) + // NB: We deviate from the spec here slightly by reusing existing timers if a timer_key is provided. + GC::Ptr existing_timer; + if (timer_key.has_value()) { + auto result = m_timers.get(timer_key.value()); + if (result.has_value()) { + existing_timer = result.value().ptr(); + existing_timer->set_callback(move(completion_step)); + } + } else { + // 2. If timerKey is not given, then set it to a new unique non-numeric value. timer_key = m_timer_id_allocator.allocate(); + } // FIXME: 3. Let startTime be the current high resolution time given global. - auto timer = Timer::create(this_impl(), timeout, move(completion_step), timer_key.value()); + auto timer = existing_timer ? GC::Ref { *existing_timer } : Timer::create(this_impl(), timeout, move(completion_step), timer_key.value(), repeat == Repeat::Yes ? Timer::Repeating::Yes : Timer::Repeating::No); // FIXME: 4. Set global's map of active timers[timerKey] to startTime plus milliseconds. m_timers.set(timer_key.value(), timer); diff --git a/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h b/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h index 874031136c4..c858fd7080a 100644 --- a/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h +++ b/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h @@ -117,7 +117,7 @@ private: No, }; i32 run_timer_initialization_steps(TimerHandler handler, i32 timeout, GC::RootVector arguments, Repeat repeat, Optional previous_id = {}); - void run_steps_after_a_timeout_impl(i32 timeout, Function completion_step, Optional timer_key = {}); + void run_steps_after_a_timeout_impl(i32 timeout, Function completion_step, Optional timer_key, Repeat); GC::Ref create_image_bitmap_impl(ImageBitmapSource& image, Optional sx, Optional sy, Optional sw, Optional sh, Optional& options) const;