perf(prediction): build the kernel's ctypes arrays via array.array (1.9x faster planning) - #4509
Conversation
The six per-simulation ctypes buffers were built as (ctypes.c_double * n)(*values), which unpacks the list into positional arguments - several times slower than copying from an array.array of the same type. Marshalling was the largest remaining cost in the planner after #4505, #4507 and #4508: 8.1s of a 13.9s profiled plan, more than everything else combined. Measured on the shapes the planner actually passes, building the charge window geometry drops from 35.0us to 12.7us per call. Content-keyed memoisation of the geometry arrays was measured as an alternative (10.8us) and rejected: it is only marginally ahead, because the key still has to be built and hashed, and it would have to stay correct across the passes that mutate window bounds in place. Reusing the soc_out buffer was also measured and is not worth it at 0.15us per allocation. from_buffer returns a view over the array.array rather than a copy, so the backing object has to outlive the kernel call. ctypes keeps it alive through the view's _objects; a new test asserts that rather than assuming it, since the failure mode is reading freed memory silently. The pool workers are separate forked processes so no buffer is shared between them, which was verified by running the same plan single-process and pooled and confirming identical results (not added as a test - it needs two full plan runs). Benchmark: worst scenario 11.03s -> 8.39s, mean optimise time across the 20 scenarios 3.717s -> 1.961s (1.9x), with plan metric and cost identical on all 20. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR speeds up the prediction planner’s Python→ctypes marshalling path by building ctypes arrays via array.array + from_buffer, reducing per-simulation overhead before calling the C++ kernel. It also updates kernel parity tests and refreshes benchmark result fixtures to reflect the improved runtimes.
Changes:
- Switch
double_array/int32_arraymarshalling helpers toarray.array(...).from_buffer(...)for faster ctypes buffer creation. - Add kernel parity “marshalling” tests to validate value round-trip and backing-buffer retention semantics.
- Update
coverage/cases/random_results.jsonwith new run timestamp and improvedruntime_snumbers.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
apps/predbat/prediction_kernel.py |
Uses array.array + from_buffer to construct ctypes arrays faster for kernel inputs. |
apps/predbat/tests/test_kernel_parity.py |
Adds parity-suite tests to validate marshalling safety/behaviour. |
coverage/cases/random_results.json |
Updates benchmark fixture metadata and runtimes after the perf change. |
Suppressed comments (1)
apps/predbat/prediction_kernel.py:281
- int32_array() uses array.array('i'), where 'i' is a C "int" and its item size is platform-dependent. That can break the function’s implied int32 contract (ctypes.c_int32) on platforms where C int is not 32-bit, and from_buffer would then interpret the backing bytes incorrectly. Add an itemsize guard with a safe fallback.
"""Create a ctypes int32 array from a Python list - see double_array for why array.array is used"""
backing = array.array("i", values)
return (ctypes.c_int32 * len(backing)).from_buffer(backing)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| backing = array.array("d", values) | ||
| return (ctypes.c_double * len(backing)).from_buffer(backing) |
There was a problem hiding this comment.
Fixed in e10031c. This one was worth more than robustness on unusual platforms - I checked what actually happens on a mismatch and it is silent, not an exception: building a c_int32 array over an array('d') backing returns [0, 1072693248, 0] rather than raising, because from_buffer only checks the buffer is large enough. So a wrong-width typecode would feed corrupted window bounds into the kernel. The typecode is now selected at import by matching itemsize against the ctypes element, with a fallback to the original (ctypes.c_double * n)(*values) construction when nothing matches - such a platform loses the speed-up instead of silently corrupting results. Covered by a new itemsize assertion in the marshalling test.
…atch Review feedback on #4509. array.array's integer typecodes are C types, so 'i' is a C int - 32 bit everywhere predbat runs, but not guaranteed to be. The failure mode is not an exception: from_buffer only checks the buffer is large enough, so a wider backing type is accepted and the kernel reads interleaved garbage. Confirmed directly - building a c_int32 array over an array('d') backing returns [0, 1072693248, 0] rather than raising. The typecode is now chosen at import by matching itemsize against the ctypes element, falling back to the slower (ctypes.c_double * n)(*values) construction if nothing matches, so a platform with unusual widths loses the speed-up rather than silently corrupting the simulation inputs. Also from the review: the retention check now uses truthiness via getattr rather than "is not None", since an empty _objects means nothing is retained and is just as unsafe as the attribute being absent; and the empty-input case is asserted for double_array as well as int32_array. Mutation-testing the new guard turned up a bug in the test itself: it asserted _objects retention unconditionally, but the fallback path copies its values and so has nothing to retain. On any platform taking the fallback the suite would have failed spuriously. The assertion is now scoped to the from_buffer path. Verified by forcing each state: correct typecodes pass, no typecode (fallback) passes, and a deliberately wrong-width typecode is caught by both the itemsize check and the round-trip check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
After #4505, #4507 and #4508, marshalling into the C++ kernel was the largest remaining cost in the planner — 8.1s of a 13.9s profiled plan, more than everything else combined:
The helpers built their buffers as
(ctypes.c_double * n)(*values), which unpacks the list into positional arguments. Copying from anarray.arrayof the same type is several times faster. Measured on the shapes the planner actually passes (266 charge windows):(c_int32 * n)(*values)array.array+from_bufferAlternatives measured and rejected
optimise_swap_exportand friends), which is a poor trade for 1.8µs.soc_outbuffer — allocation is 0.15µs/call, not worth it.Safety
from_bufferreturns a view over thearray.array, not a copy, so the backing object must outlive the kernel call — the failure mode is reading freed memory, silently and intermittently. ctypes keeps it alive through the view's_objects; a new test asserts that rather than assuming it, and also covers value round-trip and the empty-array case.The pool workers are separate forked processes (
multiprocessing.Pool,set_start_method("fork")inhass.py), so no buffer is ever shared between workers. Verified by running the same plan single-process and with a 4-process pool and confirming identical charge/export windows and limits — not added as a test since it needs two full plan runs.Impact
Cumulatively across #4505, #4507, #4508 and this: the worst scenario is down from 152.0s to 8.39s (18x), and the benchmark mean from 13.674s to 1.961s.
Test plan
run_marshalling_testsin the kernel parity suite: value round-trip,_objectsretention, empty arraykernel_parity(450 random configurations + 250 clipping layouts) passesrun_random: metric and cost identical on 20/20--quick,debug_cases, pre-commit all pass🤖 Generated with Claude Code