Fix _invalidate_all_caches performance regression#478
Merged
Conversation
The 3.24.4-era implementation walked every variable in the tax-benefit system via `get_holder(variable)`, which lazy-creates a `Holder` for each one. With thousands of variables in policyengine-us, this inflated `apply_reform` from milliseconds to seconds — the YAML full-suite went from ~17 min to ~51 min per job and started timing out at the 1-hour GitHub Actions limit. Untouched variables have no holder and therefore nothing to wipe. Iterate `population._holders.values()` on each population to hit only the variables that actually exist, preserving the set_input replay behaviour from #475. Verified against tests/core/test_apply_reform_preserves_user_inputs and tests/core/test_apply_reform_invalidates_cache (all 5 pass). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MaxGhenis
added a commit
to PolicyEngine/policyengine-us
that referenced
this pull request
Apr 18, 2026
…f fix PE-core 3.25.0 regressed YAML full-suite run time (17 min -> 51+ min per job, hit 1-hour GH Actions timeout) because `_invalidate_all_caches` walked every variable in the tax-benefit system and lazy-created a Holder for each. Fixed upstream in PolicyEngine/policyengine-core#478 by iterating only existing holders, restoring the original sub-second `apply_reform` cost. 3.25.1 ships the fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
MaxGhenis
added a commit
to PolicyEngine/policyengine-us
that referenced
this pull request
Apr 18, 2026
…f fix PE-core 3.25.0 regressed YAML full-suite run time (17 min -> 51+ min per job, hit 1-hour GH Actions timeout) because `_invalidate_all_caches` walked every variable in the tax-benefit system and lazy-created a Holder for each. Fixed upstream in PolicyEngine/policyengine-core#478 by iterating only existing holders, restoring the original sub-second `apply_reform` cost. 3.25.1 ships the fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MaxGhenis
added a commit
to PolicyEngine/policyengine-us
that referenced
this pull request
Apr 19, 2026
* Bump policyengine-core to >=3.25.0 to fix state_fips wipe regression PE-core 3.24.0-3.24.3 cache-invalidation wiped set_input values across apply_reform, which in policyengine-us manifested as state_fips getting cleared and the downstream state_name/state_code chain returning None (#8058). 3.25.0 preserves user inputs while still invalidating formula-output caches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Bump policyengine-core floor to 3.25.1 for _invalidate_all_caches perf fix PE-core 3.25.0 regressed YAML full-suite run time (17 min -> 51+ min per job, hit 1-hour GH Actions timeout) because `_invalidate_all_caches` walked every variable in the tax-benefit system and lazy-created a Holder for each. Fixed upstream in PolicyEngine/policyengine-core#478 by iterating only existing holders, restoring the original sub-second `apply_reform` cost. 3.25.1 ships the fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
3.24.4 implementation of
_invalidate_all_cacheswalked every variable in the tax-benefit system viaget_holder(variable), which lazy-creates aHolderfor each one. With thousands of variables in policyengine-us, this inflatedapply_reformfrom milliseconds to seconds.Downstream impact: policyengine-us YAML full-suite went from ~17 min to ~51 min per job and started timing out at the 1-hour GitHub Actions limit on PolicyEngine/policyengine-us#8070 (pick up PE-core 3.25).
Fix
Iterate
population._holders.values()on each population instead ofself.tax_benefit_system.variables. Untouched variables have no holder and therefore nothing to wipe — skipping them matches pre-H3 behaviour and restores performance while keeping the set_input replay from #475.Test plan
tests/core/test_apply_reform_preserves_user_inputs(4/4 pass)tests/core/test_apply_reform_invalidates_cache(1/1 pass)tests/coresuite (518 pass)Generated with Claude Code