Skip to content

Commit e81b3d3

Browse files
docs: report on the planning performance work
Records the measured result (39.5s to 24.8s on the 20-scenario benchmark, plans unchanged), what the two PRs changed, and the four design assumptions that a benchmark overturned - the scratch reuse that turned out to be a no-op, the broadcast wake that cost most of the win, the thread cap that was right on fast hardware and wrong on slow, and an inherited baseline that did not survive re-measurement. Also records the defects found on the way, including the ABI version that let a stale binary load and segfault, and the five tests that could not have failed until they were mutation-checked. Lives under docs/superpowers/ with the plans and specs rather than in the MkDocs nav: it is an engineering record, not user documentation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 06beccc commit e81b3d3

1 file changed

Lines changed: 187 additions & 0 deletions

File tree

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
# Planning performance: batching the fan-out and pooling the kernel's threads
2+
3+
Report on the work in PR #4540 and PR #4546, both merged on 2026-08-16.
4+
5+
**Result: the 20-scenario planning benchmark goes from 39.5s to 24.8s, 37% faster, with every
6+
scenario producing an identical plan.**
7+
8+
## The measurement
9+
10+
Current `main` against the two branches together. Each side runs its own Python and its own compiled
11+
kernel, interleaved on one 16-core machine, best of three rounds.
12+
13+
| 20-scenario suite | `main` | both PRs | change |
14+
|---|---|---|---|
15+
| Serial (`threads: 0`) | 41.70s | 26.26s | 37.0% faster |
16+
| Default (`threads: auto`) | 39.53s | 24.82s | 37.2% faster |
17+
18+
All 20 scenarios identical on metric, cost, both PV futures, final SoC, battery cycles and carbon —
19+
zero mismatches across 140 compared fields. The win holds with parallelism switched off, so it comes
20+
from the batching rather than a threading effect that might not transfer to other hardware.
21+
22+
## What changed
23+
24+
### Batching the fan-out (#4540)
25+
26+
The optimiser runs thousands of trial simulations per plan. Each `launch_run_prediction_*` used to
27+
run one immediately, or hand it to a pool of worker processes. It now prepares the trial inputs,
28+
queues a job on the `Prediction`, and returns a handle. The first `.get()` flushes the whole fan-out:
29+
prediction-cache hits resolved in Python, duplicate trials collapsed, and the remainder marshalled
30+
once and run in a single call across the C boundary.
31+
32+
The call sites did not have to change — every fan-out in `plan.py` was already written
33+
launch-all-then-collect-all for the old pool. The process pool, `PRED_GLOBAL` and the wrapper
34+
trampolines are all deleted.
35+
36+
### A thread pool inside the kernel (#4546)
37+
38+
With batching in place, threading still bought only 1.6%. Instrumenting the kernel over one full plan
39+
showed why:
40+
41+
| Per plan, inside `pk_run_batch` | serial | threaded |
42+
|---|---|---|
43+
| Calls / jobs | 25,091 / 253,899 | same |
44+
| Threads created | 0 | 201,545 |
45+
| Wall time in the function | 3131 ms | 2409 ms |
46+
| — of which thread creation || 1507 ms |
47+
| — of which work | 3131 ms | 902 ms |
48+
49+
The parallel work compresses well: 3131 ms of simulation becomes 902 ms across eight lanes. Thread
50+
creation then ate almost all of it. The median batch is six jobs, so each call was paying eight
51+
thread creations at 7.5 microseconds to cover roughly 180 microseconds of work. Workers now park on a
52+
condition variable for the life of the process.
53+
54+
## What measurement overturned
55+
56+
Four things that seemed sound in design did not survive a benchmark. They are recorded here and in
57+
the code rather than quietly dropped.
58+
59+
### Per-thread scratch buffers: no effect
60+
61+
The kernel allocated roughly 70KB of scratch per call, 25,000 times per plan. Removing that was
62+
expected to pay on the serial path. It measured 26.291s before and 26.344s after — nothing, against
63+
1.8% run-to-run spread.
64+
65+
The premise was a misreading. The kernel's own comment about the allocator being a bottleneck was
66+
recorded under four threads contending, which is a different problem from one thread repeatedly
67+
allocating and freeing the same-sized block and never leaving its thread cache. The change survives
68+
only because the parked workers need per-thread scratch anyway.
69+
70+
### Broadcasting to wake workers: cost 2.4%
71+
72+
Built as designed, with one condition variable broadcast to all workers, the pool was worth 1.3%. A
73+
six-job batch woke all fifteen workers and eleven went straight back to sleep — roughly 250,000
74+
wasted wakeups per plan. Giving each worker its own flag and condition variable, so only the needed
75+
lanes wake, took it to 3.7% on that change alone.
76+
77+
### Capping the default thread count: reversed
78+
79+
On a fast 16-core machine the curve peaks below the core count — 24.71s at six threads against 25.04s
80+
at sixteen — which argued for capping `auto`.
81+
82+
Re-running with each job made eight times dearer, which is how a machine where the kernel dominates
83+
behaves, the curve stops turning over at all:
84+
85+
| `threads` | normal | kernel 8x dearer |
86+
|---|---|---|
87+
| 0 (serial) | 26.33s | 48.92s |
88+
| 4 | 24.89s | 32.03s |
89+
| 6 | 24.71s | 29.98s |
90+
| 8 | 24.91s | 29.85s |
91+
| 16 | 25.04s | 28.94s |
92+
93+
Capping at four would cost 0.7% on fast hardware but 10.7% on the weak hardware the cap was meant to
94+
protect. The cap was removed; `resolve_batch_threads` in `plan.py` carries the numbers.
95+
96+
### The inherited baseline: stale
97+
98+
A 31.6s baseline had been carried forward from an earlier session's notes. Re-measured on the same
99+
machine it was 41.7s. Every figure in this report was measured fresh, interleaved, best of three.
100+
101+
## Defects found along the way
102+
103+
### A stale kernel binary was loaded instead of rejected
104+
105+
An earlier commit made the SoC output buffer optional — Python passes null for every cached run,
106+
which is the common path — but left `PK_ABI_VERSION` at 3. An ABI 3 binary writes to that pointer
107+
unconditionally, so the loader accepted a stale binary and Predbat segfaulted on its first prediction
108+
rather than falling back to the Python engine.
109+
110+
Reproduced by building the kernel from `main` and pointing `PREDBAT_KERNEL_SO` at it: `model_kernel`
111+
exits 139. Now bumped to ABI 4, so the loader reports a stale binary and uses the Python engine, which
112+
is what the check exists to do. Anyone with a locally built library from an older checkout, or an
113+
install whose per-architecture binary was not replaced, would have hit this.
114+
115+
### The old process pool was broken off the main entry point
116+
117+
Pool workers rebuilt their `Prediction` from a module global that only exists in the parent, which
118+
needs `fork` start semantics that `hass.py` sets only under `if __name__ == "__main__"`. On any
119+
spawn-default platform, macOS since Python 3.8, every worker started empty and every scenario died
120+
with `KeyError: 'dict'`. Removing the pool removes the failure mode.
121+
122+
### A trial mutating the caller's data
123+
124+
The export trial wrote its start time straight into the caller's window dictionary, safe only because
125+
each pool worker mutated its own unpickled copy. A batched fan-out shares one list across every job,
126+
so this had to become local before anything else could proceed.
127+
128+
### A header that only libc++ provides transitively
129+
130+
The pool catches `std::system_error` when the system refuses a thread. libc++ declares it through
131+
`<thread>`; libstdc++ does not, so the file compiled on macOS and failed to parse on GCC.
132+
133+
Worth knowing for the next kernel change: neither local build catches this. The native macOS build is
134+
clang/libc++, and `build_kernel_cross.sh` produces the shipped Linux binaries through zig, which
135+
bundles libc++ as well. So the cross-build targets glibc but not libstdc++, and it will happily
136+
produce six loadable Linux binaries from source that a maintainer building with `g++` cannot compile.
137+
Only CI, which builds natively with g++, closes that gap.
138+
139+
## How the tests were checked
140+
141+
Every new test was verified by deliberately breaking the thing it guards and confirming it fails.
142+
That found five tests, three of them written during this work, that could not have failed:
143+
144+
- A gate keeping debug runs on the Python engine was pinned by comparing values the two engines agree
145+
on to within 1e-6. Deleting the gate entirely left the suite green.
146+
- Nothing constrained how batch results were matched back to their jobs. A deliberately reversed
147+
mapping — every trial receiving another trial's cost — passed three separate suites.
148+
- A replacement routing test had nine of ten trials producing identical costs, so a misrouted result
149+
would have coincidentally matched. The trial shape and seed were changed until all ten differ, and
150+
the distinctness is now asserted rather than assumed.
151+
- The concurrency test passed with the dispatch mutex deleted: at 25 rounds the interpreter lock kept
152+
the two callers from overlapping. At 400 rounds over 8 lanes it deadlocks within seconds. That round
153+
count is load-bearing and the test says so.
154+
- The fork test does earn its place. Remove the `pthread_atfork` handler and the forked child
155+
deadlocks on workers that no longer exist, which the test kills on a deadline rather than hanging
156+
the suite.
157+
158+
Alongside that: the full suite of 211 tests, the 20-scenario byte-identical gate re-run at every step,
159+
and kernel parity pinned bit-identical across one, two, four and eight threads.
160+
161+
## Notes for whoever picks this up
162+
163+
`PK_PARITY_REVISION` stays at 5 while the ABI moved to 4. The bucket-table change altered how the hot
164+
loop computes SoC percent, and it is pinned equivalent by `pk_verify_soc_percent_table`, so results do
165+
not diverge. That is the same class of judgement that was got wrong for the ABI version, so it is
166+
worth a second opinion.
167+
168+
The kernel is now about 12% of plan time, which caps what any further C++ work can return. The
169+
remaining 88% is Python, and the batch-size census says those fan-outs are small and frequent — a
170+
median of six jobs against an expected thousand, because a large fan-out's ~1176 launches become ~363
171+
kernel jobs once the 44% prediction-cache hit rate and the intra-batch deduplication have taken their
172+
share. Widening them means changing optimiser search semantics in `optimise_charge_limit`, where a
173+
min/max pre-pass computes the SoC-pruning envelope that decides which trials are launched at all. That
174+
is a design decision with real plan-difference risk, not an optimisation.
175+
176+
## Method
177+
178+
Benchmarks were run on a 16-core machine, interleaved rather than grouped, best of three, with the
179+
kernel confirmed active on both sides of every comparison. Run-to-run spread on this benchmark is
180+
2-6%, so single runs were not trusted. Plan equality was checked field by field across all 20
181+
scenarios rather than on the headline metric alone.
182+
183+
Two measurement mistakes are worth recording. An early run reported plausible numbers that were
184+
actually a stale `random_results.json` being re-read after every run had crashed; the runner now
185+
deletes that file before each run so the failure mode is impossible rather than merely unlikely. And
186+
`main`'s threaded column required forcing `fork` start semantics — left alone its process pool dies
187+
with `KeyError: 'dict'`, which would have flattered these branches for an unrelated reason.

0 commit comments

Comments
 (0)