C++Now 2026 audit follow-up: cache-line fix + two spikes (Dist link blocker, static-init survey)

CLAUDE.md

@@ -74,6 +74,7 @@ Read before doing anything non-trivial; do not duplicate their content here:
 - [docs/agent-rules/asset-degradation-and-constructor-preconditions.md](docs/agent-rules/asset-degradation-and-constructor-preconditions.md) — a precondition asserted in a **constructor** delegates safety to every call site and they will disagree (issue #694: `Mesh`'s ctor vs ~30 `Ref<Mesh>::Create` sites); the missing-asset *recovery* path was itself the crash and had never been exercised by a test; relaxing a ctor is only half the change (every accessor on the now-representable invalid object must be total, or the crash just moves one call deeper); "degrades gracefully" means ONE warned-once message, not a per-frame flood; and why a "load the scene, does it crash?" check passes while the bug is fully present — the trigger is *resolution*, not *loading*.
 - [docs/agent-rules/audio-voice-budget.md](docs/agent-rules/audio-voice-budget.md) — the concurrent-voice cap / stealing / virtualization policy (issue #730): the admission point must live *inside* `AudioSource::Play()` / `SoundGraphSound::Play()` because the engine starts sounds from six call sites, not the two a grep suggests; the policy object must stay miniaudio-free or it is untestable (a real `AudioSource` needs a live device); "it's playing again" is the wrong assertion for a resumed loop — assert the **position**, and let the backend cursor win while audible; the promotion margin is what stops two near-equal voices stop/starting every frame; hosts are called with the lock released; a one-shot must auto-retire or it leaks its slot forever; the graph path can only mute (no seek/suspend API) and inverts priority polarity; and the four edits — one of them silent — a new `AudioSourceConfig` field costs.
 - [docs/agent-rules/asset-import-usd-alembic.md](docs/agent-rules/asset-import-usd-alembic.md) — the #655 interchange abstraction (`MeshImporterRegistry`/`MeshExporterRegistry` fronting Assimp/USD/Alembic + glTF export, keyed by extension with an Assimp fallback) and every gotcha vendoring the three heavyweight VFX libs into the static-everything build: **OpenUSD is a prebuilt static-monolithic install (`OLO_WITH_USD` + `OLO_USD_INSTALL_DIR`), NOT FetchContent** — its plugInfo tree is install-produced, needs `/WHOLEARCHIVE:usd_m` + `PlugRegistry::RegisterPlugins` at runtime, a `$<TARGET_PDB_FILE:usd_m>` static-lib patch, oneTBB (`tbb12.lib` auto-link + `Shlwapi`/`Dbghelp`), git long-paths, ~2 GB lib; **Alembic** (FetchContent, Imath-first) needs `IMATH_INSTALL ON` for Alembic's unconditional `export()` and a `$<BUILD_INTERFACE:>`-wrapped `Imath_INCLUDE_DIRS` for `<Imath/half.h>`; **MaterialX** collides with assimp's bundled pugixml → `/FORCE:MULTIPLE` (assimp wins, verified runtime-safe). Plus the per-format silent-correctness traps (winding, up-axis, metersPerUnit, UV origin, facevarying indexing).
+- [docs/agent-rules/spinlock-payload-cache-line-separation.md](docs/agent-rules/spinlock-payload-cache-line-separation.md) — why a lock and its guarded payload must not share a cache line (Pikus, C++Now 2026): an unlocked reader downgrades the owner's line from Exclusive to Shared, costing the owner an invalidate round-trip on its next acquire/release even with no blocking contention. Covers the judgement bar for when adjacency is actually worth padding (is the flag read by threads that skip the lock entirely; is the path contended; is the adjacency provable, not guessed) versus when it isn't (`Core/Ref.cpp`'s registry mutex is the dominant cost, not its cache-line layout — see [docs/agent-rules/intrusive-refcount-weakref-races.md](docs/agent-rules/intrusive-refcount-weakref-races.md)), and the one real fix found (`Task/TaskPrivate.h`'s `FTaskBase::TSubsequents`).
 - [docs/ops/build.md](docs/ops/build.md) — full Windows / Linux / WSL build matrix.
 - [docs/testing.md](docs/testing.md) — opinion document on **why** we test what we test, value heuristic, named anti-patterns, retirement criteria, and how tests are classified. The per-file catalogue tables are **generated, git-ignored** (`docs/test-catalogue.*.md`), not inlined here.
 

OloEngine/src/OloEngine/Task/TaskPrivate.h

@@ -160,7 +160,7 @@
             {
                 if (m_RefCount.fetch_sub(1, std::memory_order_acq_rel) == 1)
                 {
                     delete this;
                 }
             }
 
@@ -1061,12 +1061,12 @@
               public:
                 bool PushIfNotClosed(FTaskBase* NewItem)
                 {
-                    if (m_IsClosed.load(std::memory_order_acquire))
+                    if (m_Close.IsClosed.load(std::memory_order_acquire))
                     {
                         return false;
                     }
-                    TUniqueLock<FMutex> Lock(m_Mutex);
-                    if (m_IsClosed.load(std::memory_order_relaxed))
+                    TUniqueLock<FMutex> Lock(m_Close.Mutex);
+                    if (m_Close.IsClosed.load(std::memory_order_relaxed))
                     {
                         return false;
                     }
@@ -1076,20 +1076,37 @@
 
                 TArray<FTaskBase*, AllocatorType> Close()
                 {
-                    TUniqueLock<FMutex> Lock(m_Mutex);
-                    m_IsClosed.store(true, std::memory_order_release);
+                    TUniqueLock<FMutex> Lock(m_Close.Mutex);
+                    m_Close.IsClosed.store(true, std::memory_order_release);
                     return MoveTemp(m_Subsequents);
                 }
 
                 bool IsClosed() const
                 {
-                    return m_IsClosed.load(std::memory_order_acquire);
+                    return m_Close.IsClosed.load(std::memory_order_acquire);
                 }
 
               private:
                 TArray<FTaskBase*, AllocatorType> m_Subsequents;
-                std::atomic<bool> m_IsClosed{ false };
-                FMutex m_Mutex;
+                // Cache-line separated from m_Subsequents: PushIfNotClosed's unlocked
+                // fast-path read of IsClosed runs on every prerequisite->subsequent
+                // edge added anywhere in the task graph (FTaskBase::AddSubsequent),
+                // often from several worker threads racing to attach themselves to the
+                // same shared prerequisite. Without separation those reads keep
+                // dragging m_Subsequents' inline storage between Exclusive and Shared,
+                // so Close()'s Mutex.Lock() (an Exclusive-owning RMW on the same line)
+                // pays an extra invalidation round-trip it would not otherwise need
+                // (Pikus, "Lock-Free Programming is Dead", C++Now 2026). IsClosed and
+                // Mutex are grouped into one alignas'd struct — rather than alignas on
+                // IsClosed alone — so the pairing (both must share a line WITH each
+                // other, just not with m_Subsequents) survives a future member reorder
+                // or insertion instead of relying on incidental declaration order.
+                struct alignas(OLO_PLATFORM_CACHE_LINE_SIZE) FCloseState
+                {
+                    std::atomic<bool> IsClosed{ false };
+                    FMutex Mutex;
+                };
+                FCloseState m_Close;
             };
 
             TSubsequents<TInlineAllocator<1>> m_Subsequents;

docs/agent-rules/spinlock-payload-cache-line-separation.md

@@ -0,0 +1,120 @@
+# Don't put a lock and its guarded payload on the same cache line
+
+Source: Fedor Pikus, *Lock-Free Programming is Dead* (C++Now 2026), closing slide.
+Written from a repo-wide audit for a C++Now 2026 follow-up branch (issue-free —
+see `HANDOVER.md` history on `feature/cppnow26-audits`).
+
+## The mechanism
+
+A thread that merely *reads* a lock — an unlocked fast-path check, a spin-wait,
+a `TryLock`-and-bail — pulls that cache line into Shared state on its core. If
+the lock and the payload it guards share a line, that read also drags the
+payload along, even though the reader never touched the payload itself. When
+the owner then does a genuinely exclusive operation on the lock (a CAS to
+acquire, a release-store), the CPU has to pay an invalidate round-trip it
+would not otherwise need — the owner's line was Exclusive, a reader's
+unrelated peek downgraded it to Shared, and the owner has to re-acquire
+Exclusive before it can proceed. This costs real cycles even when the lock
+itself is uncontended in the traditional sense (no thread ever blocks on it).
+
+The fix is mechanical once you've found a real instance: pad the lock (or the
+payload) onto its own line with `alignas(OLO_PLATFORM_CACHE_LINE_SIZE)` —
+**use the project's existing constant** (`OloEngine/src/OloEngine/Memory/Platform.h`),
+don't invent a second one. `/wd4324` is already suppressed project-wide
+precisely because this pattern's padding warning is expected, not a defect.
+
+## Judgement: not every atomic-bool-next-to-a-mutex qualifies
+
+The audit that produced this note grepped `Threading/`, `Task/`, `Async/`,
+`Audio/`, `Core/` for `std::atomic<bool>` / `std::atomic_flag` / lock-shaped
+members and found roughly two dozen candidates. Only one was worth fixing.
+The bar that separated it from the rest:
+
+1. **Is the flag actually read by threads that don't also take the lock?**
+   A flag that's always read-then-immediately-locked (double-checked-locking
+   idiom) doesn't get the "spinning reader avoids the lock" benefit from
+   separation — the mutex's own cache-line traffic already dominates. This is
+   why `Core/Ref.cpp`'s `LiveReferencesData::isValid` was **not** padded
+   despite sitting next to `FMutex mutex` and being read unlocked at the top
+   of every `Ref<T>` add/release/lock: every one of those call sites goes on
+   to acquire the lock in the common (non-shutdown) path anyway. See
+   [intrusive-refcount-weakref-races.md](intrusive-refcount-weakref-races.md) —
+   that doc's own conclusion is the more important one here: the *global
+   mutex itself* (one lock serializing
+   every `Ref<T>` op process-wide) is the dominant cost, matching Pikus's
+   framing that "the cost of the lock doesn't matter — disruption of
+   execution flow matters." Cache-line padding is a rounding error next to a
+   structural single-global-lock bottleneck; fix the bigger problem first, if
+   it's ever profiled as hot.
+2. **Is the path actually contended?** A flag touched once at init/shutdown
+   (audio suspend flags polled by a 100µs-sleep teardown loop, async-load
+   "ready" flags checked once per load) isn't worth a padding byte — and
+   padding indiscriminately bloats every instance of a hot struct for no
+   benefit. Several `std::atomic<bool>` candidates in `Audio/SoundGraph/`
+   were rejected on this basis alone.
+3. **Is the adjacency provable, not guessed?** A hand-wavy "these two members
+   are probably on the same line" isn't enough to justify a change — either
+   the containing struct is small enough that adjacency is obvious by
+   inspection (the fix below), or it needs an actual `offsetof`/`sizeof`
+   check before touching it. `Task/WaitingQueue.h`'s `FWaitingQueue` was
+   considered (its `m_State`/`m_StandbyState` CAS traffic is about as hot as
+   this codebase gets, with `m_IsShuttingDown` declared nearby) and rejected
+   specifically because the real layout — depending on `TFunction`'s
+   captured-callback size — couldn't be confirmed without compiling and
+   inspecting it, and a wrong guess is worse than no fix.
+
+## The one fix that cleared the bar
+
+`Tasks::Private::FTaskBase::TSubsequents` (`OloEngine/src/OloEngine/Task/TaskPrivate.h`):
+
+```cpp
+TArray<FTaskBase*, AllocatorType> m_Subsequents;   // payload
+std::atomic<bool> m_IsClosed{ false };             // read unlocked, every AddSubsequent()
+FMutex m_Mutex;                                    // guards m_Subsequents, paired with m_IsClosed
+```
+
+`PushIfNotClosed()` — called via `FTaskBase::AddSubsequent()` for **every**
+prerequisite→subsequent edge registered anywhere in the task graph — reads
+`m_IsClosed` unlocked before deciding whether to take the lock at all. A busy
+task graph routinely has several worker threads racing to attach themselves
+as subsequents of the same shared prerequisite task concurrently, so this is
+a genuinely hot, multi-reader path — unlike the `Ref.cpp` case, here the
+*unlocked* read is the common case that matters (a closed subsequents list
+short-circuits before ever touching the mutex). The whole struct — a small
+inline-allocator `TArray` plus a `bool` plus a 1-byte `FMutex` — comfortably
+fits on one 64-byte line by default, so the adjacency needed no
+`offsetof` archaeology to confirm.
+
+Fix: `m_IsClosed` and `m_Mutex` are the two members that must land on the
+*same* line as each other — every access to one happens alongside the other,
+in `PushIfNotClosed()`/`Close()` — while staying off `m_Subsequents`'s line.
+`alignas(OLO_PLATFORM_CACHE_LINE_SIZE)` on `m_IsClosed` alone achieves that
+by relying on `m_Mutex` (1 byte, natural alignment 1) landing immediately
+after it in the same padded region — correct today, but implicit: a member
+inserted between them, or a reorder, would silently break the guarantee with
+no compiler diagnostic. The actual fix groups both into one `alignas`'d
+nested struct instead:
+
+```cpp
+struct alignas(OLO_PLATFORM_CACHE_LINE_SIZE) FCloseState
+{
+    std::atomic<bool> IsClosed{ false };
+    FMutex Mutex;
+};
+FCloseState m_Close;
+```
+
+— the pairing is then structural, not incidental to declaration order.
+This is the same shape as the existing `FPaddedSharedTask` wrapper in
+`Task/TaskConcurrencyLimiter.h`: when more than one member needs to share a
+line *with each other* while staying separated from something else, wrap
+them together rather than `alignas`-ing the first and trusting the rest to
+follow.
+
+## Outcome of the audit
+
+One site fixed out of ~2 dozen candidates surveyed across `Threading/`,
+`Task/`, `Async/`, `Audio/`, `Core/`. That is the expected outcome, not a
+sign the audit was too shallow — see the judgement criteria above. If a
+future pass through a different subsystem turns up more, the same three
+questions apply.