diff --git a/style.md b/style.md index 7bbdbcb..68cc42c 100644 --- a/style.md +++ b/style.md @@ -591,13 +591,24 @@ TEST_CASE("Server accepts connections") { #### Threading Checklist for Tests/Benchmarks -**MANDATORY: Before writing any `std::thread` or `threads.emplace_back()`:** +**Common threading principles (all concurrent code):** +- **Count total threads** - Include main/benchmark thread in count +- **Always assume concurrent execution needed** - Tests/benchmarks require real concurrency +- **Add synchronization primitive** - `std::latch start_latch{N}` (most common), `std::barrier`, or similar where N = total concurrent threads +- **Each thread synchronizes before doing work** - e.g., `start_latch.arrive_and_wait()` or `barrier.arrive_and_wait()` +- **Main thread synchronizes before measurement/execution** - ensures all threads start simultaneously -1. **Count total threads** - Include main/benchmark thread in count -2. **Always assume concurrent execution needed** - Tests/benchmarks require real concurrency -3. **Add `std::latch start_latch{N}`** where N = total concurrent threads -4. **Each thread calls `start_latch.arrive_and_wait()`** before doing work -5. **Main/benchmark thread calls `start_latch.arrive_and_wait()`** before measurement +**Test-specific:** +- **Perform many operations per thread creation** - amortize thread creation cost and increase chances of hitting race conditions +- **Pattern: Create test that spawns threads and runs many operations, then run that test many times** - amortizes thread creation cost while providing fresh test instances +- **Run 100-10000 operations per test, and 100-10000 test iterations** - maximizes chances of hitting race conditions +- **Always run with ThreadSanitizer** - compile with `-fsanitize=thread` + +**Benchmark-specific:** +- **NEVER create threads inside the benchmark measurement** - creates thread creation/destruction overhead, not contention +- **Create background threads OUTSIDE the benchmark** that run continuously during measurement +- **Use `std::atomic keep_running` to cleanly shut down background threads after benchmark** +- **Measure only the foreground operation under real contention from background threads** **Red flags to catch immediately:** - ❌ Creating threads in a loop without `std::latch` @@ -609,18 +620,18 @@ TEST_CASE("Server accepts connections") { ```cpp // BAD: Race likely over before threads start -std::atomic counter{0}; +int counter = 0; for (int i = 0; i < 4; ++i) { threads.emplace_back([&]() { counter++; }); // Probably sequential } // GOOD: Force threads to race simultaneously -std::atomic counter{0}; +int counter = 0; std::latch start_latch{4}; for (int i = 0; i < 4; ++i) { threads.emplace_back([&]() { start_latch.count_down_and_wait(); // All threads start together - counter++; // Now they actually race + counter++; // Now they actually race (data race on non-atomic) }); } ```