From cbd88ebbe9677bc7285c0b9e4f7577a3a2a44296 Mon Sep 17 00:00:00 2001 From: Debra Date: Sun, 16 Aug 2026 15:49:04 +0800 Subject: [PATCH 1/6] build: add lint, CI, test, and benchmark tooling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - .clang-format (Google style, 100 cols) and .clang-tidy (bugprone, concurrency, google, modernize, performance, readability checks with Google-style naming rules) - GitHub Actions CI: TSan test matrix (ubuntu/macos), Release benchmark smoke run, clang-format + clang-tidy lint job - CMake build: header-only cq INTERFACE library, warnings target (-Wall -Wextra -Wpedantic -Wconversion, clang -Wdocumentation for Doxygen tag validation), GoogleTest and Google Benchmark via FetchContent - GoogleTest unit + stress tests and Google Benchmark suite for MutexQueue - VS Code format-on-save settings, STYLE.md comment/layout standards, .gitignore Note: tests and benchmarks reference include/cq/mutex_queue.hpp, which lands in a separate code PR — CI stays red until that merges. Co-Authored-By: Claude Fable 5 --- .clang-format | 5 ++ .clang-tidy | 33 ++++++++ .github/workflows/ci.yml | 48 ++++++++++++ .gitignore | 4 + .vscode/settings.json | 12 +++ CMakeLists.txt | 59 ++++++++++++++ STYLE.md | 34 ++++++++ bench/.clang-tidy | 5 ++ bench/CMakeLists.txt | 2 + bench/queue_bench.cpp | 84 ++++++++++++++++++++ tests/.clang-tidy | 5 ++ tests/CMakeLists.txt | 7 ++ tests/mutex_queue_test.cpp | 154 +++++++++++++++++++++++++++++++++++++ tests/stress_test.cpp | 73 ++++++++++++++++++ 14 files changed, 525 insertions(+) create mode 100644 .clang-format create mode 100644 .clang-tidy create mode 100644 .github/workflows/ci.yml create mode 100644 .gitignore create mode 100644 .vscode/settings.json create mode 100644 CMakeLists.txt create mode 100644 STYLE.md create mode 100644 bench/.clang-tidy create mode 100644 bench/CMakeLists.txt create mode 100644 bench/queue_bench.cpp create mode 100644 tests/.clang-tidy create mode 100644 tests/CMakeLists.txt create mode 100644 tests/mutex_queue_test.cpp create mode 100644 tests/stress_test.cpp diff --git a/.clang-format b/.clang-format new file mode 100644 index 0000000..50c4175 --- /dev/null +++ b/.clang-format @@ -0,0 +1,5 @@ +BasedOnStyle: Google +ColumnLimit: 100 +DerivePointerAlignment: false +PointerAlignment: Left +IncludeBlocks: Preserve diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..339fe40 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,33 @@ +--- +# Google style plus correctness/concurrency checks. Naming follows the Google +# C++ Style Guide, with STL-style lower_case method names for container-like +# types (explicitly permitted by the guide for STL-consistent interfaces). +Checks: > + bugprone-*, + clang-analyzer-*, + concurrency-*, + google-*, + misc-*, + modernize-*, + performance-*, + readability-*, + -modernize-use-trailing-return-type, + -misc-include-cleaner +WarningsAsErrors: '*' +HeaderFilterRegex: 'include/cq/.*' +CheckOptions: + # Allow the `for (auto _ : state)` benchmark idiom. + readability-identifier-length.IgnoredVariableNames: '^_$' + readability-identifier-naming.ClassCase: CamelCase + readability-identifier-naming.StructCase: CamelCase + readability-identifier-naming.EnumCase: CamelCase + readability-identifier-naming.TypeAliasCase: CamelCase + readability-identifier-naming.FunctionCase: lower_case + # Google Benchmark's BM_PascalCase convention. + readability-identifier-naming.FunctionIgnoredRegexp: '^BM_.*' + readability-identifier-naming.VariableCase: lower_case + readability-identifier-naming.ParameterCase: lower_case + readability-identifier-naming.PrivateMemberSuffix: '_' + readability-identifier-naming.ConstexprVariablePrefix: 'k' + readability-identifier-naming.ConstexprVariableCase: CamelCase + readability-identifier-naming.NamespaceCase: lower_case diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..e6c2d95 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,48 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + +jobs: + test-tsan: + name: Tests (ThreadSanitizer, ${{ matrix.os }}) + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest] + steps: + - uses: actions/checkout@v4 + - name: Configure + run: cmake -B build-tsan -DENABLE_TSAN=ON -DCMAKE_BUILD_TYPE=Debug + - name: Build + run: cmake --build build-tsan -j + - name: Test + run: ctest --test-dir build-tsan --output-on-failure -j "$(getconf _NPROCESSORS_ONLN)" + + bench-build: + name: Benchmarks compile (Release) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Configure + run: cmake -B build-rel -DCMAKE_BUILD_TYPE=Release -DCQ_BUILD_TESTS=OFF + - name: Build + run: cmake --build build-rel -j + - name: Smoke-run benchmarks + run: ./build-rel/bench/queue_bench --benchmark_min_time=0.01s + + lint: + name: clang-format & clang-tidy + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + # clang-format and clang-tidy ship preinstalled on ubuntu-latest. + - name: clang-format + run: find include tests bench \( -name '*.hpp' -o -name '*.ipp' -o -name '*.cpp' \) | xargs clang-format --dry-run --Werror + - name: clang-tidy + run: | + cmake -B build-lint -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXPORT_COMPILE_COMMANDS=ON + find tests bench -name '*.cpp' | xargs clang-tidy -p build-lint diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..48b067f --- /dev/null +++ b/.gitignore @@ -0,0 +1,4 @@ +build*/ +.cache/ +compile_commands.json +.DS_Store diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..d0ace93 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,12 @@ +{ + // Template implementation files are C++ (included from headers). + "files.associations": { + "*.ipp": "cpp" + }, + // Relayout on save via the repo's .clang-format (Prettier-style). + // Works with either the clangd extension or Microsoft's C/C++ extension. + "[cpp]": { + "editor.formatOnSave": true + }, + "C_Cpp.formatting": "clangFormat" +} diff --git a/CMakeLists.txt b/CMakeLists.txt new file mode 100644 index 0000000..add5cd3 --- /dev/null +++ b/CMakeLists.txt @@ -0,0 +1,59 @@ +cmake_minimum_required(VERSION 3.24) + +project(concurrent_queue + VERSION 0.1.0 + DESCRIPTION "Concurrent queues in C++20, from locked baseline to lock-free" + LANGUAGES CXX) + +option(ENABLE_TSAN "Build with ThreadSanitizer" OFF) +option(CQ_BUILD_TESTS "Build unit and stress tests" ON) +option(CQ_BUILD_BENCHMARKS "Build Google Benchmark targets" ON) + +# Header-only library; consumers inherit C++20 via the compile feature. +add_library(cq INTERFACE) +add_library(cq::cq ALIAS cq) +target_include_directories(cq INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/include) +target_compile_features(cq INTERFACE cxx_std_20) + +add_library(cq_warnings INTERFACE) +target_compile_options(cq_warnings INTERFACE + $<$:-Wall -Wextra -Wpedantic -Wconversion> + # Validates Doxygen comments against signatures (clang-only). + $<$:-Wdocumentation> + $<$:/W4>) + +if(ENABLE_TSAN) + add_compile_options(-fsanitize=thread -g -fno-omit-frame-pointer) + add_link_options(-fsanitize=thread) +endif() + +include(FetchContent) + +if(CQ_BUILD_TESTS) + enable_testing() + FetchContent_Declare( + googletest + URL https://github.com/google/googletest/archive/refs/tags/v1.15.2.tar.gz + URL_HASH SHA256=7b42b4d6ed48810c5362c265a17faebe90dc2373c885e5216439d37927f02926 + DOWNLOAD_EXTRACT_TIMESTAMP TRUE) + set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) + set(INSTALL_GTEST OFF CACHE BOOL "" FORCE) + FetchContent_MakeAvailable(googletest) + add_subdirectory(tests) +endif() + +if(CQ_BUILD_BENCHMARKS) + if(ENABLE_TSAN) + message(STATUS "ThreadSanitizer build: skipping benchmarks") + else() + FetchContent_Declare( + benchmark + URL https://github.com/google/benchmark/archive/refs/tags/v1.9.1.tar.gz + URL_HASH SHA256=32131c08ee31eeff2c8968d7e874f3cb648034377dfc32a4c377fa8796d84981 + DOWNLOAD_EXTRACT_TIMESTAMP TRUE) + set(BENCHMARK_ENABLE_TESTING OFF CACHE BOOL "" FORCE) + set(BENCHMARK_ENABLE_INSTALL OFF CACHE BOOL "" FORCE) + FetchContent_MakeAvailable(benchmark) + add_subdirectory(bench) + endif() +endif() diff --git a/STYLE.md b/STYLE.md new file mode 100644 index 0000000..9acd30c --- /dev/null +++ b/STYLE.md @@ -0,0 +1,34 @@ +# Style guide + +This project follows the [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html) +for formatting and naming, enforced by `.clang-format` and `.clang-tidy` (both +checked in CI). + +## Comments + +- **Public API** (everything under `include/cq/`): Doxygen `///` comments on + every public class and member. Use `@tparam`, `@param` / `@param[out]`, + `@return`, and `@throws` tags. State the contract: blocking behavior, + error/close semantics, ownership, and thread-safety. +- **Internal code** (tests, benchmarks, private members, function bodies): + plain `//` prose. Explain *why*, not *what*. +- Tag hygiene is compiler-enforced: clang builds compile with + `-Wdocumentation`, which rejects `@param` names that do not match the + signature. Keep comments in sync with code or the build fails. +- `TODO(username): description` for known follow-ups. + +## Layout + +- Headers (`.hpp`) declare; template member definitions live in a matching + `.ipp` included at the bottom of the header. No function bodies in class + definitions. +- Special member functions (constructors, copy/move operations, destructor) + stay grouped at the top of the `public:` section, with a comment explaining + any deleted operations. + +## Tooling + +- Format: `clang-format -i` (Google style, 100 columns). CI rejects + unformatted code; enable format-on-save in your editor + (`.vscode/settings.json` is checked in). +- Lint: `clang-tidy -p ` with the checked-in `.clang-tidy`. diff --git a/bench/.clang-tidy b/bench/.clang-tidy new file mode 100644 index 0000000..3916133 --- /dev/null +++ b/bench/.clang-tidy @@ -0,0 +1,5 @@ +--- +InheritParentConfig: true +# The canonical Google Benchmark loop `for (auto _ : state)` trips DeadStores +# in every benchmark file; suppress it here instead of per-line NOLINTs. +Checks: '-clang-analyzer-deadcode.DeadStores' diff --git a/bench/CMakeLists.txt b/bench/CMakeLists.txt new file mode 100644 index 0000000..c3eb8ba --- /dev/null +++ b/bench/CMakeLists.txt @@ -0,0 +1,2 @@ +add_executable(queue_bench queue_bench.cpp) +target_link_libraries(queue_bench PRIVATE cq::cq cq_warnings benchmark::benchmark_main) diff --git a/bench/queue_bench.cpp b/bench/queue_bench.cpp new file mode 100644 index 0000000..34a798d --- /dev/null +++ b/bench/queue_bench.cpp @@ -0,0 +1,84 @@ +// Throughput benchmarks for cq::MutexQueue (v1 baseline). +// +// Each benchmark uses Google Benchmark's multi-thread support: the first half +// of the threads produce, the second half consume. Reported items/s is the +// end-to-end transfer rate through the queue. +// +// Run: ./queue_bench --benchmark_repetitions=10 + +#include + +#include +#include +#include + +#include + +namespace { + +constexpr std::size_t kCapacity = 1024; +constexpr std::int64_t kItemsPerThreadPair = 100'000; +constexpr int kSpscThreads = 2; +constexpr int kMpmcThreads = 8; + +// Created/destroyed by the Setup/Teardown hooks below, which run once per +// repetition outside the threaded region. +std::unique_ptr> shared_queue; + +void setup_queue(const benchmark::State& /*state*/) { + shared_queue = std::make_unique>(kCapacity); +} +void teardown_queue(const benchmark::State& /*state*/) { shared_queue.reset(); } + +// state.threads() is 2 * pairs: thread_index [0, pairs) produce, the rest consume. +void BM_MutexQueueThroughput(benchmark::State& state) { + const int pairs = state.threads() / 2; + const bool is_producer = state.thread_index() < pairs; + + for (auto _ : state) { + if (is_producer) { + for (std::int64_t i = 0; i < kItemsPerThreadPair; ++i) { + benchmark::DoNotOptimize(shared_queue->push(static_cast(i))); + } + } else { + std::uint64_t value = 0; + for (std::int64_t i = 0; i < kItemsPerThreadPair; ++i) { + benchmark::DoNotOptimize(shared_queue->pop(value)); + } + } + } + + // Count the producer side only: Google Benchmark sums the counter across + // threads, and each item passes through one producer and one consumer. + if (is_producer) { + state.SetItemsProcessed(state.iterations() * kItemsPerThreadPair); + } +} + +// SPSC: 1 producer + 1 consumer; MPMC: 4 + 4. +BENCHMARK(BM_MutexQueueThroughput) + ->Setup(setup_queue) + ->Teardown(teardown_queue) + ->Threads(kSpscThreads) + ->UseRealTime() + ->Name("MutexQueue/SPSC"); +BENCHMARK(BM_MutexQueueThroughput) + ->Setup(setup_queue) + ->Teardown(teardown_queue) + ->Threads(kMpmcThreads) + ->UseRealTime() + ->Name("MutexQueue/MPMC_4p4c"); + +// Uncontended single-thread round trip: the queue's raw locked cost. +void BM_MutexQueuePushPopSingleThread(benchmark::State& state) { + cq::MutexQueue queue(kCapacity); + std::uint64_t value = 0; + for (auto _ : state) { + benchmark::DoNotOptimize(queue.push(1)); + benchmark::DoNotOptimize(queue.pop(value)); + } + state.SetItemsProcessed(state.iterations()); +} +BENCHMARK(BM_MutexQueuePushPopSingleThread)->Name("MutexQueue/single_thread_roundtrip"); + +} // namespace diff --git a/tests/.clang-tidy b/tests/.clang-tidy new file mode 100644 index 0000000..872bb8f --- /dev/null +++ b/tests/.clang-tidy @@ -0,0 +1,5 @@ +--- +InheritParentConfig: true +# Test-only relaxations: short names (q, p, c) are idiomatic in tests, and +# GTest's TEST macro expansion inflates cognitive complexity. +Checks: '-readability-identifier-length,-readability-function-cognitive-complexity' diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt new file mode 100644 index 0000000..f7484ed --- /dev/null +++ b/tests/CMakeLists.txt @@ -0,0 +1,7 @@ +add_executable(queue_tests + mutex_queue_test.cpp + stress_test.cpp) +target_link_libraries(queue_tests PRIVATE cq::cq cq_warnings GTest::gtest_main) + +include(GoogleTest) +gtest_discover_tests(queue_tests DISCOVERY_TIMEOUT 60) diff --git a/tests/mutex_queue_test.cpp b/tests/mutex_queue_test.cpp new file mode 100644 index 0000000..adf347b --- /dev/null +++ b/tests/mutex_queue_test.cpp @@ -0,0 +1,154 @@ +// Unit tests for cq::MutexQueue (v1: mutex + condition_variable bounded queue). + +#include + +#include +#include +#include +#include + +#include + +namespace cq { +namespace { + +// Long enough for a spawned thread to reach its blocking call; the tests stay +// correct (just less interesting) if it ever proves too short. +constexpr auto kSettleTime = std::chrono::milliseconds(20); + +TEST(MutexQueue, StartsEmptyWithGivenCapacity) { + const MutexQueue q(4); + EXPECT_EQ(q.capacity(), 4U); + EXPECT_EQ(q.size(), 0U); + EXPECT_FALSE(q.closed()); +} + +TEST(MutexQueue, ZeroCapacityThrows) { EXPECT_THROW(MutexQueue(0), std::invalid_argument); } + +TEST(MutexQueue, PopsInFifoOrder) { + MutexQueue q(4); + ASSERT_TRUE(q.push(1)); + ASSERT_TRUE(q.push(2)); + ASSERT_TRUE(q.push(3)); + + int out = 0; + ASSERT_TRUE(q.pop(out)); + EXPECT_EQ(out, 1); + ASSERT_TRUE(q.pop(out)); + EXPECT_EQ(out, 2); + ASSERT_TRUE(q.pop(out)); + EXPECT_EQ(out, 3); + EXPECT_EQ(q.size(), 0U); +} + +TEST(MutexQueue, WrapsAroundRingBoundary) { + constexpr int kRoundTrips = 10; + MutexQueue q(2); + int out = 0; + for (int i = 0; i < kRoundTrips; ++i) { + ASSERT_TRUE(q.push(i)); + ASSERT_TRUE(q.pop(out)); + EXPECT_EQ(out, i); + } +} + +TEST(MutexQueue, TryPushFailsWhenFull) { + MutexQueue q(2); + EXPECT_TRUE(q.try_push(1)); + EXPECT_TRUE(q.try_push(2)); + EXPECT_FALSE(q.try_push(3)); + EXPECT_EQ(q.size(), 2U); +} + +TEST(MutexQueue, TryPopFailsWhenEmpty) { + MutexQueue q(2); + int out = 0; + EXPECT_FALSE(q.try_pop(out)); +} + +TEST(MutexQueue, SupportsMoveOnlyTypes) { + MutexQueue> q(2); + ASSERT_TRUE(q.push(std::make_unique(42))); + + std::unique_ptr out; + ASSERT_TRUE(q.pop(out)); + ASSERT_NE(out, nullptr); + EXPECT_EQ(*out, 42); +} + +TEST(MutexQueue, PopBlocksUntilPush) { + MutexQueue q(1); + int out = 0; + std::jthread consumer([&] { ASSERT_TRUE(q.pop(out)); }); + // Give the consumer a moment to block on the empty queue. + std::this_thread::sleep_for(kSettleTime); + ASSERT_TRUE(q.push(7)); + consumer.join(); + EXPECT_EQ(out, 7); +} + +TEST(MutexQueue, PushBlocksUntilPopWhenFull) { + MutexQueue q(1); + ASSERT_TRUE(q.push(1)); + bool pushed = false; + std::jthread producer([&] { pushed = q.push(2); }); + std::this_thread::sleep_for(kSettleTime); + + int out = 0; + ASSERT_TRUE(q.pop(out)); + EXPECT_EQ(out, 1); + producer.join(); + EXPECT_TRUE(pushed); + ASSERT_TRUE(q.pop(out)); + EXPECT_EQ(out, 2); +} + +TEST(MutexQueue, PushAfterCloseFails) { + MutexQueue q(2); + q.close(); + EXPECT_TRUE(q.closed()); + EXPECT_FALSE(q.push(1)); + EXPECT_FALSE(q.try_push(1)); +} + +TEST(MutexQueue, PopDrainsRemainingItemsAfterClose) { + MutexQueue q(4); + ASSERT_TRUE(q.push(1)); + ASSERT_TRUE(q.push(2)); + q.close(); + + int out = 0; + EXPECT_TRUE(q.pop(out)); + EXPECT_EQ(out, 1); + EXPECT_TRUE(q.try_pop(out)); + EXPECT_EQ(out, 2); + EXPECT_FALSE(q.pop(out)); + EXPECT_FALSE(q.try_pop(out)); +} + +TEST(MutexQueue, CloseWakesBlockedPop) { + MutexQueue q(1); + bool popped = true; + std::jthread consumer([&] { + int out = 0; + popped = q.pop(out); + }); + std::this_thread::sleep_for(kSettleTime); + q.close(); + consumer.join(); + EXPECT_FALSE(popped); +} + +TEST(MutexQueue, CloseWakesBlockedPush) { + MutexQueue q(1); + ASSERT_TRUE(q.push(1)); + bool pushed = true; + std::jthread producer([&] { pushed = q.push(2); }); + std::this_thread::sleep_for(kSettleTime); + q.close(); + producer.join(); + EXPECT_FALSE(pushed); +} + +} // namespace +} // namespace cq diff --git a/tests/stress_test.cpp b/tests/stress_test.cpp new file mode 100644 index 0000000..89b1709 --- /dev/null +++ b/tests/stress_test.cpp @@ -0,0 +1,73 @@ +// Checksum stress test (see README correctness policy): N producers push known +// values, consumers' totals must reconcile. Run under ThreadSanitizer. + +#include + +#include +#include +#include +#include + +#include + +namespace cq { +namespace { + +TEST(MutexQueueStress, ChecksumReconcilesAcrossProducersAndConsumers) { + constexpr int kProducers = 4; + constexpr int kConsumers = 4; + constexpr int kItemsPerProducer = 25'000; + constexpr std::uint64_t kTotalItems = static_cast(kProducers) * kItemsPerProducer; + // Much smaller than the item count so the ring wraps and fills constantly. + constexpr std::size_t kQueueCapacity = 64; + + MutexQueue q(kQueueCapacity); + + std::vector producers; + producers.reserve(kProducers); + for (int p = 0; p < kProducers; ++p) { + producers.emplace_back([&q, p] { + for (int i = 0; i < kItemsPerProducer; ++i) { + const auto value = + (static_cast(p) * kItemsPerProducer) + static_cast(i) + 1; + ASSERT_TRUE(q.push(value)); + } + }); + } + + std::atomic consumed_sum{0}; + std::atomic consumed_count{0}; + std::vector consumers; + consumers.reserve(kConsumers); + for (int c = 0; c < kConsumers; ++c) { + consumers.emplace_back([&] { + std::uint64_t local_sum = 0; + std::uint64_t local_count = 0; + std::uint64_t value = 0; + while (q.pop(value)) { + local_sum += value; + ++local_count; + } + consumed_sum.fetch_add(local_sum, std::memory_order_relaxed); + consumed_count.fetch_add(local_count, std::memory_order_relaxed); + }); + } + + for (auto& t : producers) { + t.join(); + } + q.close(); + for (auto& t : consumers) { + t.join(); + } + + // Sum of 1..kTotalItems: each producer p pushes the contiguous block + // [p*kItems+1, (p+1)*kItems]. + const std::uint64_t expected_sum = kTotalItems * (kTotalItems + 1) / 2; + EXPECT_EQ(consumed_count.load(), kTotalItems); + EXPECT_EQ(consumed_sum.load(), expected_sum); + EXPECT_EQ(q.size(), 0U); +} + +} // namespace +} // namespace cq From bd8f9a90e0fa515d7613bee618e1d0c7b2803937 Mon Sep 17 00:00:00 2001 From: Debra Date: Sun, 16 Aug 2026 16:03:25 +0800 Subject: [PATCH 2/6] refactor: apply review cleanups across tooling, tests, and benchmarks Lint configs: - Move bench-only exceptions (^_$ identifier, BM_ naming) from root .clang-tidy into bench/.clang-tidy so the root config only describes library style - Replace tests' blanket cognitive-complexity disable with the check's IgnoreMacros option at root - Drop PointerAlignment: Left (restates the Google-style default) CMake: - Export compile_commands.json from every build tree - Put TSan flags on a cq_sanitizers INTERFACE target (guarded per compiler) instead of global add_compile_options, so googletest is no longer instrumented - Flatten the benchmark condition; register the benchmark smoke run as a CTest test; gtest discovery moved to PRE_TEST CI: - Discover lint targets via git ls-files instead of hardcoded dirs - Pin clang-format/clang-tidy to version 18; run tidy in parallel - Bound build parallelism; cancel superseded runs; smoke-run benchmarks via ctest instead of a hardcoded binary path Tests/bench: - run_blocked() helper replaces four copies of the spawn/settle/unblock protocol; spawn_threads() collapses the stress test's thread teams - Benchmark queue held in std::optional (no heap alloc), queue ref hoisted out of the measured loop, missing-Setup guarded with SkipWithError, single registration with chained thread counts Docs: STYLE.md points at configs instead of restating their values; drop the default-restating C_Cpp.formatting setting. Co-Authored-By: Claude Fable 5 --- .clang-format | 1 - .clang-tidy | 7 +++-- .github/workflows/ci.yml | 22 +++++++++++----- .vscode/settings.json | 3 +-- CMakeLists.txt | 38 +++++++++++++++------------ STYLE.md | 9 ++++--- bench/.clang-tidy | 5 ++++ bench/CMakeLists.txt | 4 +++ bench/queue_bench.cpp | 31 +++++++++++----------- tests/.clang-tidy | 5 ++-- tests/CMakeLists.txt | 6 +++-- tests/mutex_queue_test.cpp | 51 ++++++++++++++++------------------- tests/stress_test.cpp | 54 ++++++++++++++++++++------------------ 13 files changed, 126 insertions(+), 110 deletions(-) diff --git a/.clang-format b/.clang-format index 50c4175..dc60841 100644 --- a/.clang-format +++ b/.clang-format @@ -1,5 +1,4 @@ BasedOnStyle: Google ColumnLimit: 100 DerivePointerAlignment: false -PointerAlignment: Left IncludeBlocks: Preserve diff --git a/.clang-tidy b/.clang-tidy index 339fe40..28b0d3a 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -16,15 +16,14 @@ Checks: > WarningsAsErrors: '*' HeaderFilterRegex: 'include/cq/.*' CheckOptions: - # Allow the `for (auto _ : state)` benchmark idiom. - readability-identifier-length.IgnoredVariableNames: '^_$' + # Complexity contributed by macro expansion (GTest asserts, etc.) is not + # the author's complexity. + readability-function-cognitive-complexity.IgnoreMacros: 'true' readability-identifier-naming.ClassCase: CamelCase readability-identifier-naming.StructCase: CamelCase readability-identifier-naming.EnumCase: CamelCase readability-identifier-naming.TypeAliasCase: CamelCase readability-identifier-naming.FunctionCase: lower_case - # Google Benchmark's BM_PascalCase convention. - readability-identifier-naming.FunctionIgnoredRegexp: '^BM_.*' readability-identifier-naming.VariableCase: lower_case readability-identifier-naming.ParameterCase: lower_case readability-identifier-naming.PrivateMemberSuffix: '_' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e6c2d95..da4eaa4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,6 +5,10 @@ on: branches: [main] pull_request: +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: test-tsan: name: Tests (ThreadSanitizer, ${{ matrix.os }}) @@ -18,7 +22,7 @@ jobs: - name: Configure run: cmake -B build-tsan -DENABLE_TSAN=ON -DCMAKE_BUILD_TYPE=Debug - name: Build - run: cmake --build build-tsan -j + run: cmake --build build-tsan -j "$(getconf _NPROCESSORS_ONLN)" - name: Test run: ctest --test-dir build-tsan --output-on-failure -j "$(getconf _NPROCESSORS_ONLN)" @@ -30,19 +34,23 @@ jobs: - name: Configure run: cmake -B build-rel -DCMAKE_BUILD_TYPE=Release -DCQ_BUILD_TESTS=OFF - name: Build - run: cmake --build build-rel -j + run: cmake --build build-rel -j "$(getconf _NPROCESSORS_ONLN)" - name: Smoke-run benchmarks - run: ./build-rel/bench/queue_bench --benchmark_min_time=0.01s + run: ctest --test-dir build-rel --output-on-failure lint: name: clang-format & clang-tidy runs-on: ubuntu-latest + env: + # Pinned so a runner-image rollover can't change the formatting + # contract under us; bump deliberately. + CLANG_VERSION: 18 steps: - uses: actions/checkout@v4 - # clang-format and clang-tidy ship preinstalled on ubuntu-latest. - name: clang-format - run: find include tests bench \( -name '*.hpp' -o -name '*.ipp' -o -name '*.cpp' \) | xargs clang-format --dry-run --Werror + run: git ls-files '*.hpp' '*.ipp' '*.cpp' | xargs "clang-format-$CLANG_VERSION" --dry-run --Werror - name: clang-tidy run: | - cmake -B build-lint -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXPORT_COMPILE_COMMANDS=ON - find tests bench -name '*.cpp' | xargs clang-tidy -p build-lint + cmake -B build-lint + git ls-files 'tests/*.cpp' 'bench/*.cpp' | + xargs -P "$(getconf _NPROCESSORS_ONLN)" -n1 "clang-tidy-$CLANG_VERSION" -p build-lint diff --git a/.vscode/settings.json b/.vscode/settings.json index d0ace93..90930fb 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -7,6 +7,5 @@ // Works with either the clangd extension or Microsoft's C/C++ extension. "[cpp]": { "editor.formatOnSave": true - }, - "C_Cpp.formatting": "clangFormat" + } } diff --git a/CMakeLists.txt b/CMakeLists.txt index add5cd3..d876fb4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,6 +9,9 @@ option(ENABLE_TSAN "Build with ThreadSanitizer" OFF) option(CQ_BUILD_TESTS "Build unit and stress tests" ON) option(CQ_BUILD_BENCHMARKS "Build Google Benchmark targets" ON) +# Every build tree gets a compile_commands.json for clangd/clang-tidy. +set(CMAKE_EXPORT_COMPILE_COMMANDS ON) + # Header-only library; consumers inherit C++20 via the compile feature. add_library(cq INTERFACE) add_library(cq::cq ALIAS cq) @@ -22,15 +25,20 @@ target_compile_options(cq_warnings INTERFACE $<$:-Wdocumentation> $<$:/W4>) +# Applied only to our own targets: instrumenting the FetchContent deps just +# slows the build, and TSan still intercepts their pthread-level sync. +add_library(cq_sanitizers INTERFACE) if(ENABLE_TSAN) - add_compile_options(-fsanitize=thread -g -fno-omit-frame-pointer) - add_link_options(-fsanitize=thread) + target_compile_options(cq_sanitizers INTERFACE + $<$:-fsanitize=thread -fno-omit-frame-pointer>) + target_link_options(cq_sanitizers INTERFACE + $<$:-fsanitize=thread>) endif() include(FetchContent) +enable_testing() if(CQ_BUILD_TESTS) - enable_testing() FetchContent_Declare( googletest URL https://github.com/google/googletest/archive/refs/tags/v1.15.2.tar.gz @@ -42,18 +50,14 @@ if(CQ_BUILD_TESTS) add_subdirectory(tests) endif() -if(CQ_BUILD_BENCHMARKS) - if(ENABLE_TSAN) - message(STATUS "ThreadSanitizer build: skipping benchmarks") - else() - FetchContent_Declare( - benchmark - URL https://github.com/google/benchmark/archive/refs/tags/v1.9.1.tar.gz - URL_HASH SHA256=32131c08ee31eeff2c8968d7e874f3cb648034377dfc32a4c377fa8796d84981 - DOWNLOAD_EXTRACT_TIMESTAMP TRUE) - set(BENCHMARK_ENABLE_TESTING OFF CACHE BOOL "" FORCE) - set(BENCHMARK_ENABLE_INSTALL OFF CACHE BOOL "" FORCE) - FetchContent_MakeAvailable(benchmark) - add_subdirectory(bench) - endif() +if(CQ_BUILD_BENCHMARKS AND NOT ENABLE_TSAN) + FetchContent_Declare( + benchmark + URL https://github.com/google/benchmark/archive/refs/tags/v1.9.1.tar.gz + URL_HASH SHA256=32131c08ee31eeff2c8968d7e874f3cb648034377dfc32a4c377fa8796d84981 + DOWNLOAD_EXTRACT_TIMESTAMP TRUE) + set(BENCHMARK_ENABLE_TESTING OFF CACHE BOOL "" FORCE) + set(BENCHMARK_ENABLE_INSTALL OFF CACHE BOOL "" FORCE) + FetchContent_MakeAvailable(benchmark) + add_subdirectory(bench) endif() diff --git a/STYLE.md b/STYLE.md index 9acd30c..43c8bc7 100644 --- a/STYLE.md +++ b/STYLE.md @@ -28,7 +28,8 @@ checked in CI). ## Tooling -- Format: `clang-format -i` (Google style, 100 columns). CI rejects - unformatted code; enable format-on-save in your editor - (`.vscode/settings.json` is checked in). -- Lint: `clang-tidy -p ` with the checked-in `.clang-tidy`. +- Format: `clang-format -i` (settings live in `.clang-format`). CI rejects + unformatted code; format-on-save settings are checked in + (`.vscode/settings.json`). +- Lint: `clang-tidy -p ` (settings live in `.clang-tidy`; every + build dir exports `compile_commands.json`). diff --git a/bench/.clang-tidy b/bench/.clang-tidy index 3916133..c33661a 100644 --- a/bench/.clang-tidy +++ b/bench/.clang-tidy @@ -3,3 +3,8 @@ InheritParentConfig: true # The canonical Google Benchmark loop `for (auto _ : state)` trips DeadStores # in every benchmark file; suppress it here instead of per-line NOLINTs. Checks: '-clang-analyzer-deadcode.DeadStores' +CheckOptions: + # Allow the `for (auto _ : state)` benchmark idiom. + readability-identifier-length.IgnoredVariableNames: '^_$' + # Google Benchmark's BM_PascalCase convention. + readability-identifier-naming.FunctionIgnoredRegexp: '^BM_.*' diff --git a/bench/CMakeLists.txt b/bench/CMakeLists.txt index c3eb8ba..3e0d49f 100644 --- a/bench/CMakeLists.txt +++ b/bench/CMakeLists.txt @@ -1,2 +1,6 @@ add_executable(queue_bench queue_bench.cpp) target_link_libraries(queue_bench PRIVATE cq::cq cq_warnings benchmark::benchmark_main) + +# One-iteration smoke run so CI (and `ctest`) picks up every future benchmark +# target without hardcoding binary paths. +add_test(NAME bench_smoke COMMAND queue_bench --benchmark_min_time=1x) diff --git a/bench/queue_bench.cpp b/bench/queue_bench.cpp index 34a798d..7ac75ea 100644 --- a/bench/queue_bench.cpp +++ b/bench/queue_bench.cpp @@ -10,7 +10,7 @@ #include #include -#include +#include #include @@ -18,32 +18,33 @@ namespace { constexpr std::size_t kCapacity = 1024; constexpr std::int64_t kItemsPerThreadPair = 100'000; -constexpr int kSpscThreads = 2; -constexpr int kMpmcThreads = 8; // Created/destroyed by the Setup/Teardown hooks below, which run once per // repetition outside the threaded region. -std::unique_ptr> shared_queue; +std::optional> shared_queue; -void setup_queue(const benchmark::State& /*state*/) { - shared_queue = std::make_unique>(kCapacity); -} +void setup_queue(const benchmark::State& /*state*/) { shared_queue.emplace(kCapacity); } void teardown_queue(const benchmark::State& /*state*/) { shared_queue.reset(); } // state.threads() is 2 * pairs: thread_index [0, pairs) produce, the rest consume. void BM_MutexQueueThroughput(benchmark::State& state) { const int pairs = state.threads() / 2; const bool is_producer = state.thread_index() < pairs; + if (!shared_queue.has_value()) { // a registration forgot its ->Setup hook + state.SkipWithError("shared_queue not initialized"); + return; + } + auto& queue = *shared_queue; // hoisted out of the measured loop for (auto _ : state) { if (is_producer) { for (std::int64_t i = 0; i < kItemsPerThreadPair; ++i) { - benchmark::DoNotOptimize(shared_queue->push(static_cast(i))); + benchmark::DoNotOptimize(queue.push(static_cast(i))); } } else { std::uint64_t value = 0; for (std::int64_t i = 0; i < kItemsPerThreadPair; ++i) { - benchmark::DoNotOptimize(shared_queue->pop(value)); + benchmark::DoNotOptimize(queue.pop(value)); } } } @@ -55,19 +56,17 @@ void BM_MutexQueueThroughput(benchmark::State& state) { } } -// SPSC: 1 producer + 1 consumer; MPMC: 4 + 4. +// SPSC: 1 producer + 1 consumer; MPMC: 4 + 4. Google Benchmark appends the +// /threads:N suffix to the reported name. +constexpr int kSpscThreads = 2; +constexpr int kMpmcThreads = 8; BENCHMARK(BM_MutexQueueThroughput) ->Setup(setup_queue) ->Teardown(teardown_queue) ->Threads(kSpscThreads) - ->UseRealTime() - ->Name("MutexQueue/SPSC"); -BENCHMARK(BM_MutexQueueThroughput) - ->Setup(setup_queue) - ->Teardown(teardown_queue) ->Threads(kMpmcThreads) ->UseRealTime() - ->Name("MutexQueue/MPMC_4p4c"); + ->Name("MutexQueue/throughput"); // Uncontended single-thread round trip: the queue's raw locked cost. void BM_MutexQueuePushPopSingleThread(benchmark::State& state) { diff --git a/tests/.clang-tidy b/tests/.clang-tidy index 872bb8f..f63cba8 100644 --- a/tests/.clang-tidy +++ b/tests/.clang-tidy @@ -1,5 +1,4 @@ --- InheritParentConfig: true -# Test-only relaxations: short names (q, p, c) are idiomatic in tests, and -# GTest's TEST macro expansion inflates cognitive complexity. -Checks: '-readability-identifier-length,-readability-function-cognitive-complexity' +# Test-only relaxation: short names (q, p, c) are idiomatic in tests. +Checks: '-readability-identifier-length' diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f7484ed..4574b88 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,7 +1,9 @@ add_executable(queue_tests mutex_queue_test.cpp stress_test.cpp) -target_link_libraries(queue_tests PRIVATE cq::cq cq_warnings GTest::gtest_main) +target_link_libraries(queue_tests PRIVATE cq::cq cq_warnings cq_sanitizers GTest::gtest_main) include(GoogleTest) -gtest_discover_tests(queue_tests DISCOVERY_TIMEOUT 60) +# PRE_TEST: discover at ctest time instead of running the (TSan-instrumented) +# binary on every build. +gtest_discover_tests(queue_tests DISCOVERY_TIMEOUT 60 DISCOVERY_MODE PRE_TEST) diff --git a/tests/mutex_queue_test.cpp b/tests/mutex_queue_test.cpp index adf347b..b2986c5 100644 --- a/tests/mutex_queue_test.cpp +++ b/tests/mutex_queue_test.cpp @@ -16,6 +16,17 @@ namespace { // correct (just less interesting) if it ever proves too short. constexpr auto kSettleTime = std::chrono::milliseconds(20); +// Runs blocked_op on its own thread, gives it kSettleTime to reach its +// blocking call, runs unblock, and returns blocked_op's result after joining. +bool run_blocked(auto&& blocked_op, auto&& unblock) { + bool result = false; + std::jthread worker([&] { result = blocked_op(); }); + std::this_thread::sleep_for(kSettleTime); + unblock(); + worker.join(); + return result; +} + TEST(MutexQueue, StartsEmptyWithGivenCapacity) { const MutexQueue q(4); EXPECT_EQ(q.capacity(), 4U); @@ -79,26 +90,20 @@ TEST(MutexQueue, SupportsMoveOnlyTypes) { TEST(MutexQueue, PopBlocksUntilPush) { MutexQueue q(1); int out = 0; - std::jthread consumer([&] { ASSERT_TRUE(q.pop(out)); }); - // Give the consumer a moment to block on the empty queue. - std::this_thread::sleep_for(kSettleTime); - ASSERT_TRUE(q.push(7)); - consumer.join(); + EXPECT_TRUE(run_blocked([&] { return q.pop(out); }, // + [&] { EXPECT_TRUE(q.push(7)); })); EXPECT_EQ(out, 7); } TEST(MutexQueue, PushBlocksUntilPopWhenFull) { MutexQueue q(1); ASSERT_TRUE(q.push(1)); - bool pushed = false; - std::jthread producer([&] { pushed = q.push(2); }); - std::this_thread::sleep_for(kSettleTime); - int out = 0; - ASSERT_TRUE(q.pop(out)); - EXPECT_EQ(out, 1); - producer.join(); - EXPECT_TRUE(pushed); + EXPECT_TRUE(run_blocked([&] { return q.push(2); }, + [&] { + EXPECT_TRUE(q.pop(out)); + EXPECT_EQ(out, 1); + })); ASSERT_TRUE(q.pop(out)); EXPECT_EQ(out, 2); } @@ -128,26 +133,16 @@ TEST(MutexQueue, PopDrainsRemainingItemsAfterClose) { TEST(MutexQueue, CloseWakesBlockedPop) { MutexQueue q(1); - bool popped = true; - std::jthread consumer([&] { - int out = 0; - popped = q.pop(out); - }); - std::this_thread::sleep_for(kSettleTime); - q.close(); - consumer.join(); - EXPECT_FALSE(popped); + int out = 0; + EXPECT_FALSE(run_blocked([&] { return q.pop(out); }, // + [&] { q.close(); })); } TEST(MutexQueue, CloseWakesBlockedPush) { MutexQueue q(1); ASSERT_TRUE(q.push(1)); - bool pushed = true; - std::jthread producer([&] { pushed = q.push(2); }); - std::this_thread::sleep_for(kSettleTime); - q.close(); - producer.join(); - EXPECT_FALSE(pushed); + EXPECT_FALSE(run_blocked([&] { return q.push(2); }, // + [&] { q.close(); })); } } // namespace diff --git a/tests/stress_test.cpp b/tests/stress_test.cpp index 89b1709..f9ab82e 100644 --- a/tests/stress_test.cpp +++ b/tests/stress_test.cpp @@ -13,6 +13,16 @@ namespace cq { namespace { +// Launches count threads, each running fn(thread_index). +std::vector spawn_threads(int count, auto fn) { + std::vector threads; + threads.reserve(static_cast(count)); + for (int i = 0; i < count; ++i) { + threads.emplace_back([fn, i] { fn(i); }); + } + return threads; +} + TEST(MutexQueueStress, ChecksumReconcilesAcrossProducersAndConsumers) { constexpr int kProducers = 4; constexpr int kConsumers = 4; @@ -23,35 +33,27 @@ TEST(MutexQueueStress, ChecksumReconcilesAcrossProducersAndConsumers) { MutexQueue q(kQueueCapacity); - std::vector producers; - producers.reserve(kProducers); - for (int p = 0; p < kProducers; ++p) { - producers.emplace_back([&q, p] { - for (int i = 0; i < kItemsPerProducer; ++i) { - const auto value = - (static_cast(p) * kItemsPerProducer) + static_cast(i) + 1; - ASSERT_TRUE(q.push(value)); - } - }); - } + auto producers = spawn_threads(kProducers, [&q](int p) { + for (int i = 0; i < kItemsPerProducer; ++i) { + const auto value = + (static_cast(p) * kItemsPerProducer) + static_cast(i) + 1; + ASSERT_TRUE(q.push(value)); + } + }); std::atomic consumed_sum{0}; std::atomic consumed_count{0}; - std::vector consumers; - consumers.reserve(kConsumers); - for (int c = 0; c < kConsumers; ++c) { - consumers.emplace_back([&] { - std::uint64_t local_sum = 0; - std::uint64_t local_count = 0; - std::uint64_t value = 0; - while (q.pop(value)) { - local_sum += value; - ++local_count; - } - consumed_sum.fetch_add(local_sum, std::memory_order_relaxed); - consumed_count.fetch_add(local_count, std::memory_order_relaxed); - }); - } + auto consumers = spawn_threads(kConsumers, [&](int /*c*/) { + std::uint64_t local_sum = 0; + std::uint64_t local_count = 0; + std::uint64_t value = 0; + while (q.pop(value)) { + local_sum += value; + ++local_count; + } + consumed_sum.fetch_add(local_sum, std::memory_order_relaxed); + consumed_count.fetch_add(local_count, std::memory_order_relaxed); + }); for (auto& t : producers) { t.join(); From 509da7e66ff9c7f164b8a65961a3ee421a50464f Mon Sep 17 00:00:00 2001 From: Debra Date: Sun, 16 Aug 2026 16:12:00 +0800 Subject: [PATCH 3/6] refactor: scope PR to tooling only Move test and benchmark sources out to a follow-up PR alongside the queue implementation they compile against. Build and lint scaffolding now no-ops until those directories land. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 4 +- CMakeLists.txt | 7 +- bench/.clang-tidy | 10 --- bench/CMakeLists.txt | 6 -- bench/queue_bench.cpp | 83 --------------------- tests/.clang-tidy | 4 - tests/CMakeLists.txt | 9 --- tests/mutex_queue_test.cpp | 149 ------------------------------------- tests/stress_test.cpp | 75 ------------------- 9 files changed, 7 insertions(+), 340 deletions(-) delete mode 100644 bench/.clang-tidy delete mode 100644 bench/CMakeLists.txt delete mode 100644 bench/queue_bench.cpp delete mode 100644 tests/.clang-tidy delete mode 100644 tests/CMakeLists.txt delete mode 100644 tests/mutex_queue_test.cpp delete mode 100644 tests/stress_test.cpp diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index da4eaa4..3b5b794 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,9 +48,9 @@ jobs: steps: - uses: actions/checkout@v4 - name: clang-format - run: git ls-files '*.hpp' '*.ipp' '*.cpp' | xargs "clang-format-$CLANG_VERSION" --dry-run --Werror + run: git ls-files '*.hpp' '*.ipp' '*.cpp' | xargs -r "clang-format-$CLANG_VERSION" --dry-run --Werror - name: clang-tidy run: | cmake -B build-lint git ls-files 'tests/*.cpp' 'bench/*.cpp' | - xargs -P "$(getconf _NPROCESSORS_ONLN)" -n1 "clang-tidy-$CLANG_VERSION" -p build-lint + xargs -r -P "$(getconf _NPROCESSORS_ONLN)" -n1 "clang-tidy-$CLANG_VERSION" -p build-lint diff --git a/CMakeLists.txt b/CMakeLists.txt index d876fb4..c2bcc25 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -38,7 +38,9 @@ endif() include(FetchContent) enable_testing() -if(CQ_BUILD_TESTS) +# The tests/ and bench/ sources land in follow-up PRs; each block is a no-op +# until its directory exists so the build scaffolding can merge first. +if(CQ_BUILD_TESTS AND EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/tests/CMakeLists.txt") FetchContent_Declare( googletest URL https://github.com/google/googletest/archive/refs/tags/v1.15.2.tar.gz @@ -50,7 +52,8 @@ if(CQ_BUILD_TESTS) add_subdirectory(tests) endif() -if(CQ_BUILD_BENCHMARKS AND NOT ENABLE_TSAN) +if(CQ_BUILD_BENCHMARKS AND NOT ENABLE_TSAN + AND EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/bench/CMakeLists.txt") FetchContent_Declare( benchmark URL https://github.com/google/benchmark/archive/refs/tags/v1.9.1.tar.gz diff --git a/bench/.clang-tidy b/bench/.clang-tidy deleted file mode 100644 index c33661a..0000000 --- a/bench/.clang-tidy +++ /dev/null @@ -1,10 +0,0 @@ ---- -InheritParentConfig: true -# The canonical Google Benchmark loop `for (auto _ : state)` trips DeadStores -# in every benchmark file; suppress it here instead of per-line NOLINTs. -Checks: '-clang-analyzer-deadcode.DeadStores' -CheckOptions: - # Allow the `for (auto _ : state)` benchmark idiom. - readability-identifier-length.IgnoredVariableNames: '^_$' - # Google Benchmark's BM_PascalCase convention. - readability-identifier-naming.FunctionIgnoredRegexp: '^BM_.*' diff --git a/bench/CMakeLists.txt b/bench/CMakeLists.txt deleted file mode 100644 index 3e0d49f..0000000 --- a/bench/CMakeLists.txt +++ /dev/null @@ -1,6 +0,0 @@ -add_executable(queue_bench queue_bench.cpp) -target_link_libraries(queue_bench PRIVATE cq::cq cq_warnings benchmark::benchmark_main) - -# One-iteration smoke run so CI (and `ctest`) picks up every future benchmark -# target without hardcoding binary paths. -add_test(NAME bench_smoke COMMAND queue_bench --benchmark_min_time=1x) diff --git a/bench/queue_bench.cpp b/bench/queue_bench.cpp deleted file mode 100644 index 7ac75ea..0000000 --- a/bench/queue_bench.cpp +++ /dev/null @@ -1,83 +0,0 @@ -// Throughput benchmarks for cq::MutexQueue (v1 baseline). -// -// Each benchmark uses Google Benchmark's multi-thread support: the first half -// of the threads produce, the second half consume. Reported items/s is the -// end-to-end transfer rate through the queue. -// -// Run: ./queue_bench --benchmark_repetitions=10 - -#include - -#include -#include -#include - -#include - -namespace { - -constexpr std::size_t kCapacity = 1024; -constexpr std::int64_t kItemsPerThreadPair = 100'000; - -// Created/destroyed by the Setup/Teardown hooks below, which run once per -// repetition outside the threaded region. -std::optional> shared_queue; - -void setup_queue(const benchmark::State& /*state*/) { shared_queue.emplace(kCapacity); } -void teardown_queue(const benchmark::State& /*state*/) { shared_queue.reset(); } - -// state.threads() is 2 * pairs: thread_index [0, pairs) produce, the rest consume. -void BM_MutexQueueThroughput(benchmark::State& state) { - const int pairs = state.threads() / 2; - const bool is_producer = state.thread_index() < pairs; - if (!shared_queue.has_value()) { // a registration forgot its ->Setup hook - state.SkipWithError("shared_queue not initialized"); - return; - } - auto& queue = *shared_queue; // hoisted out of the measured loop - - for (auto _ : state) { - if (is_producer) { - for (std::int64_t i = 0; i < kItemsPerThreadPair; ++i) { - benchmark::DoNotOptimize(queue.push(static_cast(i))); - } - } else { - std::uint64_t value = 0; - for (std::int64_t i = 0; i < kItemsPerThreadPair; ++i) { - benchmark::DoNotOptimize(queue.pop(value)); - } - } - } - - // Count the producer side only: Google Benchmark sums the counter across - // threads, and each item passes through one producer and one consumer. - if (is_producer) { - state.SetItemsProcessed(state.iterations() * kItemsPerThreadPair); - } -} - -// SPSC: 1 producer + 1 consumer; MPMC: 4 + 4. Google Benchmark appends the -// /threads:N suffix to the reported name. -constexpr int kSpscThreads = 2; -constexpr int kMpmcThreads = 8; -BENCHMARK(BM_MutexQueueThroughput) - ->Setup(setup_queue) - ->Teardown(teardown_queue) - ->Threads(kSpscThreads) - ->Threads(kMpmcThreads) - ->UseRealTime() - ->Name("MutexQueue/throughput"); - -// Uncontended single-thread round trip: the queue's raw locked cost. -void BM_MutexQueuePushPopSingleThread(benchmark::State& state) { - cq::MutexQueue queue(kCapacity); - std::uint64_t value = 0; - for (auto _ : state) { - benchmark::DoNotOptimize(queue.push(1)); - benchmark::DoNotOptimize(queue.pop(value)); - } - state.SetItemsProcessed(state.iterations()); -} -BENCHMARK(BM_MutexQueuePushPopSingleThread)->Name("MutexQueue/single_thread_roundtrip"); - -} // namespace diff --git a/tests/.clang-tidy b/tests/.clang-tidy deleted file mode 100644 index f63cba8..0000000 --- a/tests/.clang-tidy +++ /dev/null @@ -1,4 +0,0 @@ ---- -InheritParentConfig: true -# Test-only relaxation: short names (q, p, c) are idiomatic in tests. -Checks: '-readability-identifier-length' diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt deleted file mode 100644 index 4574b88..0000000 --- a/tests/CMakeLists.txt +++ /dev/null @@ -1,9 +0,0 @@ -add_executable(queue_tests - mutex_queue_test.cpp - stress_test.cpp) -target_link_libraries(queue_tests PRIVATE cq::cq cq_warnings cq_sanitizers GTest::gtest_main) - -include(GoogleTest) -# PRE_TEST: discover at ctest time instead of running the (TSan-instrumented) -# binary on every build. -gtest_discover_tests(queue_tests DISCOVERY_TIMEOUT 60 DISCOVERY_MODE PRE_TEST) diff --git a/tests/mutex_queue_test.cpp b/tests/mutex_queue_test.cpp deleted file mode 100644 index b2986c5..0000000 --- a/tests/mutex_queue_test.cpp +++ /dev/null @@ -1,149 +0,0 @@ -// Unit tests for cq::MutexQueue (v1: mutex + condition_variable bounded queue). - -#include - -#include -#include -#include -#include - -#include - -namespace cq { -namespace { - -// Long enough for a spawned thread to reach its blocking call; the tests stay -// correct (just less interesting) if it ever proves too short. -constexpr auto kSettleTime = std::chrono::milliseconds(20); - -// Runs blocked_op on its own thread, gives it kSettleTime to reach its -// blocking call, runs unblock, and returns blocked_op's result after joining. -bool run_blocked(auto&& blocked_op, auto&& unblock) { - bool result = false; - std::jthread worker([&] { result = blocked_op(); }); - std::this_thread::sleep_for(kSettleTime); - unblock(); - worker.join(); - return result; -} - -TEST(MutexQueue, StartsEmptyWithGivenCapacity) { - const MutexQueue q(4); - EXPECT_EQ(q.capacity(), 4U); - EXPECT_EQ(q.size(), 0U); - EXPECT_FALSE(q.closed()); -} - -TEST(MutexQueue, ZeroCapacityThrows) { EXPECT_THROW(MutexQueue(0), std::invalid_argument); } - -TEST(MutexQueue, PopsInFifoOrder) { - MutexQueue q(4); - ASSERT_TRUE(q.push(1)); - ASSERT_TRUE(q.push(2)); - ASSERT_TRUE(q.push(3)); - - int out = 0; - ASSERT_TRUE(q.pop(out)); - EXPECT_EQ(out, 1); - ASSERT_TRUE(q.pop(out)); - EXPECT_EQ(out, 2); - ASSERT_TRUE(q.pop(out)); - EXPECT_EQ(out, 3); - EXPECT_EQ(q.size(), 0U); -} - -TEST(MutexQueue, WrapsAroundRingBoundary) { - constexpr int kRoundTrips = 10; - MutexQueue q(2); - int out = 0; - for (int i = 0; i < kRoundTrips; ++i) { - ASSERT_TRUE(q.push(i)); - ASSERT_TRUE(q.pop(out)); - EXPECT_EQ(out, i); - } -} - -TEST(MutexQueue, TryPushFailsWhenFull) { - MutexQueue q(2); - EXPECT_TRUE(q.try_push(1)); - EXPECT_TRUE(q.try_push(2)); - EXPECT_FALSE(q.try_push(3)); - EXPECT_EQ(q.size(), 2U); -} - -TEST(MutexQueue, TryPopFailsWhenEmpty) { - MutexQueue q(2); - int out = 0; - EXPECT_FALSE(q.try_pop(out)); -} - -TEST(MutexQueue, SupportsMoveOnlyTypes) { - MutexQueue> q(2); - ASSERT_TRUE(q.push(std::make_unique(42))); - - std::unique_ptr out; - ASSERT_TRUE(q.pop(out)); - ASSERT_NE(out, nullptr); - EXPECT_EQ(*out, 42); -} - -TEST(MutexQueue, PopBlocksUntilPush) { - MutexQueue q(1); - int out = 0; - EXPECT_TRUE(run_blocked([&] { return q.pop(out); }, // - [&] { EXPECT_TRUE(q.push(7)); })); - EXPECT_EQ(out, 7); -} - -TEST(MutexQueue, PushBlocksUntilPopWhenFull) { - MutexQueue q(1); - ASSERT_TRUE(q.push(1)); - int out = 0; - EXPECT_TRUE(run_blocked([&] { return q.push(2); }, - [&] { - EXPECT_TRUE(q.pop(out)); - EXPECT_EQ(out, 1); - })); - ASSERT_TRUE(q.pop(out)); - EXPECT_EQ(out, 2); -} - -TEST(MutexQueue, PushAfterCloseFails) { - MutexQueue q(2); - q.close(); - EXPECT_TRUE(q.closed()); - EXPECT_FALSE(q.push(1)); - EXPECT_FALSE(q.try_push(1)); -} - -TEST(MutexQueue, PopDrainsRemainingItemsAfterClose) { - MutexQueue q(4); - ASSERT_TRUE(q.push(1)); - ASSERT_TRUE(q.push(2)); - q.close(); - - int out = 0; - EXPECT_TRUE(q.pop(out)); - EXPECT_EQ(out, 1); - EXPECT_TRUE(q.try_pop(out)); - EXPECT_EQ(out, 2); - EXPECT_FALSE(q.pop(out)); - EXPECT_FALSE(q.try_pop(out)); -} - -TEST(MutexQueue, CloseWakesBlockedPop) { - MutexQueue q(1); - int out = 0; - EXPECT_FALSE(run_blocked([&] { return q.pop(out); }, // - [&] { q.close(); })); -} - -TEST(MutexQueue, CloseWakesBlockedPush) { - MutexQueue q(1); - ASSERT_TRUE(q.push(1)); - EXPECT_FALSE(run_blocked([&] { return q.push(2); }, // - [&] { q.close(); })); -} - -} // namespace -} // namespace cq diff --git a/tests/stress_test.cpp b/tests/stress_test.cpp deleted file mode 100644 index f9ab82e..0000000 --- a/tests/stress_test.cpp +++ /dev/null @@ -1,75 +0,0 @@ -// Checksum stress test (see README correctness policy): N producers push known -// values, consumers' totals must reconcile. Run under ThreadSanitizer. - -#include - -#include -#include -#include -#include - -#include - -namespace cq { -namespace { - -// Launches count threads, each running fn(thread_index). -std::vector spawn_threads(int count, auto fn) { - std::vector threads; - threads.reserve(static_cast(count)); - for (int i = 0; i < count; ++i) { - threads.emplace_back([fn, i] { fn(i); }); - } - return threads; -} - -TEST(MutexQueueStress, ChecksumReconcilesAcrossProducersAndConsumers) { - constexpr int kProducers = 4; - constexpr int kConsumers = 4; - constexpr int kItemsPerProducer = 25'000; - constexpr std::uint64_t kTotalItems = static_cast(kProducers) * kItemsPerProducer; - // Much smaller than the item count so the ring wraps and fills constantly. - constexpr std::size_t kQueueCapacity = 64; - - MutexQueue q(kQueueCapacity); - - auto producers = spawn_threads(kProducers, [&q](int p) { - for (int i = 0; i < kItemsPerProducer; ++i) { - const auto value = - (static_cast(p) * kItemsPerProducer) + static_cast(i) + 1; - ASSERT_TRUE(q.push(value)); - } - }); - - std::atomic consumed_sum{0}; - std::atomic consumed_count{0}; - auto consumers = spawn_threads(kConsumers, [&](int /*c*/) { - std::uint64_t local_sum = 0; - std::uint64_t local_count = 0; - std::uint64_t value = 0; - while (q.pop(value)) { - local_sum += value; - ++local_count; - } - consumed_sum.fetch_add(local_sum, std::memory_order_relaxed); - consumed_count.fetch_add(local_count, std::memory_order_relaxed); - }); - - for (auto& t : producers) { - t.join(); - } - q.close(); - for (auto& t : consumers) { - t.join(); - } - - // Sum of 1..kTotalItems: each producer p pushes the contiguous block - // [p*kItems+1, (p+1)*kItems]. - const std::uint64_t expected_sum = kTotalItems * (kTotalItems + 1) / 2; - EXPECT_EQ(consumed_count.load(), kTotalItems); - EXPECT_EQ(consumed_sum.load(), expected_sum); - EXPECT_EQ(q.size(), 0U); -} - -} // namespace -} // namespace cq From 1b7d251ff67307601d12342c0d6f659983fe2a37 Mon Sep 17 00:00:00 2001 From: Debra Date: Sun, 16 Aug 2026 16:20:19 +0800 Subject: [PATCH 4/6] refactor: apply simplify-review findings to tooling - one parallelism knob (CMAKE_BUILD_PARALLEL_LEVEL / CTEST_PARALLEL_LEVEL) instead of four getconf incantations - lint all tracked *.cpp instead of hardcoded tests/bench globs, so tidy coverage tracks the build automatically - hoist the 'auto _' placeholder carve-out to the root .clang-tidy - state the pinned clang-format major version in STYLE.md Co-Authored-By: Claude Fable 5 --- .clang-tidy | 2 ++ .github/workflows/ci.yml | 16 +++++++++++----- STYLE.md | 3 ++- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 28b0d3a..46f14a6 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -19,6 +19,8 @@ CheckOptions: # Complexity contributed by macro expansion (GTest asserts, etc.) is not # the author's complexity. readability-function-cognitive-complexity.IgnoreMacros: 'true' + # `auto _` is the general placeholder idiom (a language feature in C++26). + readability-identifier-length.IgnoredVariableNames: '^_$' readability-identifier-naming.ClassCase: CamelCase readability-identifier-naming.StructCase: CamelCase readability-identifier-naming.EnumCase: CamelCase diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3b5b794..3ffddae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,6 +9,12 @@ concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true +env: + # Single parallelism knob: cmake --build and ctest read these from the + # environment, and the lint job reuses it for xargs -P. + CMAKE_BUILD_PARALLEL_LEVEL: 4 + CTEST_PARALLEL_LEVEL: 4 + jobs: test-tsan: name: Tests (ThreadSanitizer, ${{ matrix.os }}) @@ -22,9 +28,9 @@ jobs: - name: Configure run: cmake -B build-tsan -DENABLE_TSAN=ON -DCMAKE_BUILD_TYPE=Debug - name: Build - run: cmake --build build-tsan -j "$(getconf _NPROCESSORS_ONLN)" + run: cmake --build build-tsan - name: Test - run: ctest --test-dir build-tsan --output-on-failure -j "$(getconf _NPROCESSORS_ONLN)" + run: ctest --test-dir build-tsan --output-on-failure bench-build: name: Benchmarks compile (Release) @@ -34,7 +40,7 @@ jobs: - name: Configure run: cmake -B build-rel -DCMAKE_BUILD_TYPE=Release -DCQ_BUILD_TESTS=OFF - name: Build - run: cmake --build build-rel -j "$(getconf _NPROCESSORS_ONLN)" + run: cmake --build build-rel - name: Smoke-run benchmarks run: ctest --test-dir build-rel --output-on-failure @@ -52,5 +58,5 @@ jobs: - name: clang-tidy run: | cmake -B build-lint - git ls-files 'tests/*.cpp' 'bench/*.cpp' | - xargs -r -P "$(getconf _NPROCESSORS_ONLN)" -n1 "clang-tidy-$CLANG_VERSION" -p build-lint + git ls-files '*.cpp' | + xargs -r -P "$CMAKE_BUILD_PARALLEL_LEVEL" -n1 "clang-tidy-$CLANG_VERSION" -p build-lint diff --git a/STYLE.md b/STYLE.md index 43c8bc7..6df0b48 100644 --- a/STYLE.md +++ b/STYLE.md @@ -30,6 +30,7 @@ checked in CI). - Format: `clang-format -i` (settings live in `.clang-format`). CI rejects unformatted code; format-on-save settings are checked in - (`.vscode/settings.json`). + (`.vscode/settings.json`). CI pins clang-format/clang-tidy **18** — use the + same major version locally or formatting may not match. - Lint: `clang-tidy -p ` (settings live in `.clang-tidy`; every build dir exports `compile_commands.json`). From 70a85b1b5ea6ab9e721b1b6734fd9187f9358182 Mon Sep 17 00:00:00 2001 From: Debra Date: Sun, 16 Aug 2026 16:31:32 +0800 Subject: [PATCH 5/6] chore: job name reflects that benchmarks are also smoke-run Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3ffddae..6272268 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,7 +33,7 @@ jobs: run: ctest --test-dir build-tsan --output-on-failure bench-build: - name: Benchmarks compile (Release) + name: Benchmarks (Release, smoke-run) runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 From 6420b49c259ff2ebf51ae17fa1f54bea9f719c90 Mon Sep 17 00:00:00 2001 From: Debra Date: Sun, 16 Aug 2026 16:52:18 +0800 Subject: [PATCH 6/6] ci: fail ctest runs that discover zero tests once sources exist Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6272268..16f60b7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,7 +30,9 @@ jobs: - name: Build run: cmake --build build-tsan - name: Test - run: ctest --test-dir build-tsan --output-on-failure + # --no-tests=error arms itself once tests/ lands; until then the + # scaffolding is allowed to no-op. + run: ctest --test-dir build-tsan --output-on-failure --no-tests="$([ -d tests ] && echo error || echo ignore)" bench-build: name: Benchmarks (Release, smoke-run) @@ -42,7 +44,7 @@ jobs: - name: Build run: cmake --build build-rel - name: Smoke-run benchmarks - run: ctest --test-dir build-rel --output-on-failure + run: ctest --test-dir build-rel --output-on-failure --no-tests="$([ -d bench ] && echo error || echo ignore)" lint: name: clang-format & clang-tidy