Skip to content

Commit 6952664

Browse files
perf: cache window bounds and replace deepcopy on window lists (-21% planning) (#4536)
* perf(plan): replace copy.deepcopy on window lists with a shallow clone Window dicts only ever hold primitives (start/end/average/target/id/key), so copy.deepcopy's generic recursive walk is doing no work that a per-dict .copy() does not - measured 30.0us vs 1.6us per call on a 48-window list, 18.5x. Adds utils.clone_windows() and uses it for the 13 planning-path deepcopy call sites, plus a plain .copy() for the flat {minute: float} load forecast in calculate_marginal_costs. The copies exist for isolation, not just duplication: thread_run_prediction_export writes export_window[window_n]["start"] in place, and plan_window_snapshot / preclip_new are restored later. clone_windows keeps that contract - each dict is copied, so in-place writes cannot leak either way - and the new tests pin it. This is performance-neutral on the benchmark: 20 scenarios A/B, 3 reps each, came out within noise (-0.4% median against ~6% run-to-run spread), because these call sites were only ~1.5% of plan time. Kept for the explicit isolation contract and the reduced allocation churn (deepcopy invocations -30%, total calls -3.4%), not for a speed claim. Verified byte-identical: all 20 random scenarios unchanged on metric, cost, and all three PV futures (+0.0000 across the board). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(kernel): cache window bounds instead of re-deriving them per simulation A search runs thousands of simulations over the same charge/export windows, varying only the limits, but every call re-derived the window start/end bounds from the window dicts - four ctypes arrays for the kernel scenario and two tuples for the prediction cache key. That was the largest single block of Python time in a plan. Caches them in prediction_kernel keyed on the identity of the window list, with the derived fields built lazily so a caller that only wants the hash tuple never pays to build the ctypes arrays. Hit rate on the benchmark is ~94%. Correctness rests on every mutation of a window's start/end invalidating the cache, so the 16 in-place assignments on the planning path now go through set_window_start()/set_window_end(). The guard is run_window_cache_tests, which replays a full calculate_plan with VALIDATE_WINDOW_CACHE on - that re-derives the bounds on every cache hit and raises on any stale entry, so a future bare window["start"] = ... on this path fails the suite rather than silently simulating the wrong window geometry. The test also asserts the validator itself catches a planted stale entry, so it cannot pass vacuously. Pool workers unpickle fresh window lists every call and can never hit the cache, where leaving it on cost ~3% of a pooled plan, so Pool() now runs disable_window_cache as its worker initialiser. The cache is bounded and pins the lists it keys on, so a caller that never repeats a list cannot grow it without limit or alias a freed list's id() onto the wrong entry. Measured on random scenario 0 (median of 3): threads=0 2366.3ms -> 1839.0ms -22.3% threads=auto 2134.0ms -> 2146.8ms +0.6% (noise) C++ share of plan time rises from 43.7% to 56.2%; pk_run itself is unchanged at 19,209 calls of 55.0us, which is what confirms the simulation work is identical. The 20 scenario benchmark drops 42.6s -> 33.4s. Verified byte-identical: all 20 random scenarios unchanged on metric, cost and all three PV futures, and kernel_parity passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(kernel): key the window bounds cache on the window count as well as identity Addresses two of the three review comments on this PR. A window list that grows or shrinks in place keeps its id(), so identity alone was not enough to decide a cache hit. run_prediction_kernel passes n_charge/n_export from len(window_list) alongside the cached arrays, so a stale shorter array would be read past its end by the C kernel - a memory-safety failure rather than merely a wrong plan. The count is now part of the hit condition. Nothing on the planning path resizes a window list today, so this guards the class rather than fixing a live defect. The test proves it would have bitten: before the change, appending to a cached list left a one-entry bounds array against two windows, and popping left window_bound_tuple returning the longer tuple. Also corrects the correctness note, which pointed at Plan.set_window_start() and Prediction.set_window_start(). Neither exists - they are module functions here - and after the rebase the prediction path does not use them at all, since _prepare_export applies a trial start copy-on-write to a window dict and list of its own. The third comment, to build the bound tuple from a generator rather than a list comprehension, is not taken: measured over 50k calls at 200 windows, the generator is 19.3% slower (307ms against 366ms), because it pays per-item interpreter overhead the specialised list comprehension avoids. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent ea44f61 commit 6952664

7 files changed

Lines changed: 404 additions & 54 deletions

File tree

.cspell/custom-dictionary-workspace.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,7 @@ undiscounted
528528
unnormalised
529529
unparseable
530530
unpickled
531+
unpickles
531532
unpushed
532533
unsmoothed
533534
unstaged

apps/predbat/plan.py

Lines changed: 38 additions & 39 deletions
Large diffs are not rendered by default.

apps/predbat/prediction_batch.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,8 @@
2222
byte-identical plan comparison is what keeps it that way.
2323
"""
2424

25-
from operator import itemgetter
26-
2725
from const import PREDICT_STEP
28-
from prediction_kernel import BatchJob, kernel_supported, run_prediction_kernel_batch
29-
30-
# Pulls (start, end) from a window dict; used to build the prediction cache key without a Python
31-
# level loop over every window
32-
window_bounds = itemgetter("start", "end")
26+
from prediction_kernel import BatchJob, kernel_supported, run_prediction_kernel_batch, window_bound_tuple
3327

3428

3529
def prediction_cache_key(charge_limit, charge_window, export_limits, export_window, pv_scenario, end_record, step):
@@ -39,13 +33,18 @@ def prediction_cache_key(charge_limit, charge_window, export_limits, export_wind
3933
different keys depending on which path reached it. Built as one tuple hash to keep the per-window
4034
hashing in C rather than looping in Python, which matters because it runs on every simulation
4135
with a few hundred windows.
36+
37+
The window bound tuples come from the identity-keyed cache in prediction_kernel, so a fan-out
38+
that varies only the limits derives them once rather than once per simulation. That cache is kept
39+
honest by set_window_start/set_window_end invalidating it; run_window_cache_tests replays a full
40+
plan with VALIDATE_WINDOW_CACHE on to prove no mutation escapes them.
4241
"""
4342
return hash(
4443
(
4544
tuple(charge_limit),
46-
tuple(map(window_bounds, charge_window)),
45+
window_bound_tuple(charge_window),
4746
tuple(export_limits),
48-
tuple(map(window_bounds, export_window)),
47+
window_bound_tuple(export_window),
4948
pv_scenario,
5049
end_record,
5150
step,

apps/predbat/prediction_kernel.py

Lines changed: 122 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,126 @@ def int32_array(values):
389389
return (ctypes.c_int32 * len(backing)).from_buffer(backing)
390390

391391

392+
# ---------------------------------------------------------------------------
393+
# Window bounds cache
394+
#
395+
# A search re-runs thousands of simulations over the same charge/export windows, changing only the
396+
# limits. The start/end bounds handed to the kernel are therefore identical call after call, but
397+
# were re-derived from the window dicts every time - measured as the largest single block of Python
398+
# time in a plan. They are cached here, keyed on the identity of the window list.
399+
#
400+
# CORRECTNESS: the cache is only sound while no window's start or end changes underneath it. Every
401+
# such mutation must call invalidate_window_cache() - route them through the set_window_start() and
402+
# set_window_end() helpers below rather than assigning window["start"] or window["end"] directly.
403+
# The prediction path does not need them: _prepare_export applies a trial start copy-on-write, to a
404+
# window dict and list of its own, so nothing the cache has seen is touched. run_window_cache_tests replays a full plan with VALIDATE_WINDOW_CACHE on, which
405+
# re-derives the bounds on every hit and fails on any stale entry, so a missed invalidation is a
406+
# test failure rather than a silently wrong plan.
407+
#
408+
# Entries pin their window list (so a freed list cannot have its id() reused by a later
409+
# allocation, which would alias to the wrong entry) and the cache is bounded, so a caller that
410+
# never repeats a list - a forked pool worker, which unpickles fresh lists every call - thrashes
411+
# harmlessly within the bound instead of growing without limit.
412+
# ---------------------------------------------------------------------------
413+
WINDOW_CACHE_MAX = 16
414+
VALIDATE_WINDOW_CACHE = False
415+
WINDOW_CACHE_ENABLED = True
416+
_WINDOW_BOUNDS_CACHE = {}
417+
418+
419+
def disable_window_cache():
420+
"""Turn the window bounds cache off in this process - used as the pool worker initialiser.
421+
422+
A worker unpickles fresh window lists on every call, so it can never hit the cache; leaving it
423+
on there only pays the failed lookup. Measured at ~3% of a pooled plan.
424+
"""
425+
global WINDOW_CACHE_ENABLED
426+
WINDOW_CACHE_ENABLED = False
427+
_WINDOW_BOUNDS_CACHE.clear()
428+
429+
430+
def invalidate_window_cache():
431+
"""Drop all cached window bounds - must be called whenever a window's start or end changes"""
432+
_WINDOW_BOUNDS_CACHE.clear()
433+
434+
435+
def set_window_start(window, start):
436+
"""Set a window's start time, invalidating any cached bounds derived from it.
437+
438+
Use this instead of assigning window["start"] directly - see the window bounds cache notes
439+
above for why a bare assignment is unsafe.
440+
"""
441+
if window["start"] != start:
442+
window["start"] = start
443+
_WINDOW_BOUNDS_CACHE.clear()
444+
445+
446+
def set_window_end(window, end):
447+
"""Set a window's end time, invalidating any cached bounds derived from it - see set_window_start"""
448+
if window["end"] != end:
449+
window["end"] = end
450+
_WINDOW_BOUNDS_CACHE.clear()
451+
452+
453+
def _window_cache_entry(window_list):
454+
"""Return the cache entry [window_list, starts, ends, bounds_tuple] for a window list.
455+
456+
The derived fields start as None and are filled in on first use, so a caller that only wants
457+
the hash tuple never pays to build the ctypes arrays, and vice versa.
458+
"""
459+
key = id(window_list)
460+
entry = _WINDOW_BOUNDS_CACHE.get(key)
461+
# The window count is part of the hit condition, not just the identity: a list that grows or
462+
# shrinks in place keeps its id(), and run_prediction_kernel passes n_charge/n_export from
463+
# len(window_list) alongside these arrays. A stale shorter array would then be read past its end
464+
# by the C kernel - a memory-safety failure rather than merely a wrong plan. Bare start/end
465+
# writes are handled by set_window_start/set_window_end instead; nothing on the planning path
466+
# resizes a window list today, so this is a guard against the class rather than a live fix.
467+
if entry is not None and entry[0] is window_list and entry[4] == len(window_list):
468+
if VALIDATE_WINDOW_CACHE:
469+
_validate_entry(window_list, entry)
470+
return entry
471+
if len(_WINDOW_BOUNDS_CACHE) >= WINDOW_CACHE_MAX:
472+
_WINDOW_BOUNDS_CACHE.clear()
473+
entry = [window_list, None, None, None, len(window_list)]
474+
_WINDOW_BOUNDS_CACHE[key] = entry
475+
return entry
476+
477+
478+
def window_bound_arrays(window_list):
479+
"""Return (start_array, end_array) as ctypes int32 arrays for a window list, cached by identity.
480+
481+
The returned arrays are shared with other callers and must not be modified.
482+
"""
483+
if not WINDOW_CACHE_ENABLED:
484+
return int32_array([window["start"] for window in window_list]), int32_array([window["end"] for window in window_list])
485+
entry = _window_cache_entry(window_list)
486+
if entry[1] is None:
487+
entry[1] = int32_array([window["start"] for window in window_list])
488+
entry[2] = int32_array([window["end"] for window in window_list])
489+
return entry[1], entry[2]
490+
491+
492+
def window_bound_tuple(window_list):
493+
"""Return a tuple of (start, end) pairs for a window list, cached by identity (prediction cache key)"""
494+
if not WINDOW_CACHE_ENABLED:
495+
return tuple([(window["start"], window["end"]) for window in window_list])
496+
entry = _window_cache_entry(window_list)
497+
if entry[3] is None:
498+
entry[3] = tuple([(window["start"], window["end"]) for window in window_list])
499+
return entry[3]
500+
501+
502+
def _validate_entry(window_list, entry):
503+
"""Re-derive the bounds and raise if the cached entry is stale (VALIDATE_WINDOW_CACHE only)"""
504+
starts = [window["start"] for window in window_list]
505+
ends = [window["end"] for window in window_list]
506+
if entry[1] is not None and (starts != list(entry[1]) or ends != list(entry[2])):
507+
raise AssertionError("Stale window bounds cache (arrays): a window changed without invalidate_window_cache().\n starts cached {} actual {}\n ends cached {} actual {}".format(list(entry[1]), starts, list(entry[2]), ends))
508+
if entry[3] is not None and tuple(zip(starts, ends)) != entry[3]:
509+
raise AssertionError("Stale window bounds cache (tuple): a window changed without invalidate_window_cache().\n cached {}\n actual {}".format(entry[3], tuple(zip(starts, ends))))
510+
511+
392512
def kernel_context_free(handle):
393513
"""Free a kernel context by handle (used as a weakref finaliser)"""
394514
if KERNEL_LIB and handle:
@@ -774,11 +894,9 @@ def run_prediction_kernel(pred, charge_limit, charge_window, export_window, expo
774894
# The window fields keep their list comprehensions - map(itemgetter(...)) measured *slower* here at these
775895
# list lengths, the per-item call overhead outweighing what the comprehension costs.
776896
scenario.charge_limit = double_array(charge_limit)
777-
scenario.charge_start = int32_array([window["start"] for window in charge_window])
778-
scenario.charge_end = int32_array([window["end"] for window in charge_window])
897+
scenario.charge_start, scenario.charge_end = window_bound_arrays(charge_window)
779898
scenario.export_limits = double_array(export_limits)
780-
scenario.export_start = int32_array([window["start"] for window in export_window])
781-
scenario.export_end = int32_array([window["end"] for window in export_window])
899+
scenario.export_start, scenario.export_end = window_bound_arrays(export_window)
782900
# A cached run discards the per-minute SoC series (see the `if not cache` block below), so the
783901
# buffer is not allocated and the kernel is told to skip filling it. That skips a round_py per
784902
# step, which is snprintf+strtod and the single most expensive thing in the kernel's hot loop -

0 commit comments

Comments
 (0)