Skip to content

Benchmark enhancements#107

Merged
sgerbino merged 2 commits intocppalliance:developfrom
sgerbino:pr/bench
Feb 6, 2026
Merged

Benchmark enhancements#107
sgerbino merged 2 commits intocppalliance:developfrom
sgerbino:pr/bench

Conversation

@sgerbino
Copy link
Collaborator

@sgerbino sgerbino commented Feb 6, 2026

Summary by CodeRabbit

Release Notes

  • Chores
    • Renamed build flag from BOOST_COROSIO_BUILD_BENCH to BOOST_COROSIO_BUILD_PERF for performance tools configuration
    • Reorganized performance tooling directory structure and internal architecture
    • Removed JSON result export functionality from performance measurement utilities

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.
@coderabbitai
Copy link

coderabbitai bot commented Feb 6, 2026

📝 Walkthrough

Walkthrough

This 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

Cohort / File(s) Summary
CMake Configuration
CMakeLists.txt, perf/CMakeLists.txt
Renames build option from BOOST_COROSIO_BUILD_BENCH to BOOST_COROSIO_BUILD_PERF; updates comment from "Benchmarks" to "Performance tools"; adds perf/CMakeLists.txt to include bench and profile subdirectories.
Backend Selection Infrastructure
perf/common/backend_selection.hpp
Renames header guard and namespace from bench to perf; introduces context_factory type alias for factory functions; refactors dispatch_backend to accept factory lambdas instead of template-based backend selection; each backend (epoll, select, iocp) now returns a unique_ptr<basic_io_context> via factory rather than direct template instantiation.
Performance Header & Utilities
perf/common/perf.hpp
Removes metric, benchmark_result, and result_collector classes along with JSON serialization logic; retains stopwatch-related functionality; namespace changed from bench to perf; header guard renamed.
Profile Workloads
perf/profile/concurrent_io_bench.cpp, perf/profile/coroutine_post_bench.cpp, perf/profile/queue_depth_bench.cpp, perf/profile/scheduler_contention_bench.cpp, perf/profile/small_io_bench.cpp
Uniform refactoring across five profile workload files: run_workload and run_profiler_workload signatures change from template-based Context parameter to perf::context_factory; IO context instantiation via factory() with pointer-based member access (ioc->...); all namespace references updated from bench:: to perf::; include headers changed from benchmark.hpp to perf.hpp.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 Benchmarks transformed to perf with care,
Factory lambdas floating through the air,
From templates bold to closures bright,
The backends dance in factory light,
JSON stripped, but context flows just right! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Benchmark enhancements' is vague and generic, failing to convey the specific nature of changes which involve renaming benchmark features to performance tools and restructuring directory organization. Consider a more specific title that reflects the main changes, such as 'Rename benchmark infrastructure to performance tools and restructure directories' or 'Refactor bench namespace and options to perf'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 | 🔴 Critical

Data race on last_count — undefined behavior.

last_count is a plain std::uint64_t shared by all worker threads. Every thread independently triggers the reporting path (each has its own next_report local 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.cpp the equivalent variable is correctly declared std::atomic<std::uint64_t> and updated via exchange(). 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_sec and compare_exchange_strong (as done in scheduler_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: Renaming BOOST_COROSIO_BUILD_BENCHBOOST_COROSIO_BUILD_PERF is a user-facing breaking change.

Anyone passing -DBOOST_COROSIO_BUILD_BENCH=ON in 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::atoi on user input has no error detection and can produce surprising results.

std::atoi returns 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 huge size_t. For a profiler tool this is unlikely to cause real harm, but if you ever tighten input handling consider switching to std::strtol with range checks.

perf/common/perf.hpp (1)

10-11: Nit: #endif lacks the guard comment, unlike backend_selection.hpp.

Minor inconsistency — backend_selection.hpp ends with #endif // BOOST_COROSIO_PERF_BACKEND_SELECTION_HPP while this file just has #endif.

Consistency fix
-#endif
+#endif // BOOST_COROSIO_PERF_HPP

Also applies to: 238-240

perf/profile/scheduler_contention_bench.cpp (1)

110-122: post_only_runner is 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 the std::this_thread::yield() when poll() returns 0, causing a pure busy-spin. If the omission of yield is intentional for maximum contention, remove post_only_runner entirely. 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

Comment on lines 74 to 102
#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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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.hpp

Repository: cppalliance/corosio

Length of output: 138


🏁 Script executed:

#!/bin/bash
# Read the entire file to understand the structure
cat -n perf/common/backend_selection.hpp

Repository: 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.

@cppalliance-bot
Copy link

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

@cppalliance-bot
Copy link

GCOVR code coverage report https://107.corosio.prtest3.cppalliance.org/gcovr/index.html
LCOV code coverage report https://107.corosio.prtest3.cppalliance.org/genhtml/index.html
Coverage Diff Report https://107.corosio.prtest3.cppalliance.org/diff-report/index.html

Build time: 2026-02-06 20:49:40 UTC

@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.06%. Comparing base (0feb528) to head (3e26aa1).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0feb528...3e26aa1. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sgerbino sgerbino merged commit ac5807f into cppalliance:develop Feb 6, 2026
30 of 31 checks passed
@sgerbino sgerbino deleted the pr/bench branch February 6, 2026 20:59
@coderabbitai coderabbitai bot mentioned this pull request Feb 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants