Repository navigation
Pause automatic garbage collection while the YAML runner builds a reform system - #643
Merged
Merged
Conversation
A cache miss in _get_tax_benefit_system clones the whole baseline. On policyengine-us the full collections that its allocations trigger walk every object both systems hold and free almost nothing. Pause automatic collection across the clone, reforms and extensions, and restore the caller's setting in a finally. A cache hit does not touch collection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With collection only paused, the build's objects were walked three times after it resumed: by a young, a middle and a full collection, all in the next case. On policyengine-us that cost more than the collections the pause avoided. Run one full collection when a build returns, for a caller that had collection on, and test that a hit, a failed build and a caller with collection off get none. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis
marked this pull request as ready for review
October 31, 2026 01:30
A forced full collection after every cache miss walks every cached system again, so a run of misses cost quadratic time: 100 distinct parameter reforms on the country template took 6.4 s of CPU against master's 0.09 s. Force it only while the cache holds fewer than _FULL_COLLECTION_CACHE_LIMIT (3) systems, and leave later copies to the automatic collector, whose quarter-of-the-heap rule keeps full collections rare. A miss past the limit still pauses collection and restores the caller's setting. The tests' spy now records each collection's generation and asserts a full one, so gc.collect(1) no longer passes. New tests pin where the last forced collection and the first skipped one fall for several limits, that a miss past the limit still pauses and restores, and that the count covers every baseline's entries; the Hypothesis cache test checks at most the limit's number of forced collections per sequence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The padded-template model, sized to policyengine-us's proportions (a copy adds about 17.3 million tracked objects to a heap of 3.9 million), gave the least collection CPU over 16 and 32 misses with a limit of 3; limits from 1 to 8 came within 20%, and no limit cost about three times as much. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
US + core hub merge audit, core#643 at
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PolicyEngine-US's Contrib
other-shard-1job runs under a 60-minute limit. On 2026-10-30 it timed out twice on a PR that changed nothing in the shard: its YAML step took 43:43 and 42:50, against 33:39 on a passing run. PolicyEngine/policyengine-us#10556 has since moved the contrib SSA YAML route from that shard toother-shard-3.The YAML runner builds a full copy of the tax-benefit system for each new reform combination (
_get_tax_benefit_systeminpolicyengine_core/tools/test_runner.py), and on policyengine-us automatic garbage collection takes most of that build. On core master617673caunder Python 3.14.7, the first combination ofowners_draw.yamlset off 7 full collections in each of two runs, and collection spent 9.3 and 10.0 s of 12.1 and 13.1 s CPU builds. The copy added 15.7 million tracked objects to the 3.9 million the process held before it.Change
On a cache miss,
_get_tax_benefit_systemruns the build (baseline.clone(), the reforms and the extensions) inside_automatic_gc_paused(collect):gc.isenabled()and disables automatic collection;collect, it runs onegc.collect();finally, it re-enables collection only if the caller had it on, so a build that raises, or a caller that had disabled collection, keeps the caller's setting._get_tax_benefit_systemasks forcollectonly while the cache holds fewer than_FULL_COLLECTION_CACHE_LIMIT(3) systems, counting every baseline's entries. A miss past the limit still pauses collection and restores it, then leaves the new system to the automatic collector. A cache hit returns first and touches nothing. The change uses nogc.freeze()and leavesgc.set_thresholdalone.This version adds the limit. At
569c56a7every successful miss forced a full collection, and each one walked every system the cache held, so a run of misses cost quadratic time. On the country template, 100 distinct parameter reforms took 5.9 s of CPU there against 0.07 s on its base (Python 3.14.7), with 100 full collections against none.Why a limit
A forced full collection walks every tracked object in the process, so each cached system adds to its cost. CPython's automatic collector runs a full collection only once middle collections have moved into the oldest generation at least a quarter as many objects as survived the last full one. It applies that test in
gc_select_generation(Python/gc.c, lines 1258 to 1304 at v3.14.7, and the same test at v3.13.9) and ingc_collect_generations(Modules/gcmodule.cat v3.11.15 and v3.12.14), so its full collections grow rarer as the heap grows.gc.collect()runs a full collection directly (gc_collect_implinModules/gcmodule.c, all four versions) and skips the test. It also merges the younger generations into the oldest, so the copy's objects skip the young and middle collections they would otherwise go through. That trade pays while the copy makes up a large share of the heap.Choosing 3 (a model, not a PE-US measurement)
A second PE-US combination in one process passes the 4 GiB cap, so a model sets the constant.
model_probe.pybuilds country-template systems through the real_get_tax_benefit_system. A callable reform gives each new system 2 million tracked objects (__slots__instances chained to each other), allocated inside the build, where this head pauses collection. Padding on the baseline puts 0.86 million tracked objects in the heap before the first miss, 0.43 of a copy. That doubles the one-system heap's share of a PE-US copy (3.87 million against 17.3 million before #631) to stand in for CI's two-system heap. After each miss, a case allocates 375,000 short-lived objects in chunks of 3,000, running 113 to 116 young and 10 or 11 middle collections while promoting little to the oldest generation.With a smaller pre-heap (0.3 of a copy, near the one-system heap's 0.22 to 0.25) and one miss, the model's master build ran 5 full collections and this head's 1, and master spent 4.0 times this head's collection CPU. On PE-US the same build ran 7 against 1, and master spent 3.0 to 3.3 times as much.
The table gives collection CPU over a run of misses on Python 3.14.7, from two rounds of one 32-miss process per arm, with full collections in brackets. The first n misses of a 32-miss process run the same collections as a process with n misses.
33dc9bb3569c56a74baa215c)Forcing a collection after every miss beat the automatic collector over the first four misses (0.20 s a miss against 0.29 s), broke even over misses 5 to 8 (0.36 against 0.34 s), and lost from the ninth on (0.53 against 0.33 s over misses 9 to 12). Across limits, at 16 and 32 misses:
569c56a7)A limit of 3 gave the least collection CPU at both counts. Limits from 1 to 8 came within 20% of it, the pause alone cost 26% and 37% more, and no limit cost 1.8 and 2.8 times as much. A variant with equal-size systems put two padded systems of 1.75 million objects in the heap before the first miss and ran one process per count. There too this head spent the least from 8 misses on: 7.53 s at 32 misses, against master's 11.98 s and
569c56a7's 26.28 s.Treat 3 as a practical bound, not a proven best value, because the ranking moves with the heap's shape. A variant padded with real template clones starts with about 2.2 copies of heap, against 0.43 above. Its first run favoured limits 0 to 2, but a rerun at 16 misses put limit 3 ahead of limit 0 (6.61 s against 6.93 s). Timings also move between runs: the 32-miss arm at limit 3 took 10.53 s in that rerun, with the same 11 full collections as the 8.34 s in the table. No evidence shows that 2 or 4 would do materially better on PE-US.
Invariants
Each invariant has a test in
tests/core/tools/test_runner/test_gc_pause.py, which holds 22 tests.test_yaml_cases_have_the_same_outcomes_with_and_without_the_pauseruns core'stest_with_reform.yaml,test_with_reform_2.yamlandtest_with_extension.yaml, plus three dotted-parameter cases (one failing on purpose), through the runner on fresh baselines, once with the pause and once with a null context in place of_automatic_gc_paused. The exit code, all 8 case outcomes and every computed value match, and every clone ran with collection off with the pause and on without it. Under policyengine-us, the 8 runs below and the 24 runs of the first version produced identical values and outcomes for each file, whatever the arm.gc.isenabled()after a call equals its value before. A parametrized test covers a miss, a hit, and a clone, reform or extension that raises, each with collection on and off (10 cases). Another covers a miss past the limit with collection on and off. A Hypothesis test (200 examples, up to 25 calls each) sets the caller's state at random before every call.gccall, and a new key builds a distinct system with the requested reforms and extensions._FULL_COLLECTION_CACHE_LIMITfull collections. A miss runs onegc.collect(), a full collection of the oldest generation, only when the caller had collection on, the build returned, and the cache held fewer systems than the limit before the miss, counting every baseline's entries. A hit, a failed build, a caller with collection off, and a miss past the limit run none. The tests' spy records the generation each collection asks for, sogc.collect(1)fails them. A parametrized test sets the limit to 0, 1, 2, 3 and 5. It checks that the last forced collection falls on the miss that finds one system fewer than the limit cached, and that the next three misses run none. A second test fills the cache to the limit and checks that the next miss clones with collection off, runs no collection and restores the caller's setting. A third checks that systems cached for another baseline count toward the limit until that baseline goes away. The Hypothesis test checks the rule on every call and checks that no sequence forces more collections than the limit.gc.disable()acts on the whole process, so two builds overlapping in different threads can leave collection on for a caller that entered with it off, but no production route overlaps them:YamlItem.runtestmakes the only production call, and PE-US's batch threads each start a separate subprocess.Mutants
Each mutant ran as a scratch copy against the new test file, one process each under a 2 GB cap and a 900 s timeout. All 13 died.
finallystate_after…before[{clone,reform,extension}_raises-True],…restores_the_callers_state_when_the_body_raisesstate_after…beforefor a miss and the three raising builds with collection off,…body_raises,a_miss_past_the_limit…, Hypothesis cache teststate_after…before[hit-True],a_cache_hit_leaves_collection_alone, Hypothesis cache teststate_after…before[miss-False], Hypothesis cache test…paused_then_collects_once,state_after…before[miss-True],misses_collect_only_while…[1,2,3,5],the_limit_counts…every_baseline, Hypothesis cache teststate_after…beforefor the three raising builds with collection on,…body_raises…paused_then_collects_once,…body_raises,a_miss_past_the_limit…, Hypothesis cache test, YAML differential testfinally(review)state_after…beforefor the three raising builds with collection on,…body_raisesgc.collect(1)(review)…paused_then_collects_once,state_after…before[miss-True],misses_collect_only_while…[1,2,3,5],the_limit_counts…every_baseline, Hypothesis cache test569c56a7's behaviour)misses_collect_only_while…[0,1,2,3,5],a_miss_past_the_limit…,the_limit_counts…every_baseline, Hypothesis cache test<=)misses_collect_only_while…[0,1,2,3,5],a_miss_past_the_limit…,the_limit_counts…every_baseline, Hypothesis cache test…paused_then_collects_once,state_after…before[miss-True],misses_collect_only_while…[1,2,3,5],the_limit_counts…every_baseline, Hypothesis cache testthe_limit_counts…every_baselineMeasurements
The tables report CPU time, since wall times on the measuring lane swung up to threefold between rounds of the same arm.
Template probe
The review's
sequence_probe.py 100builds 100 distincttaxes.income_tax_ratereforms on the country template and keeps every system:617673ca569c56a73464850c617673ca569c56a73464850cThis head exceeds master by 0.17 s on both versions, the cost of its three forced collections.
569c56a7predates #631; against master33dc9bb3the probe gave master 0.095 s (3.13.9) and 0.072 s (3.14.7), and4baa215c0.235 s and 0.242 s.PE-US,
owners_draw.yaml's first combination (Python 3.14.7)PE-US
mainat4eeb4773ran in its locked environment withupdate_itemization.sh's edit, through the first version's harness. Each process ran the file's first case throughpytest.mainwithOpenFiscaPluginandrun_tests' arguments. It used the systempolicyengine_usbuilds at import as the baseline (a one-system heap), and read PE-US's and core's files from a preloaded bundle until the timed build. The two pairs on each master ran their arms back to back, in opposite orders.617673ca3464850c617673ca3464850c33dc9bb34baa215c33dc9bb34baa215c33dc9bb3. Collection CPU fell 67 to 70% in every pair, and every run computed the same 14 values and passed.4baa215cdiffers from this head only in a comment and the merge of master.Shard estimate (unmeasured)
These figures stay estimates until a core release reaches the job. Rebuilding the batches with
test_batched.py's own functions at PE-US4eeb4773, with SSA inother-shard-3as #10556 routes it:other-shard-1(per-subdirectory batches, 2 workers)other-shard-3(refundable-credit files, 2 workers, then SSA files, 1 worker)The ranges assume the first version's 3.1 to 8 s saved per build: the low ends at 3.1 s with the two-worker routes fully overlapped, the high ends at 8 s with no overlap. Builds past a subprocess's third get the pause without a forced collection. In the model, misses past the third saved 17 to 19% of master's collection CPU, against 63 to 64% over the first three, so the high ends lean high. The first version's 28 subprocesses, 390 cases and 96 builds described the old routing, which ran SSA in
other-shard-1.First version's measurements (
569c56a7, PE-USbdd08b8d, Python 3.14.7)At its first combination each file's build behaves the same at this head, since the cache holds no system yet. Medians of three interleaved rounds, one-system heap:
owners_draw33dc9bb3569c56a7owners_draw_credited_earnings569c56a7owners_draw_net_earnings569c56a7One interleaved pair with CI's two-system heap gave build CPU of 20.6 s against 12.2 s. Pausing without the collection (commit
a92236561) moved the work into the next case: build plus first case took 37% and 20% more CPU than master in the same round. That version'ssummary.tsvholds 22 rows, and the two two-system results live only in their JSON files. Its 232-second import median covers the 20 one-system runs; across all 24 results the median falls to 205.7 s.What this doesn't measure
Tests and checks
pytest tests/core/tools/test_runner/test_gc_pause.py: 22 passed.pytest tests/core/tools/test_runner tests/core/test_yaml.py tests/core/test_extensions.py tests/core/test_reforms.py tests/core/test_tax_benefit_system_cache_identity.pyplus Share nested metadata values between parameter clones #631's two metadata-clone test files: 201 passed at3464850c.ruff format --check .andruff check .: clean.3464850c: all checks passed.Whichever of this PR and draft #569 lands second must wrap both of #569's build paths (its reform-free clone and its reformed build) in the pause, and count the systems its bounded cache holds (a reform-free system plus at most two reform systems, evicted oldest first) toward this limit.
impact: none on results: this PR changes when Python's collector runs, not what a simulation computes. An independent PE-US before-and-after run matched on all 14 compared values, and PE-US's own core tests passed against this head (782 passed).
axiom: n/a: core engine collection timing, with no policy change.
🤖 Generated with Claude Code