[{"id":33134,"web_url":"https://patchwork.libcamera.org/comment/33134/","msgid":"","date":"2025-01-22T09:23:00","subject":"Re: [RFC PATCH v2 04/16] apps: common: event_loop: Use single event\n\tsource for deferred calls","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Tue, Jan 14, 2025 at 06:22:05PM +0000, Barnabás Pőcze wrote:\n> Instead of calling `event_base_once()` every time a deferred call\n> is added to the loop, create an event source at construction, and\n> simply trigger that when a new deferred call is scheduled.\n\nI'm not familiar with libevent, so it's a bit hard for me to grasp why\nthis is better.\n\n>\n> Signed-off-by: Barnabás Pőcze \n> ---\n> src/apps/common/event_loop.cpp | 48 +++++++++++++++++-----------------\n> src/apps/common/event_loop.h | 5 +---\n> 2 files changed, 25 insertions(+), 28 deletions(-)\n>\n> diff --git a/src/apps/common/event_loop.cpp b/src/apps/common/event_loop.cpp\n> index bc8cf17ab..b6230f4ba 100644\n> --- a/src/apps/common/event_loop.cpp\n> +++ b/src/apps/common/event_loop.cpp\n> @@ -21,12 +21,35 @@ EventLoop::EventLoop()\n> \tevthread_use_pthreads();\n> \tbase_ = event_base_new();\n> \tinstance_ = this;\n> +\n> +\tcallsTrigger_ = event_new(base_, -1, EV_PERSIST, [](evutil_socket_t, short, void *closure) {\n> +\t\tauto *self = static_cast(closure);\n> +\n> +\t\tfor (;;) {\n> +\t\t\tstd::function call;\n> +\n> +\t\t\t{\n> +\t\t\t\tstd::lock_guard locker(self->lock_);\n> +\t\t\t\tif (self->calls_.empty())\n> +\t\t\t\t\tbreak;\n> +\n> +\t\t\t\tcall = std::move(self->calls_.front());\n> +\t\t\t\tself->calls_.pop_front();\n> +\t\t\t}\n> +\n> +\t\t\tcall();\n> +\t\t}\n> +\t}, this);\n> +\tassert(callsTrigger_);\n> +\tevent_add(callsTrigger_, nullptr);\n> }\n>\n> EventLoop::~EventLoop()\n> {\n> \tinstance_ = nullptr;\n>\n> +\tevent_free(callsTrigger_);\n> +\n> \tevents_.clear();\n> \tevent_base_free(base_);\n> \tlibevent_global_shutdown();\n> @@ -57,7 +80,7 @@ void EventLoop::callLater(std::function &&func)\n> \t\tcalls_.push_back(std::move(func));\n> \t}\n>\n> -\tevent_base_once(base_, -1, EV_TIMEOUT, dispatchCallback, this, nullptr);\n> +\tevent_active(callsTrigger_, 0, 0);\n> }\n>\n> void EventLoop::addFdEvent(int fd, EventType type,\n> @@ -108,29 +131,6 @@ void EventLoop::addTimerEvent(const std::chrono::microseconds period,\n> \tevents_.push_back(std::move(event));\n> }\n>\n> -void EventLoop::dispatchCallback([[maybe_unused]] evutil_socket_t fd,\n> -\t\t\t\t [[maybe_unused]] short flags, void *param)\n> -{\n> -\tEventLoop *loop = static_cast(param);\n> -\tloop->dispatchCall();\n> -}\n> -\n> -void EventLoop::dispatchCall()\n> -{\n> -\tstd::function call;\n> -\n> -\t{\n> -\t\tstd::unique_lock locker(lock_);\n> -\t\tif (calls_.empty())\n> -\t\t\treturn;\n> -\n> -\t\tcall = calls_.front();\n> -\t\tcalls_.pop_front();\n> -\t}\n> -\n> -\tcall();\n> -}\n> -\n> EventLoop::Event::Event(std::function &&callback)\n> \t: callback_(std::move(callback)), event_(nullptr)\n> {\n> diff --git a/src/apps/common/event_loop.h b/src/apps/common/event_loop.h\n> index 760075885..aac5ed3cc 100644\n> --- a/src/apps/common/event_loop.h\n> +++ b/src/apps/common/event_loop.h\n> @@ -65,11 +65,8 @@ private:\n> \tint exitCode_;\n>\n> \tstd::deque> calls_;\n> +\t::event *callsTrigger_ = nullptr;\n>\n> \tstd::list> events_;\n> \tstd::mutex lock_;\n> -\n> -\tstatic void dispatchCallback(evutil_socket_t fd, short flags,\n> -\t\t\t\t void *param);\n> -\tvoid dispatchCall();\n> };\n> --\n> 2.48.0\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D31DFC3200\n\tfor ;\n\tWed, 22 Jan 2025 09:23:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 782D768558;\n\tWed, 22 Jan 2025 10:23:06 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 85A686187C\n\tfor ;\n\tWed, 22 Jan 2025 10:23:04 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9D777520;\n\tWed, 22 Jan 2025 10:22:01 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"WnRmpLx3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737537721;\n\tbh=BFaIEfBQ591ebHMzyScnyVSiwPBbf7QOzGUG8X7eltg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WnRmpLx38+fO6g5lk2iE/M5imEiMWH8OA8btN5Fa7SgYP76SgZN3/kzS62tbKM06E\n\tO0PvQ1AQdfTAZwtLWRkE2wcAaHIY1YyWdgj3h257HRI5+XdMzQLvPzSigt0xhHiVvO\n\tUjofbpwHdY5gW2jOrFM0JcPVtuoyjauFvAQIQb1Y=","Date":"Wed, 22 Jan 2025 10:23:00 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 04/16] apps: common: event_loop: Use single event\n\tsource for deferred calls","Message-ID":"","References":"<20250114182143.1773762-1-pobrn@protonmail.com>\n\t<20250114182143.1773762-5-pobrn@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250114182143.1773762-5-pobrn@protonmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33148,"web_url":"https://patchwork.libcamera.org/comment/33148/","msgid":"","date":"2025-01-24T09:23:39","subject":"Re: [RFC PATCH v2 04/16] apps: common: event_loop: Use single event\n\tsource for deferred calls","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2025. január 22., szerda 10:23 keltezéssel, Jacopo Mondi írta:\n\n> Hi Barnabás\n> \n> On Tue, Jan 14, 2025 at 06:22:05PM +0000, Barnabás Pőcze wrote:\n> > Instead of calling `event_base_once()` every time a deferred call\n> > is added to the loop, create an event source at construction, and\n> > simply trigger that when a new deferred call is scheduled.\n> \n> I'm not familiar with libevent, so it's a bit hard for me to grasp why\n> this is better.\n\nThe motivation is to avoid the constant allocations and related bookkeeping\ndue to `event_base_once()` since starting from a later change all request\ncompletions go through the `EventLoop::callLater()` mechanism.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> >\n> > Signed-off-by: Barnabás Pőcze \n> > ---\n> > src/apps/common/event_loop.cpp | 48 +++++++++++++++++-----------------\n> > src/apps/common/event_loop.h | 5 +---\n> > 2 files changed, 25 insertions(+), 28 deletions(-)\n> >\n> > diff --git a/src/apps/common/event_loop.cpp b/src/apps/common/event_loop.cpp\n> > index bc8cf17ab..b6230f4ba 100644\n> > --- a/src/apps/common/event_loop.cpp\n> > +++ b/src/apps/common/event_loop.cpp\n> > @@ -21,12 +21,35 @@ EventLoop::EventLoop()\n> > \tevthread_use_pthreads();\n> > \tbase_ = event_base_new();\n> > \tinstance_ = this;\n> > +\n> > +\tcallsTrigger_ = event_new(base_, -1, EV_PERSIST, [](evutil_socket_t, short, void *closure) {\n> > +\t\tauto *self = static_cast(closure);\n> > +\n> > +\t\tfor (;;) {\n> > +\t\t\tstd::function call;\n> > +\n> > +\t\t\t{\n> > +\t\t\t\tstd::lock_guard locker(self->lock_);\n> > +\t\t\t\tif (self->calls_.empty())\n> > +\t\t\t\t\tbreak;\n> > +\n> > +\t\t\t\tcall = std::move(self->calls_.front());\n> > +\t\t\t\tself->calls_.pop_front();\n> > +\t\t\t}\n> > +\n> > +\t\t\tcall();\n> > +\t\t}\n> > +\t}, this);\n> > +\tassert(callsTrigger_);\n> > +\tevent_add(callsTrigger_, nullptr);\n> > }\n> >\n> > EventLoop::~EventLoop()\n> > {\n> > \tinstance_ = nullptr;\n> >\n> > +\tevent_free(callsTrigger_);\n> > +\n> > \tevents_.clear();\n> > \tevent_base_free(base_);\n> > \tlibevent_global_shutdown();\n> > @@ -57,7 +80,7 @@ void EventLoop::callLater(std::function &&func)\n> > \t\tcalls_.push_back(std::move(func));\n> > \t}\n> >\n> > -\tevent_base_once(base_, -1, EV_TIMEOUT, dispatchCallback, this, nullptr);\n> > +\tevent_active(callsTrigger_, 0, 0);\n> > }\n> >\n> > void EventLoop::addFdEvent(int fd, EventType type,\n> > @@ -108,29 +131,6 @@ void EventLoop::addTimerEvent(const std::chrono::microseconds period,\n> > \tevents_.push_back(std::move(event));\n> > }\n> >\n> > -void EventLoop::dispatchCallback([[maybe_unused]] evutil_socket_t fd,\n> > -\t\t\t\t [[maybe_unused]] short flags, void *param)\n> > -{\n> > -\tEventLoop *loop = static_cast(param);\n> > -\tloop->dispatchCall();\n> > -}\n> > -\n> > -void EventLoop::dispatchCall()\n> > -{\n> > -\tstd::function call;\n> > -\n> > -\t{\n> > -\t\tstd::unique_lock locker(lock_);\n> > -\t\tif (calls_.empty())\n> > -\t\t\treturn;\n> > -\n> > -\t\tcall = calls_.front();\n> > -\t\tcalls_.pop_front();\n> > -\t}\n> > -\n> > -\tcall();\n> > -}\n> > -\n> > EventLoop::Event::Event(std::function &&callback)\n> > \t: callback_(std::move(callback)), event_(nullptr)\n> > {\n> > diff --git a/src/apps/common/event_loop.h b/src/apps/common/event_loop.h\n> > index 760075885..aac5ed3cc 100644\n> > --- a/src/apps/common/event_loop.h\n> > +++ b/src/apps/common/event_loop.h\n> > @@ -65,11 +65,8 @@ private:\n> > \tint exitCode_;\n> >\n> > \tstd::deque> calls_;\n> > +\t::event *callsTrigger_ = nullptr;\n> >\n> > \tstd::list> events_;\n> > \tstd::mutex lock_;\n> > -\n> > -\tstatic void dispatchCallback(evutil_socket_t fd, short flags,\n> > -\t\t\t\t void *param);\n> > -\tvoid dispatchCall();\n> > };\n> > --\n> > 2.48.0\n> >\n> >\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7B003C3220\n\tfor ;\n\tFri, 24 Jan 2025 09:23:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 716656855C;\n\tFri, 24 Jan 2025 10:23:45 +0100 (CET)","from mail-4322.protonmail.ch (mail-4322.protonmail.ch\n\t[185.70.43.22])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4A52868506\n\tfor ;\n\tFri, 24 Jan 2025 10:23:44 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"g7eOdPGg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1737710623; x=1737969823;\n\tbh=0Nj5YOFPD8XyzRFh7BwD9XaY57of+wUjCByD8TdWyQ0=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post;\n\tb=g7eOdPGg1pP1xCIzL1pN+z6lZ9Y+mM5wc+wmvb6NzWJYdqo02AO1uwbqsn56AXBkN\n\tYx3qVF5wFzFKxQvhGB1s6Lx5NlraJbpC6dHZ3zYCHDU72cpmLdrCodqc3T1BwTpjCM\n\tbEyPOLJ1ZIB7ntUgbTO6wK0n1dECt3+3aUl+bYJk4c5lGy13VBA/iaSj30N8gDCw7A\n\t8iigdGOKBHWenkKKOmopwk09e0j4ikhJbSHzTtmULxL+HRgsgjOlJqk3FKIVeZNnJi\n\tW/+sz1AndTd7ZYgmbhAtR4Du1ohRlQhufzf3LWqg+KXJ7BgN7MTZHxLMtd9IEVgyfX\n\tp+vfMdKRAFEjg==","Date":"Fri, 24 Jan 2025 09:23:39 +0000","To":"Jacopo Mondi ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 04/16] apps: common: event_loop: Use single event\n\tsource for deferred calls","Message-ID":"","In-Reply-To":"","References":"<20250114182143.1773762-1-pobrn@protonmail.com>\n\t<20250114182143.1773762-5-pobrn@protonmail.com>\n\t","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"6259ec8112f92a6ba7b7a8e7cf6aeea3baaa97d6","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]