perf: cache and canonicalize lifts of substituted values in instantiateMVars - #14520
Merged
Conversation
This PR memoizes, per (value, lift amount), the `lift_loose_bvars` copies that `instantiateMVars` makes when substituting an fvar whose substitution value contains loose bvars of enclosing binders. Previously every occurrence of such an fvar at a deeper binder depth received a fresh copy of the value; with the memo, all occurrences at the same binder depth share one copy. This addresses part of #14329: hypotheses introduced via `MVarId.assert`/`intro` (as done by `MVarId.note`, `replaceLocalDecl`, `simp at h`, ...) and referenced several times produce exactly this shape, and the per-occurrence copies compound multiplicatively along chains of delayed-assigned mvars. This does not yet cover references at several *different* binder depths (copies of copies are fresh objects the memo has no key for, so they still compound); that part, which the LNSym reproduction of #14329 also needs, is addressed separately in #14514. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JrR3ks4WxS2jbDB6ooKDea
This PR makes the lift memo retain the expressions its raw-pointer keys refer to. A substitution value can be dropped when its substitution entry is popped and only lifted copies of it survive in the output; the allocator could then reuse the address, and a later lookup with the same (address, amount) key would return an unrelated expression. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JrR3ks4WxS2jbDB6ooKDea
…teMVars This PR replaces the per-(value, amount) lift memo in `instantiateMVars` with an encapsulated lift implementation whose cache lives for the whole pass: results are memoized per (node, cutoff, amount) across calls, and whole-value results are indexed back to their origin, so lifting an already-lifted value redirects to one canonical copy per (origin, total amount) instead of copying the copy. This makes the instantiation of proof terms that repeatedly reference hypotheses introduced via `MVarId.assert`/`intro` (as done by `MVarId.note`, `replaceLocalDecl`, `simp at h`, ...) linear-sized where such copies previously compounded multiplicatively, fixing the remaining part of #14329 without a fallback to the pre-fusing resolution strategy. The origin redirect is a pure sharing optimization: it fires only when the cutoff at the redirect is at most the recorded shift (in which case all loose bvars of the copy are shifted and the composition is exact); otherwise the code falls back to the structural copy, which is always correct. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JrR3ks4WxS2jbDB6ooKDea
Collaborator
Author
|
!perf |
Collaborator
Author
|
!bench |
|
Benchmark results for a769696 against 3259610 are in. No significant results found. @nomeata
Medium changes (1🟥)
Small changes (3✅)
|
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JrR3ks4WxS2jbDB6ooKDea
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JrR3ks4WxS2jbDB6ooKDea
…ift cache This PR gates the origin-redirect lookup on the node being shared: registered nodes are also cached, so their reference count is at least 2 once they are embedded anywhere, and unshared nodes can never hit the map. It also documents why the redirect must fire at nonzero cutoffs — occurrences under binders within a substituted value place their lifted copies at the corresponding depth, and skipping the redirect there reintroduces the compounding — and adds that pattern to the regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JrR3ks4WxS2jbDB6ooKDea
Collaborator
Author
|
!bench |
|
Benchmark results for c957944 against 3259610 are in. No significant results found. @nomeata
Medium changes (1🟥)
Small changes (4✅)
|
Collaborator
|
Reference manual CI status:
|
mathlib-nightly-testing Bot
pushed a commit
to leanprover-community/batteries
that referenced
this pull request
Jul 23, 2026
mathlib-nightly-testing Bot
pushed a commit
to leanprover-community/mathlib4-nightly-testing
that referenced
this pull request
Jul 23, 2026
…ains This PR adds an elaboration benchmark exercising `instantiateMVars` on chains of hypotheses introduced via `MVarId.assert`/`intro` whose proofs reference the previous hypothesis several times, from different binder depths and from inside lambdas. No existing benchmark covers this shape, which is how the #14329 regression shipped unnoticed: without the sharing of lifted substitution values the instantiated term is exponential in the chain length, and the benchmark fails to elaborate, while the benchmarked code paths of #12233 do not touch it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JrR3ks4WxS2jbDB6ooKDea
|
Mathlib CI status (docs):
|
Collaborator
Author
|
!bench mathlib |
|
Benchmark results for leanprover-community/mathlib4-nightly-testing@b63a683 against leanprover-community/mathlib4-nightly-testing@eaeb601 are in. No significant results found. @nomeata
Medium changes (1✅)
|
mathlib-nightly-testing Bot
pushed a commit
to leanprover-community/batteries
that referenced
this pull request
Jul 23, 2026
mathlib-nightly-testing Bot
pushed a commit
to leanprover-community/mathlib4-nightly-testing
that referenced
this pull request
Jul 23, 2026
Collaborator
Author
|
!bench |
|
Benchmark results for c0296ec against 3259610 are in. No significant results found. @nomeata
Medium changes (1🟥)
Small changes (4✅)
|
nomeata
marked this pull request as ready for review
July 24, 2026 11:19
robsimmons
pushed a commit
that referenced
this pull request
Jul 29, 2026
…teMVars (#14520) This PR fixes an exponential blowup (time and memory, typically surfacing as an out-of-memory failure) in `instantiateMVars` on proof terms that repeatedly reference hypotheses introduced via `MVarId.assert`/`intro` — as done by `MVarId.note`, `replaceLocalDecl`, `simp at h`, and, per step, by LNSym's `sym_n` tactic. Fixes #14329. Since #12233, `instantiateMVars` resolves delayed metavariable assignments in a single fused pass that carries an fvar substitution through the chain. The substituted values can contain loose bvars of enclosing binders, so every occurrence of such an fvar at a deeper binder depth needs the value lifted by the depth difference, and `lift_loose_bvars` materializes a fresh copy each time. Because substitution values have all enclosing substitutions already applied, they physically embed the lifted copies made for earlier hypotheses, so lifting them copies those again: with each step referencing the previous step's hypotheses k times, the copies compound to k^N nodes. In the LNSym reproduction this means ~4x growth per simulated instruction and an OOM at 12 steps. This PR routes all lifting of substitution values through a single lift implementation with a cache that lives for the whole pass: results are memoized per (node, cutoff, amount), so all occurrences of a value at the same depth — including values shared between substitution entries — share one copy, and whole-value results are indexed back to their origin, so lifting an already-lifted value redirects to one canonical copy per (origin, cumulative amount) instead of copying the copy. Together this materializes every lift at most once, which bounds the instantiated term by one copy per distinct cumulative depth and restores linear behavior on the workloads above. The origin redirect is a pure sharing optimization: it fires only when the cutoff at the redirect is at most the recorded shift — in which case all loose bvars of the copy are shifted and the composition `lift d ∘ lift s = lift (s + d)` is exact — and otherwise falls back to the structural copy, which is always correct. The implementation follows the patterns of `lift_loose_bvars` and `replace_rec_fn`: subtree pruning via the cached loose-bvar range, sharing-gated caching of interior nodes, allocation-free rebuilds via `update_*`, and retention of the expressions behind raw-pointer cache keys. Verified against the LNSym reproduction from #14329: the instantiated proof term is back to linear size (matching the v4.29 node counts), and the full 27-step popcount32 probe elaborates in about 1.2s where it previously exceeded 41GB. The two reproducer patterns are added as `tests/elab/issue14329a.lean` (occurrences at one binder depth, covered by memoization alone) and `tests/elab/issue14329b.lean` (occurrences at several binder depths, requiring the origin redirect). Instruction-count A/B on the `elab_bench` suite (same compiler, sources with and without the patch) is neutral within ±0.2%, including the benchmarks #12233 was built for (`delayed_assign`, `delayed_sharing`, `bv_decide_*`, `big_struct_dep`), and the full test suite passes against both stage 1 and stage 2 with a clean stage 2 stdlib build. The mathlib benchmarks show one significant change: `Mathlib.CategoryTheory.Limits.FilteredColimitCommutesFiniteLimit` drops by 26.2G instructions (-55%), to below its historical minimum. This is the bounded version of the same defect: the module's instruction count had regressed from ~24G to ~30G exactly when nightly-testing first picked up #12233, and further to ~47G with nightly-2026-04-16. Its two proofs (Borceux's Theorem 2.13.4) are long element-chasing scripts that interleave existential destructuring (`obtain ⟨...⟩ := IsFiltered.sup_exists ...`, adding binder depth) with hypothesis rewriting (`simp only [...] at h`, i.e. assert-introduced hypotheses), and then reference those hypotheses several times from deeper in the proof (e.g. `h` feeding four `(h j).choose*` definitions) — so the lifted copies of their large proof terms were duplicated at every level, costing a factor of about 2 on this file, where the longer mechanically generated chains of LNSym's `sym_n` in #14329 grow exponentially and fail outright. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <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.
This PR fixes an exponential blowup (time and memory, typically surfacing as an out-of-memory failure) in
instantiateMVarson proof terms that repeatedly reference hypotheses introduced viaMVarId.assert/intro— as done byMVarId.note,replaceLocalDecl,simp at h, and, per step, by LNSym'ssym_ntactic. Fixes #14329.Since #12233,
instantiateMVarsresolves delayed metavariable assignments in a single fused pass that carries an fvar substitution through the chain. The substituted values can contain loose bvars of enclosing binders, so every occurrence of such an fvar at a deeper binder depth needs the value lifted by the depth difference, andlift_loose_bvarsmaterializes a fresh copy each time. Because substitution values have all enclosing substitutions already applied, they physically embed the lifted copies made for earlier hypotheses, so lifting them copies those again: with each step referencing the previous step's hypotheses k times, the copies compound to k^N nodes. In the LNSym reproduction this means ~4x growth per simulated instruction and an OOM at 12 steps.This PR routes all lifting of substitution values through a single lift implementation with a cache that lives for the whole pass: results are memoized per (node, cutoff, amount), so all occurrences of a value at the same depth — including values shared between substitution entries — share one copy, and whole-value results are indexed back to their origin, so lifting an already-lifted value redirects to one canonical copy per (origin, cumulative amount) instead of copying the copy. Together this materializes every lift at most once, which bounds the instantiated term by one copy per distinct cumulative depth and restores linear behavior on the workloads above. The origin redirect is a pure sharing optimization: it fires only when the cutoff at the redirect is at most the recorded shift — in which case all loose bvars of the copy are shifted and the composition
lift d ∘ lift s = lift (s + d)is exact — and otherwise falls back to the structural copy, which is always correct. The implementation follows the patterns oflift_loose_bvarsandreplace_rec_fn: subtree pruning via the cached loose-bvar range, sharing-gated caching of interior nodes, allocation-free rebuilds viaupdate_*, and retention of the expressions behind raw-pointer cache keys.Verified against the LNSym reproduction from #14329: the instantiated proof term is back to linear size (matching the v4.29 node counts), and the full 27-step popcount32 probe elaborates in about 1.2s where it previously exceeded 41GB. The two reproducer patterns are added as
tests/elab/issue14329a.lean(occurrences at one binder depth, covered by memoization alone) andtests/elab/issue14329b.lean(occurrences at several binder depths, requiring the origin redirect). Instruction-count A/B on theelab_benchsuite (same compiler, sources with and without the patch) is neutral within ±0.2%, including the benchmarks #12233 was built for (delayed_assign,delayed_sharing,bv_decide_*,big_struct_dep), and the full test suite passes against both stage 1 and stage 2 with a clean stage 2 stdlib build.The mathlib benchmarks show one significant change:
Mathlib.CategoryTheory.Limits.FilteredColimitCommutesFiniteLimitdrops by 26.2G instructions (-55%), to below its historical minimum. This is the bounded version of the same defect: the module's instruction count had regressed from ~24G to ~30G exactly when nightly-testing first picked up #12233, and further to ~47G with nightly-2026-04-16. Its two proofs (Borceux's Theorem 2.13.4) are long element-chasing scripts that interleave existential destructuring (obtain ⟨...⟩ := IsFiltered.sup_exists ..., adding binder depth) with hypothesis rewriting (simp only [...] at h, i.e. assert-introduced hypotheses), and then reference those hypotheses several times from deeper in the proof (e.g.hfeeding four(h j).choose*definitions) — so the lifted copies of their large proof terms were duplicated at every level, costing a factor of about 2 on this file, where the longer mechanically generated chains of LNSym'ssym_nin #14329 grow exponentially and fail outright.🤖 Generated with Claude Code