From 424e1bcd14837d9cdf264e468a2f622d80d90968 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 7 Feb 2024 10:47:40 +0100 Subject: [PATCH 1/6] Refactor: pass CpuBoundWork& into HttpHandler#HandleRequest() --- lib/remote/actionshandler.cpp | 3 ++- lib/remote/actionshandler.hpp | 3 ++- lib/remote/configfileshandler.cpp | 3 ++- lib/remote/configfileshandler.hpp | 3 ++- lib/remote/configpackageshandler.cpp | 3 ++- lib/remote/configpackageshandler.hpp | 3 ++- lib/remote/configstageshandler.cpp | 3 ++- lib/remote/configstageshandler.hpp | 3 ++- lib/remote/consolehandler.cpp | 3 ++- lib/remote/consolehandler.hpp | 3 ++- lib/remote/createobjecthandler.cpp | 3 ++- lib/remote/createobjecthandler.hpp | 3 ++- lib/remote/deleteobjecthandler.cpp | 3 ++- lib/remote/deleteobjecthandler.hpp | 3 ++- lib/remote/eventshandler.cpp | 3 ++- lib/remote/eventshandler.hpp | 3 ++- lib/remote/httphandler.cpp | 5 +++-- lib/remote/httphandler.hpp | 7 +++++-- lib/remote/httpserverconnection.cpp | 2 +- lib/remote/infohandler.cpp | 3 ++- lib/remote/infohandler.hpp | 3 ++- lib/remote/mallocinfohandler.cpp | 3 ++- lib/remote/mallocinfohandler.hpp | 3 ++- lib/remote/modifyobjecthandler.cpp | 3 ++- lib/remote/modifyobjecthandler.hpp | 3 ++- lib/remote/objectqueryhandler.cpp | 3 ++- lib/remote/objectqueryhandler.hpp | 3 ++- lib/remote/statushandler.cpp | 3 ++- lib/remote/statushandler.hpp | 3 ++- lib/remote/templatequeryhandler.cpp | 3 ++- lib/remote/templatequeryhandler.hpp | 3 ++- lib/remote/typequeryhandler.cpp | 3 ++- lib/remote/typequeryhandler.hpp | 3 ++- lib/remote/variablequeryhandler.cpp | 3 ++- lib/remote/variablequeryhandler.hpp | 3 ++- 35 files changed, 73 insertions(+), 37 deletions(-) diff --git a/lib/remote/actionshandler.cpp b/lib/remote/actionshandler.cpp index 016c76d5719..d97257b0244 100644 --- a/lib/remote/actionshandler.cpp +++ b/lib/remote/actionshandler.cpp @@ -23,7 +23,8 @@ bool ActionsHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/actionshandler.hpp b/lib/remote/actionshandler.hpp index ca662caba2d..83277df0999 100644 --- a/lib/remote/actionshandler.hpp +++ b/lib/remote/actionshandler.hpp @@ -23,7 +23,8 @@ class ActionsHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; }; diff --git a/lib/remote/configfileshandler.cpp b/lib/remote/configfileshandler.cpp index 779ecd1984f..087c4ee7936 100644 --- a/lib/remote/configfileshandler.cpp +++ b/lib/remote/configfileshandler.cpp @@ -21,7 +21,8 @@ bool ConfigFilesHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/configfileshandler.hpp b/lib/remote/configfileshandler.hpp index ea48b1ef429..295d1db6ac0 100644 --- a/lib/remote/configfileshandler.hpp +++ b/lib/remote/configfileshandler.hpp @@ -21,7 +21,8 @@ class ConfigFilesHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; }; diff --git a/lib/remote/configpackageshandler.cpp b/lib/remote/configpackageshandler.cpp index 98b3268909f..19cee60e93f 100644 --- a/lib/remote/configpackageshandler.cpp +++ b/lib/remote/configpackageshandler.cpp @@ -18,7 +18,8 @@ bool ConfigPackagesHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/configpackageshandler.hpp b/lib/remote/configpackageshandler.hpp index 0a05ea10acf..181e6a649bf 100644 --- a/lib/remote/configpackageshandler.hpp +++ b/lib/remote/configpackageshandler.hpp @@ -21,7 +21,8 @@ class ConfigPackagesHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; private: diff --git a/lib/remote/configstageshandler.cpp b/lib/remote/configstageshandler.cpp index edbb767e5b5..8c11de4cdd9 100644 --- a/lib/remote/configstageshandler.cpp +++ b/lib/remote/configstageshandler.cpp @@ -22,7 +22,8 @@ bool ConfigStagesHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/configstageshandler.hpp b/lib/remote/configstageshandler.hpp index 88f248c8fd2..136eea93925 100644 --- a/lib/remote/configstageshandler.hpp +++ b/lib/remote/configstageshandler.hpp @@ -22,7 +22,8 @@ class ConfigStagesHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; private: diff --git a/lib/remote/consolehandler.cpp b/lib/remote/consolehandler.cpp index f5a470a9aca..3f8eba9a41d 100644 --- a/lib/remote/consolehandler.cpp +++ b/lib/remote/consolehandler.cpp @@ -63,7 +63,8 @@ bool ConsoleHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/consolehandler.hpp b/lib/remote/consolehandler.hpp index df0d77d0189..ca686735d00 100644 --- a/lib/remote/consolehandler.hpp +++ b/lib/remote/consolehandler.hpp @@ -30,7 +30,8 @@ class ConsoleHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; static std::vector GetAutocompletionSuggestions(const String& word, ScriptFrame& frame); diff --git a/lib/remote/createobjecthandler.cpp b/lib/remote/createobjecthandler.cpp index 89977a3d36d..59a24732a53 100644 --- a/lib/remote/createobjecthandler.cpp +++ b/lib/remote/createobjecthandler.cpp @@ -23,7 +23,8 @@ bool CreateObjectHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/createobjecthandler.hpp b/lib/remote/createobjecthandler.hpp index 4bcf21b55bc..0a6030116c3 100644 --- a/lib/remote/createobjecthandler.hpp +++ b/lib/remote/createobjecthandler.hpp @@ -21,7 +21,8 @@ class CreateObjectHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; }; diff --git a/lib/remote/deleteobjecthandler.cpp b/lib/remote/deleteobjecthandler.cpp index 6a7d194d4dc..01efc2c4b1a 100644 --- a/lib/remote/deleteobjecthandler.cpp +++ b/lib/remote/deleteobjecthandler.cpp @@ -23,7 +23,8 @@ bool DeleteObjectHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/deleteobjecthandler.hpp b/lib/remote/deleteobjecthandler.hpp index 19a46e475e9..ae05afbe896 100644 --- a/lib/remote/deleteobjecthandler.hpp +++ b/lib/remote/deleteobjecthandler.hpp @@ -21,7 +21,8 @@ class DeleteObjectHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; }; diff --git a/lib/remote/eventshandler.cpp b/lib/remote/eventshandler.cpp index bdda714611d..9c548571a5e 100644 --- a/lib/remote/eventshandler.cpp +++ b/lib/remote/eventshandler.cpp @@ -47,7 +47,8 @@ bool EventsHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace asio = boost::asio; diff --git a/lib/remote/eventshandler.hpp b/lib/remote/eventshandler.hpp index c823415d328..7b1dc53c495 100644 --- a/lib/remote/eventshandler.hpp +++ b/lib/remote/eventshandler.hpp @@ -22,7 +22,8 @@ class EventsHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; }; diff --git a/lib/remote/httphandler.cpp b/lib/remote/httphandler.cpp index 71a89e4ce21..623059dd188 100644 --- a/lib/remote/httphandler.cpp +++ b/lib/remote/httphandler.cpp @@ -52,7 +52,8 @@ void HttpHandler::ProcessRequest( boost::beast::http::request& request, boost::beast::http::response& response, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { Dictionary::Ptr node = m_UrlTree; @@ -108,7 +109,7 @@ void HttpHandler::ProcessRequest( */ try { for (const HttpHandler::Ptr& handler : handlers) { - if (handler->HandleRequest(stream, user, request, url, response, params, yc, server)) { + if (handler->HandleRequest(stream, user, request, url, response, params, yc, server, handlingRequest)) { processed = true; break; } diff --git a/lib/remote/httphandler.hpp b/lib/remote/httphandler.hpp index a6a730255f7..ea84223c3ab 100644 --- a/lib/remote/httphandler.hpp +++ b/lib/remote/httphandler.hpp @@ -7,6 +7,7 @@ #include "remote/url.hpp" #include "remote/httpserverconnection.hpp" #include "remote/apiuser.hpp" +#include "base/io-engine.hpp" #include "base/registry.hpp" #include "base/tlsstream.hpp" #include @@ -34,7 +35,8 @@ class HttpHandler : public Object boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) = 0; static void Register(const Url::Ptr& url, const HttpHandler::Ptr& handler); @@ -44,7 +46,8 @@ class HttpHandler : public Object boost::beast::http::request& request, boost::beast::http::response& response, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ); private: diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index b9a8ab814d6..6d7dae3151f 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -436,7 +436,7 @@ bool ProcessRequest( try { CpuBoundWork handlingRequest (yc); - HttpHandler::ProcessRequest(stream, authenticatedUser, request, response, yc, server); + HttpHandler::ProcessRequest(stream, authenticatedUser, request, response, yc, server, handlingRequest); } catch (const std::exception& ex) { if (hasStartedStreaming) { return false; diff --git a/lib/remote/infohandler.cpp b/lib/remote/infohandler.cpp index 6a1360b40ca..84872fec9f8 100644 --- a/lib/remote/infohandler.cpp +++ b/lib/remote/infohandler.cpp @@ -16,7 +16,8 @@ bool InfoHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/infohandler.hpp b/lib/remote/infohandler.hpp index e1fe983149f..24f49c6bf94 100644 --- a/lib/remote/infohandler.hpp +++ b/lib/remote/infohandler.hpp @@ -21,7 +21,8 @@ class InfoHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; }; diff --git a/lib/remote/mallocinfohandler.cpp b/lib/remote/mallocinfohandler.cpp index ac73e36509e..40812b535ee 100644 --- a/lib/remote/mallocinfohandler.cpp +++ b/lib/remote/mallocinfohandler.cpp @@ -25,7 +25,8 @@ bool MallocInfoHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context&, - HttpServerConnection& + HttpServerConnection&, + CpuBoundWork& ) { namespace http = boost::beast::http; diff --git a/lib/remote/mallocinfohandler.hpp b/lib/remote/mallocinfohandler.hpp index 0e188f3eb2f..e9ed043cc09 100644 --- a/lib/remote/mallocinfohandler.hpp +++ b/lib/remote/mallocinfohandler.hpp @@ -20,7 +20,8 @@ class MallocInfoHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; }; diff --git a/lib/remote/modifyobjecthandler.cpp b/lib/remote/modifyobjecthandler.cpp index a817faad814..549229ebf98 100644 --- a/lib/remote/modifyobjecthandler.cpp +++ b/lib/remote/modifyobjecthandler.cpp @@ -21,7 +21,8 @@ bool ModifyObjectHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/modifyobjecthandler.hpp b/lib/remote/modifyobjecthandler.hpp index f4693013f30..a158f1c0645 100644 --- a/lib/remote/modifyobjecthandler.hpp +++ b/lib/remote/modifyobjecthandler.hpp @@ -21,7 +21,8 @@ class ModifyObjectHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; }; diff --git a/lib/remote/objectqueryhandler.cpp b/lib/remote/objectqueryhandler.cpp index ad73030dafb..a262146cad0 100644 --- a/lib/remote/objectqueryhandler.cpp +++ b/lib/remote/objectqueryhandler.cpp @@ -96,7 +96,8 @@ bool ObjectQueryHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/objectqueryhandler.hpp b/lib/remote/objectqueryhandler.hpp index 691b2cfcf21..8ecabe6d885 100644 --- a/lib/remote/objectqueryhandler.hpp +++ b/lib/remote/objectqueryhandler.hpp @@ -21,7 +21,8 @@ class ObjectQueryHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; private: diff --git a/lib/remote/statushandler.cpp b/lib/remote/statushandler.cpp index 310ce0d8763..c9d39ddbf49 100644 --- a/lib/remote/statushandler.cpp +++ b/lib/remote/statushandler.cpp @@ -76,7 +76,8 @@ bool StatusHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/statushandler.hpp b/lib/remote/statushandler.hpp index c722ab3e2ee..a380d955733 100644 --- a/lib/remote/statushandler.hpp +++ b/lib/remote/statushandler.hpp @@ -21,7 +21,8 @@ class StatusHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; }; diff --git a/lib/remote/templatequeryhandler.cpp b/lib/remote/templatequeryhandler.cpp index e70dafb6501..4d4f8891c47 100644 --- a/lib/remote/templatequeryhandler.cpp +++ b/lib/remote/templatequeryhandler.cpp @@ -83,7 +83,8 @@ bool TemplateQueryHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/templatequeryhandler.hpp b/lib/remote/templatequeryhandler.hpp index 503bc856060..a894b253efd 100644 --- a/lib/remote/templatequeryhandler.hpp +++ b/lib/remote/templatequeryhandler.hpp @@ -21,7 +21,8 @@ class TemplateQueryHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; }; diff --git a/lib/remote/typequeryhandler.cpp b/lib/remote/typequeryhandler.cpp index 4e82653986c..c78e80327a9 100644 --- a/lib/remote/typequeryhandler.cpp +++ b/lib/remote/typequeryhandler.cpp @@ -54,7 +54,8 @@ bool TypeQueryHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/typequeryhandler.hpp b/lib/remote/typequeryhandler.hpp index 5489cb232d2..85d5cb2bb1d 100644 --- a/lib/remote/typequeryhandler.hpp +++ b/lib/remote/typequeryhandler.hpp @@ -21,7 +21,8 @@ class TypeQueryHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; }; diff --git a/lib/remote/variablequeryhandler.cpp b/lib/remote/variablequeryhandler.cpp index f17a9f5f2ee..d516aecd565 100644 --- a/lib/remote/variablequeryhandler.cpp +++ b/lib/remote/variablequeryhandler.cpp @@ -64,7 +64,8 @@ bool VariableQueryHandler::HandleRequest( boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) { namespace http = boost::beast::http; diff --git a/lib/remote/variablequeryhandler.hpp b/lib/remote/variablequeryhandler.hpp index 48e73be356d..6a354fa4df2 100644 --- a/lib/remote/variablequeryhandler.hpp +++ b/lib/remote/variablequeryhandler.hpp @@ -21,7 +21,8 @@ class VariableQueryHandler final : public HttpHandler boost::beast::http::response& response, const Dictionary::Ptr& params, boost::asio::yield_context& yc, - HttpServerConnection& server + HttpServerConnection& server, + CpuBoundWork& handlingRequest ) override; }; From 4374c3618897d4d09b0a9d3860e91b076442ea1e Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 7 Feb 2024 10:53:03 +0100 Subject: [PATCH 2/6] /v1/events: just release CpuBoundWork instead of using IoBoundWorkSlot IoBoundWorkSlot#~IoBoundWorkSlot() will wait for a free semaphore slot which will be almost immediately released by CpuBoundWork#~CpuBoundWork(). Just releasing the already aquired slot via CpuBoundWork#Done() is more efficient. --- lib/remote/eventshandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/remote/eventshandler.cpp b/lib/remote/eventshandler.cpp index 9c548571a5e..4cbcbbd6209 100644 --- a/lib/remote/eventshandler.cpp +++ b/lib/remote/eventshandler.cpp @@ -106,7 +106,7 @@ bool EventsHandler::HandleRequest( response.result(http::status::ok); response.set(http::field::content_type, "application/json"); - IoBoundWorkSlot dontLockTheIoThread (yc); + handlingRequest.Done(); http::async_write(stream, response, yc); stream.async_flush(yc); From 46378f9e256ff351c9a35885e3df4fd1e690dec2 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 7 Feb 2024 10:59:49 +0100 Subject: [PATCH 3/6] Remove unused IoBoundWorkSlot --- lib/base/io-engine.cpp | 23 ----------------------- lib/base/io-engine.hpp | 20 -------------------- 2 files changed, 43 deletions(-) diff --git a/lib/base/io-engine.cpp b/lib/base/io-engine.cpp index 26125feb310..041e72d0f67 100644 --- a/lib/base/io-engine.cpp +++ b/lib/base/io-engine.cpp @@ -50,29 +50,6 @@ void CpuBoundWork::Done() } } -IoBoundWorkSlot::IoBoundWorkSlot(boost::asio::yield_context yc) - : yc(yc) -{ - IoEngine::Get().m_CpuBoundSemaphore.fetch_add(1); -} - -IoBoundWorkSlot::~IoBoundWorkSlot() -{ - auto& ioEngine (IoEngine::Get()); - - for (;;) { - auto availableSlots (ioEngine.m_CpuBoundSemaphore.fetch_sub(1)); - - if (availableSlots < 1) { - ioEngine.m_CpuBoundSemaphore.fetch_add(1); - IoEngine::YieldCurrentCoroutine(yc); - continue; - } - - break; - } -} - LazyInit> IoEngine::m_Instance ([]() { return std::unique_ptr(new IoEngine()); }); IoEngine& IoEngine::Get() diff --git a/lib/base/io-engine.hpp b/lib/base/io-engine.hpp index 326f04fdc47..75360489204 100644 --- a/lib/base/io-engine.hpp +++ b/lib/base/io-engine.hpp @@ -43,25 +43,6 @@ class CpuBoundWork bool m_Done; }; -/** - * Scope break for CPU-bound work done in an I/O thread - * - * @ingroup base - */ -class IoBoundWorkSlot -{ -public: - IoBoundWorkSlot(boost::asio::yield_context yc); - IoBoundWorkSlot(const IoBoundWorkSlot&) = delete; - IoBoundWorkSlot(IoBoundWorkSlot&&) = delete; - IoBoundWorkSlot& operator=(const IoBoundWorkSlot&) = delete; - IoBoundWorkSlot& operator=(IoBoundWorkSlot&&) = delete; - ~IoBoundWorkSlot(); - -private: - boost::asio::yield_context yc; -}; - /** * Async I/O engine * @@ -70,7 +51,6 @@ class IoBoundWorkSlot class IoEngine { friend CpuBoundWork; - friend IoBoundWorkSlot; public: IoEngine(const IoEngine&) = delete; From 6d5b44713f4fe9914686c4d638d7f97be297c7be Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 7 Feb 2024 11:24:10 +0100 Subject: [PATCH 4/6] CpuBoundWork#~CpuBoundWork(): just call #Done() to deduplicate code --- lib/base/io-engine.cpp | 7 ------- lib/base/io-engine.hpp | 6 +++++- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/base/io-engine.cpp b/lib/base/io-engine.cpp index 041e72d0f67..fd624972ff3 100644 --- a/lib/base/io-engine.cpp +++ b/lib/base/io-engine.cpp @@ -34,13 +34,6 @@ CpuBoundWork::CpuBoundWork(boost::asio::yield_context yc) } } -CpuBoundWork::~CpuBoundWork() -{ - if (!m_Done) { - IoEngine::Get().m_CpuBoundSemaphore.fetch_add(1); - } -} - void CpuBoundWork::Done() { if (!m_Done) { diff --git a/lib/base/io-engine.hpp b/lib/base/io-engine.hpp index 75360489204..e342743105a 100644 --- a/lib/base/io-engine.hpp +++ b/lib/base/io-engine.hpp @@ -35,7 +35,11 @@ class CpuBoundWork CpuBoundWork(CpuBoundWork&&) = delete; CpuBoundWork& operator=(const CpuBoundWork&) = delete; CpuBoundWork& operator=(CpuBoundWork&&) = delete; - ~CpuBoundWork(); + + inline ~CpuBoundWork() + { + Done(); + } void Done(); From b41328739fdb3fd4e8cb0bdaf64c7ae598eadf3b Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 20 Feb 2024 12:41:41 +0100 Subject: [PATCH 5/6] [Refactor] CpuBoundWork#CpuBoundWork(): require an io_context::strand --- lib/base/io-engine.cpp | 2 +- lib/base/io-engine.hpp | 3 ++- lib/remote/httpserverconnection.cpp | 12 +++++++----- lib/remote/jsonrpcconnection.cpp | 2 +- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/base/io-engine.cpp b/lib/base/io-engine.cpp index fd624972ff3..871981a82ad 100644 --- a/lib/base/io-engine.cpp +++ b/lib/base/io-engine.cpp @@ -16,7 +16,7 @@ using namespace icinga; -CpuBoundWork::CpuBoundWork(boost::asio::yield_context yc) +CpuBoundWork::CpuBoundWork(boost::asio::yield_context yc, boost::asio::io_context::strand&) : m_Done(false) { auto& ioEngine (IoEngine::Get()); diff --git a/lib/base/io-engine.hpp b/lib/base/io-engine.hpp index e342743105a..622a92dd00c 100644 --- a/lib/base/io-engine.hpp +++ b/lib/base/io-engine.hpp @@ -17,6 +17,7 @@ #include #include #include +#include #include namespace icinga @@ -30,7 +31,7 @@ namespace icinga class CpuBoundWork { public: - CpuBoundWork(boost::asio::yield_context yc); + CpuBoundWork(boost::asio::yield_context yc, boost::asio::io_context::strand&); CpuBoundWork(const CpuBoundWork&) = delete; CpuBoundWork(CpuBoundWork&&) = delete; CpuBoundWork& operator=(const CpuBoundWork&) = delete; diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index 6d7dae3151f..2c97ae8e583 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -338,7 +338,8 @@ bool EnsureValidBody( ApiUser::Ptr& authenticatedUser, boost::beast::http::response& response, bool& shuttingDown, - boost::asio::yield_context& yc + boost::asio::yield_context& yc, + boost::asio::io_context::strand& strand ) { namespace http = boost::beast::http; @@ -428,13 +429,14 @@ bool ProcessRequest( boost::beast::http::response& response, HttpServerConnection& server, bool& hasStartedStreaming, - boost::asio::yield_context& yc + boost::asio::yield_context& yc, + boost::asio::io_context::strand& strand ) { namespace http = boost::beast::http; try { - CpuBoundWork handlingRequest (yc); + CpuBoundWork handlingRequest (yc, strand); HttpHandler::ProcessRequest(stream, authenticatedUser, request, response, yc, server, handlingRequest); } catch (const std::exception& ex) { @@ -542,13 +544,13 @@ void HttpServerConnection::ProcessMessages(boost::asio::yield_context yc) break; } - if (!EnsureValidBody(*m_Stream, buf, parser, authenticatedUser, response, m_ShuttingDown, yc)) { + if (!EnsureValidBody(*m_Stream, buf, parser, authenticatedUser, response, m_ShuttingDown, yc, m_IoStrand)) { break; } m_Seen = std::numeric_limits::max(); - if (!ProcessRequest(*m_Stream, request, authenticatedUser, response, *this, m_HasStartedStreaming, yc)) { + if (!ProcessRequest(*m_Stream, request, authenticatedUser, response, *this, m_HasStartedStreaming, yc, m_IoStrand)) { break; } diff --git a/lib/remote/jsonrpcconnection.cpp b/lib/remote/jsonrpcconnection.cpp index 44f1662dde1..a33a0ef4a2c 100644 --- a/lib/remote/jsonrpcconnection.cpp +++ b/lib/remote/jsonrpcconnection.cpp @@ -78,7 +78,7 @@ void JsonRpcConnection::HandleIncomingMessages(boost::asio::yield_context yc) m_Seen = Utility::GetTime(); try { - CpuBoundWork handleMessage (yc); + CpuBoundWork handleMessage (yc, m_IoStrand); MessageHandler(message); From 26ef66e0425fb814729cebb7cefe01db5d4cf58b Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Thu, 22 Feb 2024 10:55:55 +0100 Subject: [PATCH 6/6] CpuBoundWork#CpuBoundWork(): don't spin on atomic int to acquire slot This is inefficient and involves unfair scheduling. The latter implies possible bad surprises regarding waiting durations on busy nodes. Instead, use AsioConditionVariable#Wait() if there are no free slots. It's notified by others' CpuBoundWork#~CpuBoundWork() once finished. --- lib/base/io-engine.cpp | 51 +++++++++++++++++++++++++++++-------- lib/base/io-engine.hpp | 57 +++++++++++++++++++++++++++--------------- 2 files changed, 78 insertions(+), 30 deletions(-) diff --git a/lib/base/io-engine.cpp b/lib/base/io-engine.cpp index 871981a82ad..a4a813d6910 100644 --- a/lib/base/io-engine.cpp +++ b/lib/base/io-engine.cpp @@ -16,28 +16,55 @@ using namespace icinga; -CpuBoundWork::CpuBoundWork(boost::asio::yield_context yc, boost::asio::io_context::strand&) +CpuBoundWork::CpuBoundWork(boost::asio::yield_context yc, boost::asio::io_context::strand& strand) : m_Done(false) { auto& ioEngine (IoEngine::Get()); + auto& sem (ioEngine.m_CpuBoundSemaphore); + std::unique_lock lock (sem.Mutex); - for (;;) { - auto availableSlots (ioEngine.m_CpuBoundSemaphore.fetch_sub(1)); + if (sem.FreeSlots) { + --sem.FreeSlots; + return; + } + + auto cv (Shared::Make(ioEngine.GetIoContext())); + bool gotSlot = false; + auto pos (sem.Waiting.insert(sem.Waiting.end(), IoEngine::CpuBoundQueueItem{&strand, cv, &gotSlot})); + + lock.unlock(); + + try { + cv->Wait(yc); + } catch (...) { + std::unique_lock lock (sem.Mutex); - if (availableSlots < 1) { - ioEngine.m_CpuBoundSemaphore.fetch_add(1); - IoEngine::YieldCurrentCoroutine(yc); - continue; + if (gotSlot) { + lock.unlock(); + Done(); + } else { + sem.Waiting.erase(pos); } - break; + throw; } } void CpuBoundWork::Done() { if (!m_Done) { - IoEngine::Get().m_CpuBoundSemaphore.fetch_add(1); + auto& sem (IoEngine::Get().m_CpuBoundSemaphore); + std::unique_lock lock (sem.Mutex); + + if (sem.Waiting.empty()) { + ++sem.FreeSlots; + } else { + auto next (sem.Waiting.front()); + + *next.GotSlot = true; + sem.Waiting.pop_front(); + boost::asio::post(*next.Strand, [cv = std::move(next.CV)]() { cv->Set(); }); + } m_Done = true; } @@ -58,7 +85,11 @@ boost::asio::io_context& IoEngine::GetIoContext() IoEngine::IoEngine() : m_IoContext(), m_KeepAlive(boost::asio::make_work_guard(m_IoContext)), m_Threads(decltype(m_Threads)::size_type(Configuration::Concurrency * 2u)), m_AlreadyExpiredTimer(m_IoContext) { m_AlreadyExpiredTimer.expires_at(boost::posix_time::neg_infin); - m_CpuBoundSemaphore.store(Configuration::Concurrency * 3u / 2u); + + { + std::unique_lock lock (m_CpuBoundSemaphore.Mutex); + m_CpuBoundSemaphore.FreeSlots = Configuration::Concurrency * 3u / 2u; + } for (auto& thread : m_Threads) { thread = std::thread(&IoEngine::RunEventLoop, this); diff --git a/lib/base/io-engine.hpp b/lib/base/io-engine.hpp index 622a92dd00c..f370bde6696 100644 --- a/lib/base/io-engine.hpp +++ b/lib/base/io-engine.hpp @@ -6,10 +6,14 @@ #include "base/exception.hpp" #include "base/lazy-init.hpp" #include "base/logger.hpp" +#include "base/shared.hpp" #include "base/shared-object.hpp" #include +#include #include +#include #include +#include #include #include #include @@ -31,7 +35,7 @@ namespace icinga class CpuBoundWork { public: - CpuBoundWork(boost::asio::yield_context yc, boost::asio::io_context::strand&); + CpuBoundWork(boost::asio::yield_context yc, boost::asio::io_context::strand& strand); CpuBoundWork(const CpuBoundWork&) = delete; CpuBoundWork(CpuBoundWork&&) = delete; CpuBoundWork& operator=(const CpuBoundWork&) = delete; @@ -48,6 +52,25 @@ class CpuBoundWork bool m_Done; }; + +/** + * Condition variable which doesn't block I/O threads + * + * @ingroup base + */ +class AsioConditionVariable +{ +public: + AsioConditionVariable(boost::asio::io_context& io, bool init = false); + + void Set(); + void Clear(); + void Wait(boost::asio::yield_context yc); + +private: + boost::asio::deadline_timer m_Timer; +}; + /** * Async I/O engine * @@ -110,6 +133,13 @@ class IoEngine } private: + struct CpuBoundQueueItem + { + boost::asio::io_context::strand* Strand; + Shared::Ptr CV; + bool* GotSlot; + }; + IoEngine(); void RunEventLoop(); @@ -120,29 +150,16 @@ class IoEngine boost::asio::executor_work_guard m_KeepAlive; std::vector m_Threads; boost::asio::deadline_timer m_AlreadyExpiredTimer; - std::atomic_int_fast32_t m_CpuBoundSemaphore; -}; -class TerminateIoThread : public std::exception -{ + struct { + std::mutex Mutex; + uint_fast32_t FreeSlots; + std::list Waiting; + } m_CpuBoundSemaphore; }; -/** - * Condition variable which doesn't block I/O threads - * - * @ingroup base - */ -class AsioConditionVariable +class TerminateIoThread : public std::exception { -public: - AsioConditionVariable(boost::asio::io_context& io, bool init = false); - - void Set(); - void Clear(); - void Wait(boost::asio::yield_context yc); - -private: - boost::asio::deadline_timer m_Timer; }; /**