Skip to content

Commit 35044cf

Browse files
JovanGerbmichaellee94
authored andcommitted
perf(Translate): use cache in guessReorder (leanprover-community#41516)
This PR fixes a performance issue in `to_additive`/`to_dual`. The `guessReorder` function did not use a cache, meaning that it would visit the same expressions multiple times unnecessarily.
1 parent 300984f commit 35044cf

2 files changed

Lines changed: 33 additions & 25 deletions

File tree

Mathlib/Tactic/Translate/Core.lean

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -956,6 +956,8 @@ where
956956
Also try to autogenerate the `reorder` and `relevant_arg` options for this translation. -/
957957
partial def checkExistingType (t : TranslateData) (src tgt : Name) (cfg : Config) (lint := true) :
958958
MetaM (Reorder × RelevantArg) := withoutExporting do
959+
withTraceNode `translate_detail (fun _ =>
960+
return m!"checking translation `{.ofConstName src}` → `{.ofConstName tgt}`") do
959961
let srcDecl ← getConstInfo src
960962
let tgtDecl ← getConstInfo tgt
961963
unless srcDecl.numLevelParams == tgtDecl.numLevelParams do
@@ -967,7 +969,9 @@ partial def checkExistingType (t : TranslateData) (src tgt : Name) (cfg : Config
967969
srcType ← b.insertBoundaries srcType t.attrName
968970
let (srcType', relevantArg?) ← applyReplacementForall t cfg.dontTranslate srcType
969971
srcType := srcType'
970-
let reorder' ← guessReorder srcType tgtDecl.type
972+
let reorder' ← withTraceNode `translate_detail (fun _ =>
973+
return m!"guessing the reorder between `{srcType}` and `{tgtDecl.type}`") do
974+
guessReorder srcType tgtDecl.type
971975
trace[translate_detail] "The guessed reorder is {reorder'}"
972976
let reorder ←
973977
if let some reorder := cfg.reorder? then

Mathlib/Tactic/Translate/Reorder.lean

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,8 @@ partial def guessReorder (src tgt : Expr) : MetaM ArgReorder := withReducible do
274274
forallBoundedTelescope tgt depth fun tgtVars tgt ↦ do
275275
let srcMap : Std.HashMap FVarId Nat := .ofArray <| srcVars.mapIdx fun i x => (x.fvarId!, i)
276276
let tgtMap : Std.HashMap FVarId Nat := .ofArray <| tgtVars.mapIdx fun i x => (x.fvarId!, i)
277-
let perm := (visit src tgt (.replicate depth none) (srcMap, tgtMap)).elim [] decomposePerm
277+
let perm := (← visit src tgt (.replicate depth none) |>.run (srcMap, tgtMap) |>.run' {} |>.run)
278+
|>.elim [] decomposePerm
278279
-- Recursively guess the reorder in the hypotheses
279280
let mut argReorders := #[]
280281
for i in *...depth do
@@ -296,30 +297,33 @@ where
296297
/-- Determine for each `i : Fin n` to what `j : Fin n` it should get translated. -/
297298
visit (src tgt : Expr) {n : Nat} (map : Vector (Option (Fin n)) n) :
298299
ReaderT (Std.HashMap FVarId Nat × Std.HashMap FVarId Nat)
299-
Option (Vector (Option (Fin n)) n) := do
300-
match src, tgt with
301-
| .forallE _ d₁ b₁ _, .forallE _ d₂ b₂ _ => visit d₁ d₂ map >>= visit b₁ b₂
302-
| .lam _ d₁ b₁ _ , .lam _ d₂ b₂ _ => visit d₁ d₂ map >>= visit b₁ b₂
303-
| .mdata _ e₁ , .mdata _ e₂ => visit e₁ e₂ map
304-
| .letE _ t₁ v₁ b₁ _, .letE _ t₂ v₂ b₂ _ => visit t₁ t₂ map >>= visit v₁ v₂ >>= visit b₁ b₂
305-
| .app f₁ a₁ , .app f₂ a₂ => visit f₁ f₂ map >>= visit a₁ a₂
306-
| .proj _ _ e₁ , .proj _ _ e₂ => visit e₁ e₂ map
307-
| .fvar fvarId₁ , .fvar fvarId₂ =>
308-
let some i₁ := (← read).1[fvarId₁]? | some map
309-
let some i₂ := (← read).2[fvarId₂]? | some map
310-
if h : i₂ < n then
311-
if let some i₂' := map[i₁]! then
312-
guard (i₂ == i₂') -- If `i₂ ≠ i₂'`, it's not clear what `i₁` should be translated to.
313-
some map
300+
StateRefT (Std.HashSet (Expr × Expr)) (OptionT BaseIO) (Vector (Option (Fin n)) n) := do
301+
if (← get).contains (src, tgt) then return map
302+
let map ← match src, tgt with
303+
| .forallE _ d₁ b₁ _, .forallE _ d₂ b₂ _ => visit d₁ d₂ map >>= visit b₁ b₂
304+
| .lam _ d₁ b₁ _ , .lam _ d₂ b₂ _ => visit d₁ d₂ map >>= visit b₁ b₂
305+
| .mdata _ e₁ , .mdata _ e₂ => visit e₁ e₂ map
306+
| .letE _ t₁ v₁ b₁ _, .letE _ t₂ v₂ b₂ _ => visit t₁ t₂ map >>= visit v₁ v₂ >>= visit b₁ b₂
307+
| .app f₁ a₁ , .app f₂ a₂ => visit f₁ f₂ map >>= visit a₁ a₂
308+
| .proj _ _ e₁ , .proj _ _ e₂ => visit e₁ e₂ map
309+
| .fvar fvarId₁ , .fvar fvarId₂ =>
310+
let some i₁ := (← read).1[fvarId₁]? | pure map
311+
let some i₂ := (← read).2[fvarId₂]? | pure map
312+
if h : i₂ < n then
313+
if let some i₂' := map[i₁]! then
314+
guard (i₂ == i₂') -- If `i₂ ≠ i₂'`, it's not clear what `i₁` should be translated to.
315+
pure map
316+
else
317+
pure <| map.set! i₁ (some ⟨i₂, h⟩)
314318
else
315-
some <| map.set! i₁ (some ⟨i₂, h⟩)
316-
else
317-
panic! s!"index {i₂} is out of bounds ({n})"
318-
/- To avoid false positives, we do a sanity check to make sure that the two expressions are
319-
indeed of the same shape. Note that we cannot check for `e₁ == e₁`, because the universes
320-
in `e₁` and `e₂` might be different (because we decide only later whether to swap them). -/
321-
| .lit _, .lit _ | .bvar _, .bvar _ | .sort _, .sort _ | .const .., .const .. => some map
322-
| _, _ => none
319+
panic! s!"index {i₂} is out of bounds ({n})"
320+
/- To avoid false positives, we do a sanity check to make sure that the two expressions are
321+
indeed of the same shape. Note that we cannot check for `e₁ == e₁`, because the universes
322+
in `e₁` and `e₂` might be different (because we decide only later whether to swap them). -/
323+
| .lit _, .lit _ | .bvar _, .bvar _ | .sort _, .sort _ | .const .., .const .. => pure map
324+
| _, _ => failure
325+
modify (·.insert (src, tgt))
326+
return map
323327

324328
/-! ### Syntax for specifying a reorder -/
325329

0 commit comments

Comments
 (0)