perf: cache window bounds and replace deepcopy on window lists (-21% planning)

[springfall2008]

Two related changes to how the planner handles window lists. Both are byte-identical on the 20-scenario random benchmark.

1. clone_windows instead of copy.deepcopy (10e4092)

Window dicts only hold primitives, so deepcopy's recursive walk does no work a per-dict .copy() doesn't — 30.0µs vs 1.6µs on a 48-window list.

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 get restored later. clone_windows keeps that contract and run_clone_windows_tests pins it.

This one is performance-neutral (-0.4% median against ~6% run-to-run spread — these sites were only ~1.5% of plan time). Kept for the explicit isolation contract and reduced allocation churn, not for a speed claim.

2. Window bounds cache (d271dc7) — the actual win

A search runs thousands of simulations over the same windows, varying only the limits, but every call re-derived the start/end bounds from the dicts: 4 ctypes arrays for the kernel scenario, 2 tuples for the prediction cache key. That was the largest single block of Python time in a plan. Now cached by window-list identity, built lazily. ~94% hit rate.

	before	after
threads=0	2366.3ms	1839.0ms	-22.3%
threads=auto	2134.0ms	2146.8ms	+0.6% (noise)
C++ share of plan	43.7%	56.2%
20-scenario benchmark	42.6s	33.4s	-21%

pk_run itself is unchanged — 19,209 calls at 55.0µs before and after — which is what confirms the simulation work is identical and only Python overhead was removed.

Correctness

The cache is only sound while every mutation of a window's start/end invalidates it, so the 16 in-place assignments on the planning path 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 — re-deriving bounds on every cache hit and raising on any stale entry. A future bare window["start"] = ... on this path fails the suite rather than silently simulating the wrong geometry. The test also asserts the validator catches a planted stale entry, so it can't pass vacuously.

Pool workers unpickle fresh lists every call and can never hit the cache, where leaving it on cost ~3% of a pooled plan — so Pool() 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 can't grow it without limit or alias a freed list's id() onto the wrong entry.

Verification

• 20/20 random scenarios byte-identical: metric, cost, and all three PV futures +0.0000
• kernel_parity passes (no kernel ABI change)
• Full suite + pre-commit green

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR reduces Python-side overhead in planning by (1) replacing copy.deepcopy on window lists with a purpose-built shallow clone and (2) caching derived (start,end) bounds for window lists to avoid recomputation across repeated simulations, while adding targeted tests to protect correctness.

Changes:

• Add utils.clone_windows() and replace copy.deepcopy(...) usages on window-list snapshots with shallow per-dict copies.
• Add a window-bounds cache in prediction_kernel and route window start mutations through set_window_start() (and disable caching in pool workers).
• Extend the unit test suite with clone_windows and window-cache validation tests; update spell-check dictionary for new wording.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
apps/predbat/utils.py	Adds clone_windows() helper for fast window-list cloning.
apps/predbat/unit_test.py	Registers new window clone/cache test runners; adds isolated runner for cache replay test.
apps/predbat/tests/test_window.py	Adds clone_windows tests and comprehensive window-cache validation tests.
apps/predbat/prediction.py	Uses window_bound_tuple() for prediction cache key; routes export window start mutation via set_window_start().
apps/predbat/prediction_kernel.py	Introduces window bounds caching and mutation helpers; uses cached arrays in run_prediction_kernel.
apps/predbat/plan.py	Replaces deepcopies with clone_windows()/shallow copies and disables window cache in pool workers; routes start mutations via set_window_start().
apps/predbat/marginal.py	Replaces deepcopy of flat load dict with .copy() for performance.
.cspell/custom-dictionary-workspace.txt	Adds “unpickles” to custom dictionary.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[springfall2008]

Rebased onto main (now including #4549) and worked through the three review comments. Two applied and are fixed in 9bf0549; one does not hold and I've left the code as it was.

Review comments

1. Correctness note points at functions that don't exist — fixed. It said to route mutations through Plan.set_window_start() / Prediction.set_window_start(); they're module functions in prediction_kernel. After the rebase it was doubly wrong: the prediction path doesn't use them at all any more, because _prepare_export applies the trial start copy-on-write to a window dict and list of its own. The note now says so.

2. Cache should guard against in-place length changes — fixed, and it was real. 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 — but the failure mode is worse than a wrong plan, since run_prediction_kernel passes n_charge/n_export from len(window_list) alongside the cached arrays, so a stale shorter array is read past its end by the C kernel.

The test written for it fails without the guard, which is what makes it worth having:

ERROR: bounds arrays are stale after the window list grew - len 1 against 2 windows
ERROR: window_bound_tuple is stale after the window list shrank: ((0, 30), (60, 120))

3. Use a generator instead of tuple([listcomp]) — not taken. Measured over 50,000 calls at 200 windows:

	time
tuple([(w["start"], w["end"]) for w in windows])	307 ms
tuple((w["start"], w["end"]) for w in windows)	366 ms (19.3% slower)

The generator pays per-item interpreter overhead that the specialised list comprehension avoids, so the change would work against the performance intent rather than with it.

Rebase notes

The branch predated #4540's batching redesign, so this was more than a textual merge. Three judgement calls worth flagging:

• Kept main's copy-on-write in _prepare_export. This branch changed it to set_window_start(window, start), which writes to the caller's window in place — the defect perf: batch the prediction fan-out into one kernel call (35% faster planning) #4540 fixed. With a batched fan-out sharing one list across every job, that would corrupt sibling trials of the same window. The accessors are still used at all 16 plan.py sites, which is where they belong.
• Moved the cached bounds into the shared prediction_cache_key. This branch only optimised the direct run_prediction path, since the batch path didn't exist when it was written. Routing it through the shared helper means enqueue_prediction benefits too, and that is where the large majority of the ~44k key builds per plan come from.
• Dropped the process pool re-introduction (wrapped_run_prediction_*, Pool(initializer=disable_window_cache)), removed in perf: batch the prediction fan-out into one kernel call (35% faster planning) #4540. disable_window_cache is now unused but left defined.

Also kept main's list(export_window) over clone_windows in optimise_export: since #4540 no dict on that path is mutated, so the per-dict copies buy nothing.

Measurement

Interleaved against main (ea44f618), best of three, kernel confirmed active and batch-capable on both sides:

20-scenario suite	main	this branch	change
Optimisation time	20.272s	16.644s	17.9% faster
id 6	4.054s	3.012s	25.7% faster
id 7	2.180s	1.661s	23.8% faster
id 17	1.253s	0.980s	21.8% faster

960 fields compared across three rounds of all 20 scenarios — zero mismatches. Full suite of 217 tests and pre-commit green.

Cumulatively with #4549, planning is 36.3% faster than it was before that PR (26.13s → 16.64s).

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

apps/predbat/prediction_kernel.py:498

• window_bound_tuple() builds an intermediate list before converting to a tuple (both in the cache-disabled path and when initially populating the cached value). This adds avoidable allocations on a hot path; using a generator expression produces the same tuple without the temporary list.

    if not WINDOW_CACHE_ENABLED:
        return tuple([(window["start"], window["end"]) for window in window_list])
    entry = _window_cache_entry(window_list)
    if entry[3] is None:
        entry[3] = tuple([(window["start"], window["end"]) for window in window_list])