feat(linear-static-3d-euler-beam): step 17 - parallel-for-tbb-review-fix

This commit is contained in:
KOKO\Mimi
2026-08-09 19:06:24 +09:00
parent 5bd0c54a0a
commit 59da6c6b96
2 changed files with 72 additions and 15 deletions
@@ -681,3 +681,38 @@
- handoff: Step 18 can inject either backend for independent element-local - handoff: Step 18 can inject either backend for independent element-local
computation into caller-owned stable element-order slots before its separate computation into caller-owned stable element-order slots before its separate
deterministic COO-to-CSR reduction. deterministic COO-to-CSR reduction.
### Step 17 Review Fix Round 1 — atomic test evidence
- classification: test-evidence reliability hardening; no production defect
was observed or corrected.
- baseline_GREEN: before changing the test,
`ctest --test-dir .harness/build -C Debug -R ParallelFor --output-on-failure`
exited 0 with the existing production adapter passing 3/3. Because the
review changes only how a test observes duplicate callbacks, no
implementation-owned RED was expected or manufactured.
- reliability_issue: the prior `std::vector` scalar writes were race-free only
if the exact-once contract was already true. A duplicate concurrent callback
for one index could write the same non-atomic scalar concurrently, making the
negative case undefined instead of a deterministic test failure.
- correction: T17-PFOR-001 now uses an explicitly initialized atomic zero-count
counter and explicitly initialized per-index atomic visit counters.
T17-PFOR-002 uses explicitly initialized atomic serial/TBB output slots plus
per-index atomic visit counters. All callback increments use
`fetch_add(1, memory_order_relaxed)` and post-execution assertions require
every count to equal one, so duplicate same-index callbacks are observed
without a data race. Output stores/loads are relaxed atomics because only
per-slot observation is required after the blocking adapter returns.
- scope: only `tests/unit/assembly/parallel_for_test.cpp`, this cumulative
Step 17 section, and the ignored `task-17-report.md` changed. Production,
CMake, the phase index, upstream contracts, and reference artifacts were not
modified in this review round; the exact three test names remain unchanged.
| stage | exact command | exit_code | observed_result |
| --- | --- | ---: | --- |
| REVIEW1-baseline-GREEN | `ctest --test-dir .harness/build -C Debug -R ParallelFor --output-on-failure` | 0 | existing correct production passed 3/3 before test hardening; no implementation RED expected |
| REVIEW1-VERIFY-build | `cmake --build .harness/build --config Debug` | 0 | C++17 atomic test evidence compiled and linked under MSVC `/W4 /WX` without a warning |
| REVIEW1-VERIFY-targeted | `ctest --test-dir .harness/build -C Debug -R ParallelFor --output-on-failure` | 0 | hardened exact three ParallelFor tests passed 3/3 |
| REVIEW1-VERIFY-discovery | `ctest --test-dir .harness/build -C Debug --show-only=json-v1` | 0 | 42 tests discovered, including the unchanged three exact ParallelFor names |
| REVIEW1-VERIFY-full | `ctest --test-dir .harness/build -C Debug --output-on-failure` | 0 | full accumulated suite passed 42/42 |
| REVIEW1-VERIFY-scans | public TBB leak, production scope, atomic reliability, reference, and diff/whitespace scans | 0 | public leaks 0; forbidden production scope 0; atomic vectors 5; relaxed duplicate counters 4; legacy non-atomic visit/output paths 0; reference unchanged; diff clean |
+37 -15
View File
@@ -3,6 +3,7 @@
#include <gtest/gtest.h> #include <gtest/gtest.h>
#include <array> #include <array>
#include <atomic>
#include <cstddef> #include <cstddef>
#include <functional> #include <functional>
#include <stdexcept> #include <stdexcept>
@@ -28,43 +29,64 @@ TEST(ParallelFor, ZeroOneManyExecuteExactlyOnce) {
const TbbParallelFor tbb; const TbbParallelFor tbb;
for (const ParallelFor& parallelFor : parallelForBackends(serial, tbb)) { for (const ParallelFor& parallelFor : parallelForBackends(serial, tbb)) {
bool zeroBodyCalled = false; std::atomic<std::size_t> zeroVisits{0U};
parallelFor.execute(0U, [&zeroBodyCalled](std::size_t) { parallelFor.execute(0U, [&zeroVisits](std::size_t) {
zeroBodyCalled = true; zeroVisits.fetch_add(1U, std::memory_order_relaxed);
}); });
EXPECT_FALSE(zeroBodyCalled); EXPECT_EQ(zeroVisits.load(std::memory_order_relaxed), 0U);
for (const std::size_t count : {1U, 257U}) { for (const std::size_t count : {1U, 257U}) {
std::vector<int> visits(count, 0); std::vector<std::atomic<std::size_t>> visits(count);
for (auto& visit : visits) {
visit.store(0U, std::memory_order_relaxed);
}
parallelFor.execute(count, [&visits](std::size_t index) { parallelFor.execute(count, [&visits](std::size_t index) {
++visits[index]; visits[index].fetch_add(1U, std::memory_order_relaxed);
}); });
EXPECT_EQ(visits, std::vector<int>(count, 1)); for (std::size_t index = 0; index < count; ++index) {
EXPECT_EQ(visits[index].load(std::memory_order_relaxed), 1U);
}
} }
} }
} }
TEST(ParallelFor, SerialAndTbbProduceStableIndexedOutput) { TEST(ParallelFor, SerialAndTbbProduceStableIndexedOutput) {
constexpr std::size_t count = 1024U; constexpr std::size_t count = 1024U;
std::vector<std::size_t> serialOutput(count, 0U); std::vector<std::atomic<std::size_t>> serialOutput(count);
std::vector<std::size_t> tbbOutput(count, 0U); std::vector<std::atomic<std::size_t>> tbbOutput(count);
std::vector<std::atomic<std::size_t>> serialVisits(count);
std::vector<std::atomic<std::size_t>> tbbVisits(count);
for (std::size_t index = 0; index < count; ++index) {
serialOutput[index].store(0U, std::memory_order_relaxed);
tbbOutput[index].store(0U, std::memory_order_relaxed);
serialVisits[index].store(0U, std::memory_order_relaxed);
tbbVisits[index].store(0U, std::memory_order_relaxed);
}
const auto valueForIndex = [](std::size_t index) { const auto valueForIndex = [](std::size_t index) {
return (index + 17U) * (index + 3U); return (index + 17U) * (index + 3U);
}; };
const SerialParallelFor serial; const SerialParallelFor serial;
serial.execute(count, [&serialOutput, &valueForIndex](std::size_t index) { serial.execute(count, [&serialOutput, &serialVisits, &valueForIndex](std::size_t index) {
serialOutput[index] = valueForIndex(index); serialOutput[index].store(valueForIndex(index), std::memory_order_relaxed);
serialVisits[index].fetch_add(1U, std::memory_order_relaxed);
}); });
const TbbParallelFor tbb; const TbbParallelFor tbb;
tbb.execute(count, [&tbbOutput, &valueForIndex](std::size_t index) { tbb.execute(count, [&tbbOutput, &tbbVisits, &valueForIndex](std::size_t index) {
tbbOutput[index] = valueForIndex(index); tbbOutput[index].store(valueForIndex(index), std::memory_order_relaxed);
tbbVisits[index].fetch_add(1U, std::memory_order_relaxed);
}); });
EXPECT_EQ(tbbOutput, serialOutput);
for (std::size_t index = 0; index < count; ++index) { for (std::size_t index = 0; index < count; ++index) {
EXPECT_EQ(tbbOutput[index], valueForIndex(index)); EXPECT_EQ(serialVisits[index].load(std::memory_order_relaxed), 1U);
EXPECT_EQ(tbbVisits[index].load(std::memory_order_relaxed), 1U);
EXPECT_EQ(
tbbOutput[index].load(std::memory_order_relaxed),
serialOutput[index].load(std::memory_order_relaxed));
EXPECT_EQ(
tbbOutput[index].load(std::memory_order_relaxed),
valueForIndex(index));
} }
} }