Conversation
Reorganize bench/ → perf/ with cleaner subdirectory layout: perf/bench/ for benchmarks, perf/profile/ for profiler workloads, and perf/common/ for shared utilities. Merge the separate corosio_bench and asio_bench executables into a single corosio_bench binary with --library selection. Convert iteration-based benchmarks to time-based using a configurable --duration flag. Split namespaces into perf:: for shared utilities and bench:: for benchmark-specific types. Rename CMake option to BOOST_COROSIO_BUILD_PERF.
Move coroutine benchmarks into asio/coroutine/ subdirectory and add matching callback-based benchmarks in asio/callback/ using recursive async op structs. Add --library asio_callback option to the bench binary.
📝 WalkthroughWalkthroughThis pull request renames the benchmarking infrastructure from "bench" to "perf," migrates the directory structure (bench/ to perf/), and refactors the backend dispatch mechanism to use factory-based context creation instead of template specialization. JSON serialization functionality is removed. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
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)
perf/profile/concurrent_io_bench.cpp (1)
117-148:⚠️ Potential issue | 🔴 CriticalData race on
last_count— undefined behavior.
last_countis a plainstd::uint64_tshared by all worker threads. Every thread independently triggers the reporting path (each has its ownnext_reportlocal on line 127), so multiple threads concurrently read (line 139) and write (line 144) this variable without synchronization. The comment on line 133 ("Only first thread reports progress") is not enforced by any mechanism.In
scheduler_contention_bench.cppthe equivalent variable is correctly declaredstd::atomic<std::uint64_t>and updated viaexchange(). Apply the same pattern here.🐛 Proposed fix — make last_count atomic and use exchange
- std::uint64_t last_count = 0; + std::atomic<std::uint64_t> last_count{0};Inside the worker lambda, replace the read and write:
- std::uint64_t current = ops.load(std::memory_order_relaxed); - double rate = static_cast<double>(current - last_count) / 2.0; - - std::cout << " [" << std::fixed << std::setprecision(0) << elapsed << "s] " - << perf::format_rate(rate) << " (" << current << " total)\n"; - - last_count = current; + std::uint64_t current = ops.load(std::memory_order_relaxed); + std::uint64_t last = last_count.exchange(current, std::memory_order_relaxed); + double rate = static_cast<double>(current - last) / 2.0; + + std::cout << " [" << std::fixed << std::setprecision(0) << elapsed << "s] " + << perf::format_rate(rate) << " (" << current << " total)\n";Additionally, consider guarding the report interval with a shared
std::atomic<int> next_report_secandcompare_exchange_strong(as done inscheduler_contention_bench.cpp) so only one thread prints per interval.
🤖 Fix all issues with AI agents
In `@perf/common/backend_selection.hpp`:
- Around line 74-102: dispatch_backend() is missing a BOOST_COROSIO_HAS_KQUEUE
branch so requests for the "kqueue" backend fall through; add a guarded branch
similar to the existing epoll/select/iocp cases: under `#if`
BOOST_COROSIO_HAS_KQUEUE check for std::strcmp(backend, "kqueue") == 0 and
invoke func with a lambda returning std::make_unique<corosio::kqueue_context>()
and the name "kqueue", then return 0; this aligns dispatch_backend() with
default_backend_name() and print_available_backends() and uses the
kqueue_context symbol.
🧹 Nitpick comments (4)
CMakeLists.txt (1)
37-37: RenamingBOOST_COROSIO_BUILD_BENCH→BOOST_COROSIO_BUILD_PERFis a user-facing breaking change.Anyone passing
-DBOOST_COROSIO_BUILD_BENCH=ONin CI scripts or local build invocations will silently lose the perf build. Consider adding a one-release deprecation shim:💡 Optional backward-compat shim
# Place before the new option definition if(DEFINED BOOST_COROSIO_BUILD_BENCH) message(DEPRECATION "BOOST_COROSIO_BUILD_BENCH is deprecated, use BOOST_COROSIO_BUILD_PERF") set(BOOST_COROSIO_BUILD_PERF ${BOOST_COROSIO_BUILD_BENCH}) endif()perf/profile/small_io_bench.cpp (1)
259-270:std::atoion user input has no error detection and can produce surprising results.
std::atoireturns 0 on failure (indistinguishable from a valid "0") and has undefined behavior on out-of-range values. For--buffer, a negative input silently wraps to a hugesize_t. For a profiler tool this is unlikely to cause real harm, but if you ever tighten input handling consider switching tostd::strtolwith range checks.perf/common/perf.hpp (1)
10-11: Nit:#endiflacks the guard comment, unlikebackend_selection.hpp.Minor inconsistency —
backend_selection.hppends with#endif // BOOST_COROSIO_PERF_BACKEND_SELECTION_HPPwhile this file just has#endif.Consistency fix
-#endif +#endif // BOOST_COROSIO_PERF_HPPAlso applies to: 238-240
perf/profile/scheduler_contention_bench.cpp (1)
110-122:post_only_runneris dead code; inline runner lambda omits yield.
post_only_runner(lines 110-122) is defined but never called. The runner lambda at lines 260-264 duplicates its logic, yet drops thestd::this_thread::yield()whenpoll()returns 0, causing a pure busy-spin. If the omission of yield is intentional for maximum contention, removepost_only_runnerentirely. If not, either call the existing function or add the yield to the lambda.♻️ Option A: Remove the dead function and use the lambda (if busy-spin is intentional)
-// Runner thread for post-only mode - only runs, never posts -void post_only_runner( - corosio::basic_io_context& ioc, - std::atomic<bool>& stop) -{ - while (!stop.load(std::memory_order_relaxed)) - { - auto n = ioc.poll(); - if (n == 0) - std::this_thread::yield(); - } - // Drain remaining work - ioc.poll(); -}♻️ Option B: Use the existing function (if yield is desired)
for (int t = 0; t < num_runners; ++t) { runners.emplace_back([&]() { - while (!stop.load(std::memory_order_relaxed)) - ioc->poll(); - ioc->poll(); // Drain + post_only_runner(*ioc, stop); }); }Also applies to: 260-264
| #if BOOST_COROSIO_HAS_EPOLL | ||
| if (std::strcmp(backend, "epoll") == 0) | ||
| { | ||
| func.template operator()<corosio::epoll_context>("epoll"); | ||
| func([]() -> std::unique_ptr<corosio::basic_io_context> { | ||
| return std::make_unique<corosio::epoll_context>(); | ||
| }, "epoll"); | ||
| return 0; | ||
| } | ||
| #endif | ||
|
|
||
| #if BOOST_COROSIO_HAS_SELECT | ||
| if (std::strcmp(backend, "select") == 0) | ||
| { | ||
| func.template operator()<corosio::select_context>("select"); | ||
| func([]() -> std::unique_ptr<corosio::basic_io_context> { | ||
| return std::make_unique<corosio::select_context>(); | ||
| }, "select"); | ||
| return 0; | ||
| } | ||
| #endif | ||
|
|
||
| #if BOOST_COROSIO_HAS_IOCP | ||
| if (std::strcmp(backend, "iocp") == 0) | ||
| { | ||
| func.template operator()<corosio::iocp_context>("iocp"); | ||
| func([]() -> std::unique_ptr<corosio::basic_io_context> { | ||
| return std::make_unique<corosio::iocp_context>(); | ||
| }, "iocp"); | ||
| return 0; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check if kqueue_context exists
echo "=== Searching for kqueue_context ==="
rg -n 'kqueue_context' --type=cpp --type-add 'cpp:*.hpp'
echo ""
echo "=== Checking for BOOST_COROSIO_HAS_KQUEUE ==="
rg -n 'BOOST_COROSIO_HAS_KQUEUE' --type=cpp --type-add 'cpp:*.hpp'Repository: cppalliance/corosio
Length of output: 841
🏁 Script executed:
#!/bin/bash
# Now examine the full backend_selection.hpp file to see the complete context
echo "=== Reading backend_selection.hpp ==="
wc -l perf/common/backend_selection.hppRepository: cppalliance/corosio
Length of output: 138
🏁 Script executed:
#!/bin/bash
# Read the entire file to understand the structure
cat -n perf/common/backend_selection.hppRepository: cppalliance/corosio
Length of output: 3976
Add missing kqueue dispatch branch to dispatch_backend().
default_backend_name() (line 33) returns "kqueue" when BOOST_COROSIO_HAS_KQUEUE is defined, and print_available_backends() (lines 51–52) advertises it. However, dispatch_backend() lacks a corresponding #if BOOST_COROSIO_HAS_KQUEUE case. On BSD/macOS, a call to dispatch_backend("kqueue", func) will fall through to the error at line 104 instead of dispatching to the kqueue implementation.
Add a kqueue branch between the select and iocp cases (after line 92) to match the advertised backends:
Suggested kqueue branch
`#if` BOOST_COROSIO_HAS_KQUEUE
if (std::strcmp(backend, "kqueue") == 0)
{
func([]() -> std::unique_ptr<corosio::basic_io_context> {
return std::make_unique<corosio::kqueue_context>();
}, "kqueue");
return 0;
}
`#endif`🤖 Prompt for AI Agents
In `@perf/common/backend_selection.hpp` around lines 74 - 102, dispatch_backend()
is missing a BOOST_COROSIO_HAS_KQUEUE branch so requests for the "kqueue"
backend fall through; add a guarded branch similar to the existing
epoll/select/iocp cases: under `#if` BOOST_COROSIO_HAS_KQUEUE check for
std::strcmp(backend, "kqueue") == 0 and invoke func with a lambda returning
std::make_unique<corosio::kqueue_context>() and the name "kqueue", then return
0; this aligns dispatch_backend() with default_backend_name() and
print_available_backends() and uses the kqueue_context symbol.
|
An automated preview of the documentation is available at https://107.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-06 20:43:28 UTC |
|
GCOVR code coverage report https://107.corosio.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-02-06 20:49:40 UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #107 +/- ##
========================================
Coverage 80.06% 80.06%
========================================
Files 65 65
Lines 5569 5569
========================================
Hits 4459 4459
Misses 1110 1110 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Summary by CodeRabbit
Release Notes
BOOST_COROSIO_BUILD_BENCHtoBOOST_COROSIO_BUILD_PERFfor performance tools configuration