Update multi-threaded tests/benchmarks guidance

This commit is contained in:
2025-09-11 12:01:18 -04:00
parent 994e31032f
commit d35a4fa4db

View File

@@ -591,13 +591,24 @@ TEST_CASE("Server accepts connections") {
#### Threading Checklist for Tests/Benchmarks #### 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 **Test-specific:**
2. **Always assume concurrent execution needed** - Tests/benchmarks require real concurrency - **Perform many operations per thread creation** - amortize thread creation cost and increase chances of hitting race conditions
3. **Add `std::latch start_latch{N}`** where N = total concurrent threads - **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
4. **Each thread calls `start_latch.arrive_and_wait()`** before doing work - **Run 100-10000 operations per test, and 100-10000 test iterations** - maximizes chances of hitting race conditions
5. **Main/benchmark thread calls `start_latch.arrive_and_wait()`** before measurement - **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<bool> 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:** **Red flags to catch immediately:**
- ❌ Creating threads in a loop without `std::latch` - ❌ Creating threads in a loop without `std::latch`
@@ -609,18 +620,18 @@ TEST_CASE("Server accepts connections") {
```cpp ```cpp
// BAD: Race likely over before threads start // BAD: Race likely over before threads start
std::atomic<int> counter{0}; int counter = 0;
for (int i = 0; i < 4; ++i) { for (int i = 0; i < 4; ++i) {
threads.emplace_back([&]() { counter++; }); // Probably sequential threads.emplace_back([&]() { counter++; }); // Probably sequential
} }
// GOOD: Force threads to race simultaneously // GOOD: Force threads to race simultaneously
std::atomic<int> counter{0}; int counter = 0;
std::latch start_latch{4}; std::latch start_latch{4};
for (int i = 0; i < 4; ++i) { for (int i = 0; i < 4; ++i) {
threads.emplace_back([&]() { threads.emplace_back([&]() {
start_latch.count_down_and_wait(); // All threads start together 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)
}); });
} }
``` ```