Skip to content

perf: better isDefEq cache in the presence of metavariables #8883

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

JovanGerb
Copy link
Contributor

@JovanGerb JovanGerb commented Jun 19, 2025

This PR fixes the problem that unification in the presence of metavariables is often exponentially slow. This comes up every now and then in mathlib, and could get worse as mathlib continues to grow. This is implemented by handling the defEqTrans cache more carefully: we keep the cache as long as no metavariables are assigned.

To know when to invalidate the cache, we keep track of the number of assignments inside of a MetavarContext. We also store this number in Cache.defEqTrans, so if this number doesn't match the number in the current MetavarContext, we invalidate the cache. We also revert the transient cache in checkpointDefEq when needed (instead of emptying it).

Alternatively, to identify if the MetavarContext has progressed, it would be possible to define and use PersistentHashMap.size to find the number of assignments in the MetavarContext, but I didn't go for this option.

For this to work, in unification we need to wrap a checkpointDefEq around each branch that is backtracked on if it returns false. This was already meant to be like this, but some of these checkpoints were missing. In particular the addition in line 1932 (checkpointDefEq (isDefEqProjDelta t s i)) was needed to build mathlib. I added it in other places where I deemed this necessary.

The test file shows some examples improved by this fix.

The mathilb speedup from the fix is:

File Instructions %
build -5685.569⬝10⁹ (-3.77%)
lint -593.369⬝10⁹ (-8.44%)

Only one proof in mathlib had to be fixed. That particular failure was a unification that I think should not have been expected to work, and the fix was just a small change.

Zulip#lean4 > Exponential blowup in unification with metavariables @ 💬

@JovanGerb JovanGerb marked this pull request as draft June 19, 2025 13:18
@JovanGerb JovanGerb changed the title perf(isDefEq): better cache in the presence of metavariables perf: better isDefEq cache in the presence of metavariables Jun 19, 2025
@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 Jun 19, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jun 19, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 19, 2025
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Jun 19, 2025

Mathlib CI status (docs):

@leanprover-community-bot leanprover-community-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Jun 19, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jun 21, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR 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.

2 participants