Skip to content

fix: add checkSystem calls to hasAssignableMVar#13219

Merged
nomeata merged 1 commit into
masterfrom
joachim/hasAssignableMVar
Mar 31, 2026
Merged

fix: add checkSystem calls to hasAssignableMVar#13219
nomeata merged 1 commit into
masterfrom
joachim/hasAssignableMVar

Conversation

@nomeata
Copy link
Copy Markdown
Collaborator

@nomeata nomeata commented Mar 31, 2026

This PR moves hasAssignableMVar, hasAssignableLevelMVar, and isLevelMVarAssignable from MetavarContext.lean to a new Lean.Meta.HasAssignableMVar module, changing them from generic [Monad m] [MonadMCtx m] functions to MetaM functions. This enables adding checkSystem calls in the recursive traversal, which ensures cancellation and heartbeat checks happen during what can be a very expensive computation.

All callers of these functions were already in MetaM, so this change is safe. The motivating case is the 4595_slowdown.lean test, where hasAssignableMVar (with PersistentHashMap.find? lookups on mctx.lDepth) was the dominant CPU cost during elaboration of category theory definitions. Without checkSystem calls, cancellation requests could be delayed by over 2 seconds.

The test file 4595_slowdown.lean gets a slightly increased maxHeartbeats limit because checkSystem now detects heartbeat exhaustion mid-traversal rather than after the function returns.

@nomeata nomeata added the changelog-language Language features and metaprograms label Mar 31, 2026
@nomeata nomeata requested a review from leodemoura as a code owner March 31, 2026 18:02
@nomeata nomeata changed the title fix: move hasAssignableMVar to MetaM and add checkSystem calls fix: add checkSystem calls to hasAssignableMVar Mar 31, 2026
@nomeata
Copy link
Copy Markdown
Collaborator Author

nomeata commented Mar 31, 2026

!radar

@leanprover-radar
Copy link
Copy Markdown

leanprover-radar commented Mar 31, 2026

Benchmark results for 512b52a against 67b6e81 are in. Significant changes detected! @nomeata

  • 🟥 build exited with code -1
  • 🟥 other exited with code -1

No significant changes detected.

This PR moves `isLevelMVarAssignable`, `hasAssignableLevelMVar`, and
`hasAssignableMVar` from `Lean.MetavarContext` (where they were generic
over `[Monad m] [MonadMCtx m]`) to a new `Lean.Meta.HasAssignableMVar`
module where they are specialized to `MetaM`. This allows adding
`checkSystem` calls in the recursive traversal, enabling cancellation
and heartbeat checking during what can be a very expensive operation on
large expressions (e.g. category theory proofs with many universe
metavariables).

All callers were already in `MetaM` or a monad extending it, so the
type change is compatible.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@nomeata nomeata force-pushed the joachim/hasAssignableMVar branch from 512b52a to 0e229d7 Compare March 31, 2026 18:35
@github-actions github-actions Bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Mar 31, 2026
@mathlib-lean-pr-testing
Copy link
Copy Markdown

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 67b6e815b989a8c0aa328eedeca9216df44cbd12 --onto 4786e082dca22873d14d2a5b9b7c8843380c6e78. You can force Mathlib CI using the force-mathlib-ci label. (2026-03-31 18:47:37)

@leanprover-bot
Copy link
Copy Markdown
Collaborator

Reference manual CI status:

  • ❗ Reference manual CI will not be attempted unless your PR branches off the nightly-with-manual branch. Try git rebase 67b6e815b989a8c0aa328eedeca9216df44cbd12 --onto cfa8c5a036d6990635c6ec50b02d0e806995cec3. You can force reference manual CI using the force-manual-ci label. (2026-03-31 18:47:38)

@nomeata
Copy link
Copy Markdown
Collaborator Author

nomeata commented Mar 31, 2026

!radar

@leanprover-radar
Copy link
Copy Markdown

leanprover-radar commented Mar 31, 2026

Benchmark results for 0e229d7 against 67b6e81 are in. There are no significant changes. @nomeata

  • build//instructions: -910.2M (-0.01%)

Small changes (8✅)

  • build/module/Init.Data.List.Sort.Impl//instructions: -53.8M (-0.40%)
  • build/module/Lean.Meta.AppBuilder//instructions: -156.8M (-1.74%) (reduced significance based on absolute threshold)
  • build/module/Lean.Meta.LevelDefEq//instructions: -79.4M (-3.31%) (reduced significance based on absolute threshold)
  • build/module/Lean.Meta.Tactic.Contradiction//instructions: -163.6M (-2.95%) (reduced significance based on absolute threshold)
  • build/module/Lean.Meta.Tactic.Grind.MatchCond//instructions: -566.4M (-3.28%) (reduced significance based on absolute threshold)
  • build/module/Lean.Meta.Tactic.Simp.Main//instructions: -329.3M (-1.52%) (reduced significance based on absolute threshold)
  • build/module/Lean.Meta.Tactic.Simp.Rewrite//instructions: -349.5M (-1.85%) (reduced significance based on absolute threshold)
  • build/module/Lean.MetavarContext//instructions: -406.7M (-3.99%) (reduced significance based on *//lines)

@nomeata nomeata added this pull request to the merge queue Mar 31, 2026
Merged via the queue into master with commit 916004b Mar 31, 2026
19 checks passed
volodeyka pushed a commit that referenced this pull request Apr 16, 2026
This PR moves `hasAssignableMVar`, `hasAssignableLevelMVar`, and
`isLevelMVarAssignable` from `MetavarContext.lean` to a new
`Lean.Meta.HasAssignableMVar` module, changing them from generic `[Monad
m] [MonadMCtx m]` functions to `MetaM` functions. This enables adding
`checkSystem` calls in the recursive traversal, which ensures
cancellation and heartbeat checks happen during what can be a very
expensive computation.

All callers of these functions were already in `MetaM`, so this change
is safe. The motivating case is the `4595_slowdown.lean` test, where
`hasAssignableMVar` (with `PersistentHashMap.find?` lookups on
`mctx.lDepth`) was the dominant CPU cost during elaboration of category
theory definitions. Without `checkSystem` calls, cancellation requests
could be delayed by over 2 seconds.

The test file `4595_slowdown.lean` gets a slightly increased
`maxHeartbeats` limit because `checkSystem` now detects heartbeat
exhaustion mid-traversal rather than after the function returns.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-language Language features and metaprograms toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants