|
FhSim
3.1.0
Marine systems simulation
|
| ID | 0009 |
| Class | BUG |
| Severity | 2 |
| Status | blocked |
| Models | — (library API ConstraintSolver/CoreBoundThreadPool; annotated on the fhsim_coribo library group) |
| Found | 2026-09-25, reported downstream from fhsim_marine_elements MARE-0045 (3.1.0 and 3.2.0); line numbers checked at c972717 |
| Decision needed | Model owner: change the one-argument constructor to a pool of size 0 (inline), or remove it so that callers must choose a pool size |
src/ConstraintSolver/ConstraintSolver.cpp:7-11:
The one-argument constructor (include/fhsim_coribo/ConstraintSolver.h:34) builds a core-bound pool with one thread per hardware thread, bound to cores 0, 1, 2, ... A pool of size 0 runs every task inline on the calling thread (src/Utilities/AsynchronousTask.cpp:20-30). This repo's own WinchableCable already passes 0 (src/SimObjects/WinchableCable.cpp:17); the default hits other users of the library.
Measured downstream (fhsim_marine_elements MARE-0045). For the small rigid-body models there (TrawlCable, MooringCable, RundDorg: 30 to 100 bodies) the per-step work is far too small to share, and the pool threads cost more than they save (mostly futex wake-ups). On a 32-thread machine, one simulated second of the TrawlCable test fixture takes 12.4 s with this constructor and 0.22 s with a pool of size 0, a factor of 56; MooringCable 12.8 s against 0.37 s, RundDorg 3.3 s against 0.07 s. The time grows with the pool size (TrawlCable, pool size 0: 0.22 s, 1: 1.9 s, 2: 1.7 s, 4: 2.7 s, 8: 3.7 s, 16: 6.5 s, 32: 12.4 s), so the cost scales with the machine: the same input is slowest on the largest workstation.
A pool of size 1 is not a cheap single thread: it starts one worker, bound to core start_ix, and the calling thread hands it every task and waits. Every pool also binds its threads from core 0 by default, so solvers in parallel processes share the same cores: eight parallel runs of the RundDorg fixture take 3.0 to 3.8 s with a pool of size 1, and 0.08 s with a pool of size 0, whose results are byte-identical to size 1.
With more than one thread the results are also not reproducible between runs (the work order depends on scheduling); with one thread they are byte-identical.
The affinity masks shift past their width (src/Utilities/AsynchronousTask.cpp):
static_cast<size_t>(0x1)<<(i+start_ix) (:15), undefined from core index 64 on; with hyperthread merging it is static_cast<size_t>(0x3)<<(2*(i+start_ix)) (:10), undefined from 32 on;setCpuMask loops i over all 1024 bits of cpu_set_t (:145) testing 0x1<<i & cpu_mask on an int (:146), which is undefined from i = 32 on (i = 31 too before C++20) in every setCpuMask call, for every pool with a worker.A start_ix + pool_size beyond the hardware threads gives threads that cannot be bound.
Small models built with ConstraintSolver(double) run many times slower than inline, more so on machines with more hardware threads, and their results differ from run to run. Parallel processes pile onto the same cores. The shifts are undefined behaviour on every pooled solver.
include/fhsim_coribo/AsynchronousTask.h that size 0 means inline and size 1 means one bound worker, and in include/fhsim_coribo/ConstraintSolver.h that the pool size is a performance setting with a run-to-run reproducibility cost.cpu_set_t operation (for example CPU_SET directly per core index), and check start_ix + pool_size against the hardware threads.fhsim_marine_elements now always passes an explicit pool (CreateConstraintSolver, default one thread, as a pool of size 0).
A unit test that constructs ConstraintSolver(double) and checks that its pool has no worker threads (or, if the constructor is removed, a compile check). For the mask: a test that builds a CoreBoundThreadPool with start_ix near 32 and 64 under UBSan, and checks the resulting affinity with pthread_getaffinity_np.
Changing the default changes run time and, for callers that used more than one thread, the run-to-run variation of their results; it does not change the results of a single-thread run. The mask change is local.