-
Notifications
You must be signed in to change notification settings - Fork 7
add uring_context #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Casey Bodley <[email protected]>
uring_context manages a single instance of io_uring which has an associated submission queue and completion queue each async operation prepares a submission queue entry (sqe) but does not call io_uring_submit() to submit it (except for resume_at(), see comment). this allows sqes to be submitted in batches by run_one(), where a single system call in io_uring_submit_and_wait() can submit pending sqes and await the next completion queue entry (cqe) io_uring_sqe_set_data() associates each sqe with its io_base pointer. run_one() calls io_uring_cqe_get_data() to retreive that pointer and call its work() function to invoke complete()/cancel()/error() depending on the return code in cqe->res unlike poll_context, cancel() and resume_at() are treated as their own async operations initiated with io_uring_prep_cancel() and io_uring_prep_timeout() respectively Signed-off-by: Casey Bodley <[email protected]>
examples/CMakeLists.txt
Outdated
| if (BEMAN_NET_WITH_URING) | ||
| target_compile_definitions(${EXAMPLE_TARGET} PRIVATE BEMAN_NET_USE_URING) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[pre-commit] reported by reviewdog 🐶
| if (BEMAN_NET_WITH_URING) | |
| target_compile_definitions(${EXAMPLE_TARGET} PRIVATE BEMAN_NET_USE_URING) | |
| if(BEMAN_NET_WITH_URING) | |
| target_compile_definitions( | |
| ${EXAMPLE_TARGET} | |
| PRIVATE BEMAN_NET_USE_URING | |
| ) |
| #else | ||
| ::std::unique_ptr<::beman::net::detail::context_base> d_owned{new ::beman::net::detail::poll_context()}; | ||
| #endif | ||
| ::beman::net::detail::context_base& d_context{*this->d_owned}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[pre-commit] reported by reviewdog 🐶
| ::beman::net::detail::context_base& d_context{*this->d_owned}; | |
| ::beman::net::detail::context_base& d_context{*this->d_owned}; |
for testing, this BEMAN_NET_USE_URING define is added to example targets when cmake option BEMAN_NET_WITH_URING is enabled Signed-off-by: Casey Bodley <[email protected]>
with poll_context: when stop_source.request_stop() calls poll_context::cancel(), the target operation's io_base::cancel() is called inline. this calls sender_awaiter::stop() which transitions from stop_state::stopping to stopped. because request_stop() returns back to callback_t in stop_state::stopped, complete_stopped() is called there with uring_context: uring_context::cancel() is asynchronous, so callback_t is still in stop_state::stopping when request_stop() returns. run_one() eventually sees the target operation complete with ECANCELED and calls io_base::cancel(). when that calls sender_awaiter::stop(), the state is still stop_state::stopping, so complete_stopped() never gets called this causes beman.net.examples.client to exit before demo::scope gets the stop signal from the 2 coroutines: > ERROR: scope destroyed with live jobs: 2 to address this, callback_t now handles this async case by transitioning back to stop_state::running after stop_source.request_stop() returns Signed-off-by: Casey Bodley <[email protected]>
dietmarkuehl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven’t looked at the actual implementation, yet, but I hope to provide feedback on that soon, too (it may be early next week - I aim for 2025-11-12 the latest).
Thank you very much for the contribution!
| set(TARGET_ALIAS beman::${TARGET_NAME}) | ||
| set(TARGETS_EXPORT_NAME ${CMAKE_PROJECT_NAME}) | ||
|
|
||
| option(BEMAN_NET_WITH_URING "Enable liburing io context" OFF) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Linux there are at least 4 sequencers: select, poll, epoll_*, and iouring_*. On other systems there are additional sequence, e.g. kqueue* and IOCP. The interface context_base and io_context are set up to make it an object creation choice which sequencer is used. My vision on setting this up is to have cmake-level checks to determine what implementations are being build. There should be a default sequencer and that may either be prioritized based on what sequencers can be build, an cmake-option, or a combination thereof.
I can provide examples of how I think that could look like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @dietmarkuehl, thanks for the feedback here - and for your standardization efforts as well
my initial goal was just to treat existing functionality as the default to avoid breaking things that are already stable. but i'm happy to explore a better design for this part
coming from an asio background, i'm used to the epoll backend as default but with preprocessor macros to customize. regarding uring, from https://www.boost.org/doc/libs/latest/doc/html/boost_asio/using.html:
The backend is disabled by default, and must be enabled by defining both BOOST_ASIO_HAS_IO_URING and BOOST_ASIO_DISABLE_EPOLL.
while BOOST_ASIO_DISABLE_EPOLL alone "disables epoll support on Linux, forcing the use of a select-based implementation."
as a header-only library, asio relies on the application to link against liburing so the application has to opt in by defining BOOST_ASIO_HAS_IO_URING
my intuition for linux is that the preference should be uring -> epoll -> poll. while asio is concerned about supporting older kernels without the required uring functionality, that may not be necessary for a library targeting a future standard
because this isn't a header-only library, selection of uring does need to happen during the configure step. so i'm thinking this cmake variable should default to ON where support is expected:
cmake_dependent_option(BEMAN_NET_WITH_URING "Enable liburing io context" ON "CMAKE_SYSTEM_NAME MATCHES Linux" OFF)when enabled, this option would inject a compile definition like BEMAN_NET_HAS_URING:
target_compile_definitions(${TARGET_LIBRARY} PUBLIC BEMAN_NET_HAS_URING)allowing io_context to select the backend with:
#if defined(BEMAN_NET_HAS_URING) && !defined(BEMAN_NET_DISABLE_URING)
::std::unique_ptr<::beman::net::detail::context_base> d_owned{new ::beman::net::detail::uring_context()};
#elif defined(BEMAN_NET_HAS_EPOLL) && !defined(BEMAN_NET_DISABLE_EPOLL)
::std::unique_ptr<::beman::net::detail::context_base> d_owned{new ::beman::net::detail::epoll_context()};
#else
::std::unique_ptr<::beman::net::detail::context_base> d_owned{new ::beman::net::detail::poll_context()};
#endifso to summarize: the library defines the order of preference for each platform, and the application can customize using the DISABLE defines
| #include <beman/net/detail/io_context_scheduler.hpp> | ||
| #ifdef BEMAN_NET_USE_URING | ||
| #include <beman/net/detail/uring_context.hpp> | ||
| #else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As commented above, I don’t think these should be exclusive.
| private: | ||
| #ifdef BEMAN_NET_USE_URING | ||
| ::std::unique_ptr<::beman::net::detail::context_base> d_owned{new ::beman::net::detail::uring_context()}; | ||
| #else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is fine, though (in principle; I’d think the details may be different): here the the default is chosen.
introduce a uring backend, with cmake option BEMAN_NET_WITH_URING to enable the liburing dependency and #ifdef BEMAN_NET_USE_URING to select uring_context instead of poll_context
uring_context manages a single instance of io_uring which has an associated submission queue and completion queue
each async operation prepares a submission queue entry (sqe) but does not call io_uring_submit() to submit it (except for resume_at(), see comment). this allows sqes to be submitted in batches by run_one(), where a single system call in io_uring_submit_and_wait() can submit pending sqes and await the next completion queue entry (cqe)
io_uring_sqe_set_data() associates each sqe with its io_base pointer. run_one() calls io_uring_cqe_get_data() to retreive that pointer and call its work() function to invoke complete()/cancel()/error() depending on the return code in cqe->res
unlike poll_context, cancel() and resume_at() are treated as their own async operations initiated with io_uring_prep_cancel() and io_uring_prep_timeout() respectively
this async cancellation strategy was not compatible with demo::task, which did not call complete_stopped() unless the cancelled op completes inside of stop_source.request_stop() (see commit message for details). the proposed solution seems to work for both poll_context and uring_context
tested primarily with the client/server examples