From b77996884efd5ce0127b6d357629ca159753c327 Mon Sep 17 00:00:00 2001 From: Dan Klishch Date: Sun, 18 Feb 2024 00:11:38 -0500 Subject: [PATCH] LibCore+Ladybird: Don't force timer ids to be integer just to remap them If we don't force event loop to fit timer id into integer, we can eliminate awkward IDAllocator inside EventLoopImplementations. --- .../Application/EventLoopImplementation.h | 4 +-- .../Application/EventLoopImplementation.mm | 8 ++--- Ladybird/Qt/EventLoopImplementationQt.cpp | 18 ++++------- Ladybird/Qt/EventLoopImplementationQt.h | 4 +-- Userland/Libraries/LibCore/EventLoop.cpp | 4 +-- Userland/Libraries/LibCore/EventLoop.h | 4 +-- .../LibCore/EventLoopImplementation.h | 4 +-- .../LibCore/EventLoopImplementationUnix.cpp | 30 ++++++++----------- .../LibCore/EventLoopImplementationUnix.h | 4 +-- Userland/Libraries/LibCore/EventReceiver.h | 2 +- 10 files changed, 36 insertions(+), 46 deletions(-) diff --git a/Ladybird/AppKit/Application/EventLoopImplementation.h b/Ladybird/AppKit/Application/EventLoopImplementation.h index 6ed0bb5968..ad794742bf 100644 --- a/Ladybird/AppKit/Application/EventLoopImplementation.h +++ b/Ladybird/AppKit/Application/EventLoopImplementation.h @@ -16,8 +16,8 @@ class CFEventLoopManager final : public Core::EventLoopManager { public: virtual NonnullOwnPtr make_implementation() override; - virtual int register_timer(Core::EventReceiver&, int interval_milliseconds, bool should_reload, Core::TimerShouldFireWhenNotVisible) override; - virtual void unregister_timer(int timer_id) override; + virtual intptr_t register_timer(Core::EventReceiver&, int interval_milliseconds, bool should_reload, Core::TimerShouldFireWhenNotVisible) override; + virtual void unregister_timer(intptr_t timer_id) override; virtual void register_notifier(Core::Notifier&) override; virtual void unregister_notifier(Core::Notifier&) override; diff --git a/Ladybird/AppKit/Application/EventLoopImplementation.mm b/Ladybird/AppKit/Application/EventLoopImplementation.mm index 3e99530979..feb61662cf 100644 --- a/Ladybird/AppKit/Application/EventLoopImplementation.mm +++ b/Ladybird/AppKit/Application/EventLoopImplementation.mm @@ -48,7 +48,7 @@ NonnullOwnPtr CFEventLoopManager::make_implementa return CFEventLoopImplementation::create(); } -int CFEventLoopManager::register_timer(Core::EventReceiver& receiver, int interval_milliseconds, bool should_reload, Core::TimerShouldFireWhenNotVisible should_fire_when_not_visible) +intptr_t CFEventLoopManager::register_timer(Core::EventReceiver& receiver, int interval_milliseconds, bool should_reload, Core::TimerShouldFireWhenNotVisible should_fire_when_not_visible) { auto& thread_data = ThreadData::the(); @@ -82,12 +82,12 @@ int CFEventLoopManager::register_timer(Core::EventReceiver& receiver, int interv return timer_id; } -void CFEventLoopManager::unregister_timer(int timer_id) +void CFEventLoopManager::unregister_timer(intptr_t timer_id) { auto& thread_data = ThreadData::the(); - thread_data.timer_id_allocator.deallocate(timer_id); + thread_data.timer_id_allocator.deallocate(static_cast(timer_id)); - auto timer = thread_data.timers.take(timer_id); + auto timer = thread_data.timers.take(static_cast(timer_id)); VERIFY(timer.has_value()); CFRunLoopTimerInvalidate(*timer); CFRelease(*timer); diff --git a/Ladybird/Qt/EventLoopImplementationQt.cpp b/Ladybird/Qt/EventLoopImplementationQt.cpp index 2c9e8dd728..b0f05aa45e 100644 --- a/Ladybird/Qt/EventLoopImplementationQt.cpp +++ b/Ladybird/Qt/EventLoopImplementationQt.cpp @@ -29,8 +29,6 @@ struct ThreadData { return *s_thread_data; } - IDAllocator timer_id_allocator; - HashMap> timers; HashMap> notifiers; }; @@ -90,13 +88,11 @@ static void qt_timer_fired(Core::TimerShouldFireWhenNotVisible should_fire_when_ object.dispatch_event(event); } -int EventLoopManagerQt::register_timer(Core::EventReceiver& object, int milliseconds, bool should_reload, Core::TimerShouldFireWhenNotVisible should_fire_when_not_visible) +intptr_t EventLoopManagerQt::register_timer(Core::EventReceiver& object, int milliseconds, bool should_reload, Core::TimerShouldFireWhenNotVisible should_fire_when_not_visible) { - auto& thread_data = ThreadData::the(); - auto timer = make(); + auto timer = new QTimer; timer->setInterval(milliseconds); timer->setSingleShot(!should_reload); - auto timer_id = thread_data.timer_id_allocator.allocate(); auto weak_object = object.make_weak_ptr(); QObject::connect(timer, &QTimer::timeout, [should_fire_when_not_visible, weak_object = move(weak_object)] { auto object = weak_object.strong_ref(); @@ -105,15 +101,13 @@ int EventLoopManagerQt::register_timer(Core::EventReceiver& object, int millisec qt_timer_fired(should_fire_when_not_visible, *object); }); timer->start(); - thread_data.timers.set(timer_id, move(timer)); - return timer_id; + return bit_cast(timer); } -void EventLoopManagerQt::unregister_timer(int timer_id) +void EventLoopManagerQt::unregister_timer(intptr_t timer_id) { - auto& thread_data = ThreadData::the(); - thread_data.timer_id_allocator.deallocate(timer_id); - VERIFY(thread_data.timers.remove(timer_id)); + auto* timer = bit_cast(timer_id); + delete timer; } static void qt_notifier_activated(Core::Notifier& notifier) diff --git a/Ladybird/Qt/EventLoopImplementationQt.h b/Ladybird/Qt/EventLoopImplementationQt.h index 8711a42eec..f4a689e942 100644 --- a/Ladybird/Qt/EventLoopImplementationQt.h +++ b/Ladybird/Qt/EventLoopImplementationQt.h @@ -26,8 +26,8 @@ public: virtual ~EventLoopManagerQt() override; virtual NonnullOwnPtr make_implementation() override; - virtual int register_timer(Core::EventReceiver&, int milliseconds, bool should_reload, Core::TimerShouldFireWhenNotVisible) override; - virtual void unregister_timer(int timer_id) override; + virtual intptr_t register_timer(Core::EventReceiver&, int milliseconds, bool should_reload, Core::TimerShouldFireWhenNotVisible) override; + virtual void unregister_timer(intptr_t timer_id) override; virtual void register_notifier(Core::Notifier&) override; virtual void unregister_notifier(Core::Notifier&) override; diff --git a/Userland/Libraries/LibCore/EventLoop.cpp b/Userland/Libraries/LibCore/EventLoop.cpp index 73dbfb94f4..bbe7667179 100644 --- a/Userland/Libraries/LibCore/EventLoop.cpp +++ b/Userland/Libraries/LibCore/EventLoop.cpp @@ -125,12 +125,12 @@ void EventLoop::notify_forked(ForkEvent) current().m_impl->notify_forked_and_in_child(); } -int EventLoop::register_timer(EventReceiver& object, int milliseconds, bool should_reload, TimerShouldFireWhenNotVisible fire_when_not_visible) +intptr_t EventLoop::register_timer(EventReceiver& object, int milliseconds, bool should_reload, TimerShouldFireWhenNotVisible fire_when_not_visible) { return EventLoopManager::the().register_timer(object, milliseconds, should_reload, fire_when_not_visible); } -void EventLoop::unregister_timer(int timer_id) +void EventLoop::unregister_timer(intptr_t timer_id) { EventLoopManager::the().unregister_timer(timer_id); } diff --git a/Userland/Libraries/LibCore/EventLoop.h b/Userland/Libraries/LibCore/EventLoop.h index eca5c320d5..bdca156520 100644 --- a/Userland/Libraries/LibCore/EventLoop.h +++ b/Userland/Libraries/LibCore/EventLoop.h @@ -76,8 +76,8 @@ public: bool was_exit_requested() const; // The registration functions act upon the current loop of the current thread. - static int register_timer(EventReceiver&, int milliseconds, bool should_reload, TimerShouldFireWhenNotVisible); - static void unregister_timer(int timer_id); + static intptr_t register_timer(EventReceiver&, int milliseconds, bool should_reload, TimerShouldFireWhenNotVisible); + static void unregister_timer(intptr_t timer_id); static void register_notifier(Badge, Notifier&); static void unregister_notifier(Badge, Notifier&); diff --git a/Userland/Libraries/LibCore/EventLoopImplementation.h b/Userland/Libraries/LibCore/EventLoopImplementation.h index fa36f37f24..f84e6b7429 100644 --- a/Userland/Libraries/LibCore/EventLoopImplementation.h +++ b/Userland/Libraries/LibCore/EventLoopImplementation.h @@ -23,8 +23,8 @@ public: virtual NonnullOwnPtr make_implementation() = 0; - virtual int register_timer(EventReceiver&, int milliseconds, bool should_reload, TimerShouldFireWhenNotVisible) = 0; - virtual void unregister_timer(int timer_id) = 0; + virtual intptr_t register_timer(EventReceiver&, int milliseconds, bool should_reload, TimerShouldFireWhenNotVisible) = 0; + virtual void unregister_timer(intptr_t timer_id) = 0; virtual void register_notifier(Notifier&) = 0; virtual void unregister_notifier(Notifier&) = 0; diff --git a/Userland/Libraries/LibCore/EventLoopImplementationUnix.cpp b/Userland/Libraries/LibCore/EventLoopImplementationUnix.cpp index 98eb38e4de..6f8b565902 100644 --- a/Userland/Libraries/LibCore/EventLoopImplementationUnix.cpp +++ b/Userland/Libraries/LibCore/EventLoopImplementationUnix.cpp @@ -4,7 +4,6 @@ * SPDX-License-Identifier: BSD-2-Clause */ -#include #include #include #include @@ -94,7 +93,7 @@ struct ThreadData { } // Each thread has its own timers, notifiers and a wake pipe. - HashMap> timers; + HashTable timers; Vector poll_fds; HashMap notifier_by_ptr; @@ -105,8 +104,6 @@ struct ThreadData { int wake_pipe_fds[2] { -1, -1 }; pid_t pid { 0 }; - - IDAllocator id_allocator; }; EventLoopImplementationUnix::EventLoopImplementationUnix() @@ -234,7 +231,7 @@ try_select_again: auto now = MonotonicTime::now_coarse(); for (auto& it : thread_data.timers) { - auto& timer = *it.value; + auto& timer = *it; if (!timer.has_expired(now)) continue; auto owner = timer.owner.strong_ref(); @@ -364,9 +361,9 @@ Optional EventLoopManagerUnix::get_next_timer_expiration() auto now = MonotonicTime::now_coarse(); Optional soonest {}; for (auto& it : ThreadData::the().timers) { - auto& fire_time = it.value->fire_time; - auto owner = it.value->owner.strong_ref(); - if (it.value->fire_when_not_visible == TimerShouldFireWhenNotVisible::No + auto& fire_time = it->fire_time; + auto owner = it->owner.strong_ref(); + if (it->fire_when_not_visible == TimerShouldFireWhenNotVisible::No && owner && !owner->is_visible_for_timer_purposes()) { continue; } @@ -493,27 +490,26 @@ void EventLoopManagerUnix::unregister_signal(int handler_id) info.signal_handlers.remove(remove_signal_number); } -int EventLoopManagerUnix::register_timer(EventReceiver& object, int milliseconds, bool should_reload, TimerShouldFireWhenNotVisible fire_when_not_visible) +intptr_t EventLoopManagerUnix::register_timer(EventReceiver& object, int milliseconds, bool should_reload, TimerShouldFireWhenNotVisible fire_when_not_visible) { VERIFY(milliseconds >= 0); auto& thread_data = ThreadData::the(); - auto timer = make(); + auto timer = new EventLoopTimer; timer->owner = object; timer->interval = Duration::from_milliseconds(milliseconds); timer->reload(MonotonicTime::now_coarse()); timer->should_reload = should_reload; timer->fire_when_not_visible = fire_when_not_visible; - int timer_id = thread_data.id_allocator.allocate(); - timer->timer_id = timer_id; - thread_data.timers.set(timer_id, move(timer)); - return timer_id; + thread_data.timers.set(timer); + return bit_cast(timer); } -void EventLoopManagerUnix::unregister_timer(int timer_id) +void EventLoopManagerUnix::unregister_timer(intptr_t timer_id) { auto& thread_data = ThreadData::the(); - thread_data.id_allocator.deallocate(timer_id); - VERIFY(thread_data.timers.remove(timer_id)); + auto* timer = bit_cast(timer_id); + VERIFY(thread_data.timers.remove(timer)); + delete timer; } void EventLoopManagerUnix::register_notifier(Notifier& notifier) diff --git a/Userland/Libraries/LibCore/EventLoopImplementationUnix.h b/Userland/Libraries/LibCore/EventLoopImplementationUnix.h index b19bf11685..d29eb7bef9 100644 --- a/Userland/Libraries/LibCore/EventLoopImplementationUnix.h +++ b/Userland/Libraries/LibCore/EventLoopImplementationUnix.h @@ -17,8 +17,8 @@ public: virtual NonnullOwnPtr make_implementation() override; - virtual int register_timer(EventReceiver&, int milliseconds, bool should_reload, TimerShouldFireWhenNotVisible) override; - virtual void unregister_timer(int timer_id) override; + virtual intptr_t register_timer(EventReceiver&, int milliseconds, bool should_reload, TimerShouldFireWhenNotVisible) override; + virtual void unregister_timer(intptr_t timer_id) override; virtual void register_notifier(Notifier&) override; virtual void unregister_notifier(Notifier&) override; diff --git a/Userland/Libraries/LibCore/EventReceiver.h b/Userland/Libraries/LibCore/EventReceiver.h index f51a788fac..9878925795 100644 --- a/Userland/Libraries/LibCore/EventReceiver.h +++ b/Userland/Libraries/LibCore/EventReceiver.h @@ -169,7 +169,7 @@ protected: private: EventReceiver* m_parent { nullptr }; ByteString m_name; - int m_timer_id { 0 }; + intptr_t m_timer_id { 0 }; Vector> m_children; Function m_event_filter; };