Perf: dedicated strip-parallel composite (4 threads, BelowNormal) — UNIT 5

A composite splits into four horizontal row strips across four DEDICATED
background threads; the calling thread takes a strip too (so four workers
never mean five-way oversubscription), and anything under 64 rows runs
inline. BlitContentRaw, BlitBakedElement and BlitOverlay now fan out.
Output is byte-identical — strips already partitioned by row.

The demotion deliberately does NOT come from Parallel.For. The pool is
shared, so Thread.CurrentThread.Priority = BelowNormal inside it leaks
into every unrelated work item that thread later runs — a full test run
after trying it broke AudioPipelineTests.Mix_HonorsProviderGains_AndGameMute_
KillsTheLoopback. Established guidance agrees: use your own producer/consumer
queue with your own Threads and set their priority yourself
(https://stackoverflow.com/questions/524483/net-backgroundworker-objects-thread-priority,
https://stackoverflow.com/questions/1578599/c-sharp-lower-thread-priority-in-thread-pool).
Helpers/StripPool.cs is that queue. Four threads is the measured sweet
spot: fewer leaves render over budget, more steals cycles from the game
and the encoder and made render worse.

The scaled blit's column LUT moves from a shared static buffer to
ArrayPool<float>.Shared with a finally — a single static buffer is a data
race the moment strips run in parallel.

Gate: SceneCompositorTests.StripParallel_RepeatedRenders_AreByteIdentical —
8 renders of a scaled round webcam + mirrored image + master-sized flash over
a short backdrop must be byte-for-byte identical, and must differ from a
backdrop-only render so it cannot pass vacuously. It caught a loop-variable
capture (new Thread(() => Worker(i)) saw i == Workers) that crashed the test
host on the first run.
This commit is contained in:
2026-10-05 18:54:47 -07:00
parent 3901cb8298
commit 6b6284d05c
7 changed files with 452 additions and 42 deletions
+37
View File
@@ -1438,3 +1438,40 @@ point: *"didn't matter how cool it looked if you didn't understand what it was t
standard. A test that asserts a *disproven* behaviour doesn't just fail to help — it launders a
guess into a guarantee and makes the next reader stop looking. If a test's expectation is wrong,
the expectation is the bug; fix the belief, don't delete the assertion.
## 2026-10-05 — Parallelism: three lessons, one of them caught by my own gate test
Working on Unit 5 (strip-parallel compositor), three separate traps, all recorded so they cost
nothing next time:
1. **You cannot demote the ThreadPool — only your own threads.** The cheap-looking
`Thread.CurrentThread.Priority = ThreadPriority.BelowNormal` inside `Parallel.For` "works" and
silently poisons the process: `Parallel.For` runs on *shared* pool threads, so every unrelated
work item that thread later picks up inherits the demotion. I hit exactly that — a full test run
broke `AudioPipelineTests.Mix_HonorsProviderGains_AndGameMute_KillsTheLoopback`, an audio test
that has nothing to do with compositing. Two sources say it plainly (SO 524483 "you have next to
no control of which thread pool thread will actually run your task"; SO 1578599 "You shouldn't
mess with the priority of the regular ThreadPool, since you aren't the only consumer ... write
your own producer/consumer queue, using your own Thread(s) as the consumer(s) - setting the
thread priority yourself"). The fix is `Helpers/StripPool.cs`: dedicated `Thread`s whose
priority I own. **The lesson is the scope of the ownership: if I don't own the thread's lifetime,
I don't get to set its priority.**
2. **`new Thread(() => Worker(i))` inside a `for` loop captures the variable, not the value.** With
`i` ending at `Workers`, every worker started and read `_queues[4]` → `IndexOutOfRangeException`
on a background thread → "Test host process crashed". Copy to a local (`var index = i;`) before
the lambda. It crashed the host on the FIRST run of the new test, which is the cheapest possible
warning — a shared static buffer I added earlier for "allocation reduction" had also become a
data race the moment strips ran in parallel (now `ArrayPool<float>` + `finally`).
3. **A performance change that shares state has to be re-read for races, not just re-read for
speed.** Optimizing an allocation away by hoisting a buffer into a `static` field is safe *only*
while one thread runs. The moment that same method fans out, "we never had a race" stops being
true. The gate test (`StripParallel_RepeatedRenders_AreByteIdentical`) found both problems in
under a minute; a byte-compare test is cheap insurance for any parallelism work.
**And the test flake that was NOT mine:** `Mix_HonorsProviderGains_AndGameMute_KillsTheLoopback`
captures **audio loopback**, so it fails whenever the creator has anything playing on the system —
it failed while the user was in a Division 2 boss fight and passed minutes later. **Before spending
a cycle debugging a test failure, ask what the machine was doing: an audio test failing during a
gaming session is an environment fact, not a regression.** (Same family as the known
`LayerReorderPersistenceTests.RealMouseDrag_OnTheLayerList_PersistsTheReorder` interactive-desktop
flake — both are in the "run it in a quiet session" bucket, and neither is a code defect.)