Conversation
Automatically detect and enable Link Time Optimization for both Corosio and Asio benchmarks. This ensures fair comparisons and optimal performance measurement.
Remove the separate lambda-based benchmark and keep only the coroutine version for fair comparison with corosio. Both libraries now benchmark coroutine dispatch rather than comparing coroutines against callbacks.
Replace per-iteration timeout calculation with Linux timerfd for kernel-managed timer expiry. Add task operation sentinel pattern to interleave reactor runs with handler execution, preventing timer starvation.
- Remove unused any_completed variable - Skip redundant error handling when EPOLLIN/EPOLLOUT already processed ops - Simplify thread notification: notify_one for single completion, notify_all for multiple completions
- Add task sentinel pattern for timer starvation prevention - Simplify do_one() by removing deadline tracking - Optimize thread notification logic - Remove unused includes
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR adds kernel timerfd support to the epoll scheduler, replaces calculate_timeout with update_timerfd, introduces a task_op sentinel to interleave reactor and handler execution, and refactors epoll socket I/O to use coroutine-based initiators (read/write) with new do_read_io/do_write_io flows. Select scheduler receives the task_op sentinel and wake logic changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as EPoll Scheduler
participant Kernel as Kernel (timerfd)
participant Epoll as Epoll
participant TimerSvc as Timer Service
Scheduler->>Kernel: create timerfd
Scheduler->>Epoll: epoll_ctl(ADD, timerfd, EPOLLIN|EPOLLERR)
Note over Scheduler,TimerSvc: main reactor loop
Scheduler->>TimerSvc: query nearest timer expiry
Scheduler->>Kernel: update_timerfd(expiry)
Scheduler->>Epoll: epoll_wait(timeout|-1)
alt Timerfd fires
Kernel->>Epoll: EPOLLIN on timerfd
Epoll->>Scheduler: return timerfd event
Scheduler->>Scheduler: detect timerfd, skip normal processing
Scheduler->>Scheduler: requeue/handle task_op_ to run reactor/handlers
else Socket I/O
Epoll->>Scheduler: socket event
Scheduler->>Scheduler: process socket completion
end
sequenceDiagram
participant App as Application
participant Socket as epoll_socket_impl
participant Initiator as Read/Write Initiator (coroutine)
participant KernelIO as Kernel I/O
App->>Socket: read_some(buffers)
Socket->>Initiator: make_read_initiator(&cached, impl)
Initiator->>Initiator: initial_suspend (frame reused or new)
Initiator->>Socket: do_read_io()
Socket->>KernelIO: readv()/recvmsg()
alt Data available
KernelIO->>Socket: bytes read
Socket->>Socket: complete op, post completion
Socket->>Initiator: symmetric transfer / resume
else EAGAIN (would block)
KernelIO->>Socket: EAGAIN
Socket->>Socket: schedule in-flight op, suspend initiator
KernelIO->>Epoll: wait for readability
end
Initiator->>Initiator: final_suspend -> destroy (frame cleanup)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
An automated preview of the documentation is available at https://101.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-02-03 21:18:03 UTC |
Use initiator coroutines for read/write operations to ensure the caller's coroutine is fully suspended before I/O starts. This prevents a race condition in multi-threaded scenarios where immediate completion could try to resume a coroutine that isn't fully suspended yet. - Add read_initiator and write_initiator coroutine types with cached frame allocation to avoid per-operation heap allocations - Move I/O logic to do_read_io() and do_write_io() methods - Modify read_some/write_some to return initiator handles for symmetric transfer - Add destructor to clean up initiator frames and handles
|
GCOVR code coverage report https://101.corosio.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-02-03 21:22:34 UTC |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #101 +/- ##
===========================================
- Coverage 80.55% 80.54% -0.01%
===========================================
Files 61 63 +2
Lines 5272 5331 +59
===========================================
+ Hits 4247 4294 +47
- Misses 1025 1037 +12
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/corosio/src/detail/epoll/scheduler.cpp (1)
574-709:⚠️ Potential issue | 🟠 MajorRead timerfd to clear pending expirations.
The comment asserting that
timerfd_settime()resets the readable state is incorrect per Linux documentation. Pertimerfd_gettime(2), a timerfd becomes readable when expirations occur since the lasttimerfd_settime()or since the last successfulread()—callingtimerfd_settime()does not consume accumulated expirations. Skipping the timerfd event without reading it leaves the fd readable, which can causeepoll_wait()to spin immediately on the next iteration if no other descriptors are ready.Read the timerfd to drain pending expirations, matching the pattern already used for eventfd at line 596:
Proposed fix
- // timerfd_settime() in update_timerfd() resets the readable state - if (events[i].data.ptr == &timer_fd_) - continue; + // Clear timerfd readability by draining the expiration count + if (events[i].data.ptr == &timer_fd_) + { + std::uint64_t expirations; + [[maybe_unused]] auto r = ::read(timer_fd_, &expirations, sizeof(expirations)); + continue; + }
🤖 Fix all issues with AI agents
In `@src/corosio/src/detail/epoll/scheduler.cpp`:
- Around line 533-568: In epoll_scheduler::update_timerfd, check the return
value of ::timerfd_settime(timer_fd_, flags, &ts, nullptr); and handle errors
the same way other syscalls in this file do: if it returns -1 capture errno and
surface it (e.g., log via the file's logger and/or throw a std::system_error
with std::error_code(errno, std::generic_category())) so failures to arm/disarm
timer_fd_ are not ignored; update the function to perform that check and
error-path behavior consistent with epoll_ctl and timerfd_create handling in
this file.
🧹 Nitpick comments (2)
src/corosio/src/detail/epoll/sockets.hpp (1)
86-162: Consider unifyingread_initiatorandwrite_initiatorinto a template.Both initiator types are structurally identical, differing only in names. A parameterized template (e.g., on a tag type or a
void (epoll_socket_impl::*)()member pointer) could eliminate the duplication:template<auto DoIo> struct io_initiator { /* ... */ }; using read_initiator = io_initiator<&epoll_socket_impl::do_read_io>; using write_initiator = io_initiator<&epoll_socket_impl::do_write_io>;That said, the current explicit approach is clear and matches the IOCP backend pattern shown in
src/corosio/src/detail/iocp/sockets.hpp, so keeping them separate for consistency is reasonable.src/corosio/src/detail/epoll/sockets.cpp (1)
319-325: Explicitn == 0case might improve clarity.Unlike
do_read_io()which explicitly handlesn == 0(EOF),do_write_io()lets it fall through to the error path at line 365. For stream sockets with non-empty buffers,sendmsgreturning 0 is unexpected (it should return -1 with an error on connection issues), so the current defensive behavior is reasonable.However, an explicit check would make the intent clearer:
Optional: explicit handling
if (n > 0) { desc_data_.write_ready.store(false, std::memory_order_relaxed); op.complete(0, static_cast<std::size_t>(n)); svc_.post(&op); return; } +if (n == 0) +{ + // Unexpected for non-empty write; treat as would-block and retry + desc_data_.write_ready.store(false, std::memory_order_relaxed); +} + if (errno == EAGAIN || errno == EWOULDBLOCK)
Consistent with other syscall error handling in this file.
2 participants
Summary by CodeRabbit
Refactor
Bug Fixes