[RFC PATCH v2 04/16] apps: common: event_loop: Use single event source for deferred calls

[Barnabás Pőcze]

Hi


2025. január 22., szerda 10:23 keltezéssel, Jacopo Mondi <jacopo.mondi at 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 at 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
> >
> >
>