Message ID | 20250114182143.1773762-5-pobrn@protonmail.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Barnabás On Tue, Jan 14, 2025 at 06:22:05PM +0000, Barnabás Pőcze wrote: > Instead of calling `event_base_once()` every time a deferred call > is added to the loop, create an event source at construction, and > simply trigger that when a new deferred call is scheduled. I'm not familiar with libevent, so it's a bit hard for me to grasp why this is better. > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > --- > src/apps/common/event_loop.cpp | 48 +++++++++++++++++----------------- > src/apps/common/event_loop.h | 5 +--- > 2 files changed, 25 insertions(+), 28 deletions(-) > > diff --git a/src/apps/common/event_loop.cpp b/src/apps/common/event_loop.cpp > index bc8cf17ab..b6230f4ba 100644 > --- a/src/apps/common/event_loop.cpp > +++ b/src/apps/common/event_loop.cpp > @@ -21,12 +21,35 @@ EventLoop::EventLoop() > evthread_use_pthreads(); > base_ = event_base_new(); > instance_ = this; > + > + callsTrigger_ = event_new(base_, -1, EV_PERSIST, [](evutil_socket_t, short, void *closure) { > + auto *self = static_cast<EventLoop *>(closure); > + > + for (;;) { > + std::function<void()> call; > + > + { > + std::lock_guard locker(self->lock_); > + if (self->calls_.empty()) > + break; > + > + call = std::move(self->calls_.front()); > + self->calls_.pop_front(); > + } > + > + call(); > + } > + }, this); > + assert(callsTrigger_); > + event_add(callsTrigger_, nullptr); > } > > EventLoop::~EventLoop() > { > instance_ = nullptr; > > + event_free(callsTrigger_); > + > events_.clear(); > event_base_free(base_); > libevent_global_shutdown(); > @@ -57,7 +80,7 @@ void EventLoop::callLater(std::function<void()> &&func) > calls_.push_back(std::move(func)); > } > > - event_base_once(base_, -1, EV_TIMEOUT, dispatchCallback, this, nullptr); > + event_active(callsTrigger_, 0, 0); > } > > void EventLoop::addFdEvent(int fd, EventType type, > @@ -108,29 +131,6 @@ void EventLoop::addTimerEvent(const std::chrono::microseconds period, > events_.push_back(std::move(event)); > } > > -void EventLoop::dispatchCallback([[maybe_unused]] evutil_socket_t fd, > - [[maybe_unused]] short flags, void *param) > -{ > - EventLoop *loop = static_cast<EventLoop *>(param); > - loop->dispatchCall(); > -} > - > -void EventLoop::dispatchCall() > -{ > - std::function<void()> call; > - > - { > - std::unique_lock<std::mutex> locker(lock_); > - if (calls_.empty()) > - return; > - > - call = calls_.front(); > - calls_.pop_front(); > - } > - > - call(); > -} > - > EventLoop::Event::Event(std::function<void()> &&callback) > : callback_(std::move(callback)), event_(nullptr) > { > diff --git a/src/apps/common/event_loop.h b/src/apps/common/event_loop.h > index 760075885..aac5ed3cc 100644 > --- a/src/apps/common/event_loop.h > +++ b/src/apps/common/event_loop.h > @@ -65,11 +65,8 @@ private: > int exitCode_; > > std::deque<std::function<void()>> calls_; > + ::event *callsTrigger_ = nullptr; > > std::list<std::unique_ptr<Event>> events_; > std::mutex lock_; > - > - static void dispatchCallback(evutil_socket_t fd, short flags, > - void *param); > - void dispatchCall(); > }; > -- > 2.48.0 > >
Hi 2025. január 22., szerda 10:23 keltezéssel, Jacopo Mondi <jacopo.mondi@ideasonboard.com> írta: > Hi Barnabás > > On Tue, Jan 14, 2025 at 06:22:05PM +0000, Barnabás Pőcze wrote: > > Instead of calling `event_base_once()` every time a deferred call > > is added to the loop, create an event source at construction, and > > simply trigger that when a new deferred call is scheduled. > > I'm not familiar with libevent, so it's a bit hard for me to grasp why > this is better. The motivation is to avoid the constant allocations and related bookkeeping due to `event_base_once()` since starting from a later change all request completions go through the `EventLoop::callLater()` mechanism. Regards, Barnabás Pőcze > > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > --- > > src/apps/common/event_loop.cpp | 48 +++++++++++++++++----------------- > > src/apps/common/event_loop.h | 5 +--- > > 2 files changed, 25 insertions(+), 28 deletions(-) > > > > diff --git a/src/apps/common/event_loop.cpp b/src/apps/common/event_loop.cpp > > index bc8cf17ab..b6230f4ba 100644 > > --- a/src/apps/common/event_loop.cpp > > +++ b/src/apps/common/event_loop.cpp > > @@ -21,12 +21,35 @@ EventLoop::EventLoop() > > evthread_use_pthreads(); > > base_ = event_base_new(); > > instance_ = this; > > + > > + callsTrigger_ = event_new(base_, -1, EV_PERSIST, [](evutil_socket_t, short, void *closure) { > > + auto *self = static_cast<EventLoop *>(closure); > > + > > + for (;;) { > > + std::function<void()> call; > > + > > + { > > + std::lock_guard locker(self->lock_); > > + if (self->calls_.empty()) > > + break; > > + > > + call = std::move(self->calls_.front()); > > + self->calls_.pop_front(); > > + } > > + > > + call(); > > + } > > + }, this); > > + assert(callsTrigger_); > > + event_add(callsTrigger_, nullptr); > > } > > > > EventLoop::~EventLoop() > > { > > instance_ = nullptr; > > > > + event_free(callsTrigger_); > > + > > events_.clear(); > > event_base_free(base_); > > libevent_global_shutdown(); > > @@ -57,7 +80,7 @@ void EventLoop::callLater(std::function<void()> &&func) > > calls_.push_back(std::move(func)); > > } > > > > - event_base_once(base_, -1, EV_TIMEOUT, dispatchCallback, this, nullptr); > > + event_active(callsTrigger_, 0, 0); > > } > > > > void EventLoop::addFdEvent(int fd, EventType type, > > @@ -108,29 +131,6 @@ void EventLoop::addTimerEvent(const std::chrono::microseconds period, > > events_.push_back(std::move(event)); > > } > > > > -void EventLoop::dispatchCallback([[maybe_unused]] evutil_socket_t fd, > > - [[maybe_unused]] short flags, void *param) > > -{ > > - EventLoop *loop = static_cast<EventLoop *>(param); > > - loop->dispatchCall(); > > -} > > - > > -void EventLoop::dispatchCall() > > -{ > > - std::function<void()> call; > > - > > - { > > - std::unique_lock<std::mutex> locker(lock_); > > - if (calls_.empty()) > > - return; > > - > > - call = calls_.front(); > > - calls_.pop_front(); > > - } > > - > > - call(); > > -} > > - > > EventLoop::Event::Event(std::function<void()> &&callback) > > : callback_(std::move(callback)), event_(nullptr) > > { > > diff --git a/src/apps/common/event_loop.h b/src/apps/common/event_loop.h > > index 760075885..aac5ed3cc 100644 > > --- a/src/apps/common/event_loop.h > > +++ b/src/apps/common/event_loop.h > > @@ -65,11 +65,8 @@ private: > > int exitCode_; > > > > std::deque<std::function<void()>> calls_; > > + ::event *callsTrigger_ = nullptr; > > > > std::list<std::unique_ptr<Event>> events_; > > std::mutex lock_; > > - > > - static void dispatchCallback(evutil_socket_t fd, short flags, > > - void *param); > > - void dispatchCall(); > > }; > > -- > > 2.48.0 > > > > >
diff --git a/src/apps/common/event_loop.cpp b/src/apps/common/event_loop.cpp index bc8cf17ab..b6230f4ba 100644 --- a/src/apps/common/event_loop.cpp +++ b/src/apps/common/event_loop.cpp @@ -21,12 +21,35 @@ EventLoop::EventLoop() evthread_use_pthreads(); base_ = event_base_new(); instance_ = this; + + callsTrigger_ = event_new(base_, -1, EV_PERSIST, [](evutil_socket_t, short, void *closure) { + auto *self = static_cast<EventLoop *>(closure); + + for (;;) { + std::function<void()> call; + + { + std::lock_guard locker(self->lock_); + if (self->calls_.empty()) + break; + + call = std::move(self->calls_.front()); + self->calls_.pop_front(); + } + + call(); + } + }, this); + assert(callsTrigger_); + event_add(callsTrigger_, nullptr); } EventLoop::~EventLoop() { instance_ = nullptr; + event_free(callsTrigger_); + events_.clear(); event_base_free(base_); libevent_global_shutdown(); @@ -57,7 +80,7 @@ void EventLoop::callLater(std::function<void()> &&func) calls_.push_back(std::move(func)); } - event_base_once(base_, -1, EV_TIMEOUT, dispatchCallback, this, nullptr); + event_active(callsTrigger_, 0, 0); } void EventLoop::addFdEvent(int fd, EventType type, @@ -108,29 +131,6 @@ void EventLoop::addTimerEvent(const std::chrono::microseconds period, events_.push_back(std::move(event)); } -void EventLoop::dispatchCallback([[maybe_unused]] evutil_socket_t fd, - [[maybe_unused]] short flags, void *param) -{ - EventLoop *loop = static_cast<EventLoop *>(param); - loop->dispatchCall(); -} - -void EventLoop::dispatchCall() -{ - std::function<void()> call; - - { - std::unique_lock<std::mutex> locker(lock_); - if (calls_.empty()) - return; - - call = calls_.front(); - calls_.pop_front(); - } - - call(); -} - EventLoop::Event::Event(std::function<void()> &&callback) : callback_(std::move(callback)), event_(nullptr) { diff --git a/src/apps/common/event_loop.h b/src/apps/common/event_loop.h index 760075885..aac5ed3cc 100644 --- a/src/apps/common/event_loop.h +++ b/src/apps/common/event_loop.h @@ -65,11 +65,8 @@ private: int exitCode_; std::deque<std::function<void()>> calls_; + ::event *callsTrigger_ = nullptr; std::list<std::unique_ptr<Event>> events_; std::mutex lock_; - - static void dispatchCallback(evutil_socket_t fd, short flags, - void *param); - void dispatchCall(); };
Instead of calling `event_base_once()` every time a deferred call is added to the loop, create an event source at construction, and simply trigger that when a new deferred call is scheduled. Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> --- src/apps/common/event_loop.cpp | 48 +++++++++++++++++----------------- src/apps/common/event_loop.h | 5 +--- 2 files changed, 25 insertions(+), 28 deletions(-)