Skip to content

Commit b318ba5

Browse files
leodemouraclaude
andauthored
perf: do not reference the input string from recursive-call annotations (#15223)
This PR fixes a quadratic slowdown when elaborating files with many recursive definitions. The `share common exprs` step of definition elaboration hashed the whole input file once per recursive call, so a file with thousands of structurally or well-founded recursive definitions could spend most of its elaboration time there. Recursive calls are annotated with their `Syntax` via `mkRecAppWithSyntax`, so that `WF` and `Structural` can report errors at the call site. Every token of that syntax carries a `SourceInfo.original` whose `leading` and `trailing` substrings share the whole input string, and identifiers additionally carry `rawVal`, another substring of it. The annotated expression is later hash-consed by `ShareCommon.shareCommon'` in `shareCommonPreDefs`, a traversal that follows every pointer and has no identity cache for strings. It therefore reached the input string once per substring and hashed it every time. The cost per definition was proportional to the file size, and the cost per file was quadratic in the number of recursive definitions. `mkRecAppWithSyntax` now stores a copy of the syntax that does not reference the input string. `SourceInfo.original` becomes `SourceInfo.synthetic` with the same range and `canonical := true`, and identifiers get a fresh copy of their raw value. All consumers of the annotation use it through `withRef`, which only needs the source range, so error positions are unchanged. The syntax tree is kept rather than reduced to a position because the `partial_fixpoint` error message prints the recursive call, and syntax produced by quotations may have no position to recover the text from. The only observable difference is that a comment written inside a recursive call no longer appears in that message. The cost of the bug was (number of recursive calls) × (size of the input string). The two experiments below vary each factor separately. The new benchmark `tests/elab_bench/share_common_rec_app.lean` generates the input as a string and elaborates it with `Lean.Elab.process`, so the source infos point at that generated string. It measures the wall-clock time of elaborating the whole input. Both rows contain the same 2000 recursive definitions; the second row appends a 4 MB comment, which is inert but makes the input string large. | Input | Before | After | Speedup | |---|---|---|---| | 2000 recursive defs, 112 KB | 5.2 s | 4.9 s | 1.06x | | 2000 recursive defs + 4 MB comment, 4.1 MB | 12.2 s | 5.2 s | 2.3x | The first row barely changes because a 112 KB file is small, so the hashing was a small part of the 5 s of ordinary elaboration. The second row shows the bug: the comment alone cost 7 s before the fix. After the fix the two rows are equal, that is, the input size no longer matters. If this annotation ever references the input string again, the second row will jump. The second experiment isolates the affected step. The numbers are the profiler's `share common exprs` time summed over all declarations of a generated file, not the whole elaboration time. | Input | Before | After | Speedup | |---|---|---|---| | 1000 recursive defs, 56 KB | 0.19 s | 0.015 s | 13x | | 8000 recursive defs, 450 KB | 6.05 s | 0.25 s | 24x | | 1000 recursive defs + 2 MB comment, 2.1 MB | 2.76 s | 0.016 s | 172x | | 8000 non-recursive `match` defs, 450 KB | 0.15 s | 0.17 s | none | After the fix the step costs about 0.02 ms per definition regardless of the input size: 1000 definitions take 0.015 s with or without the 2 MB comment, and 8000 take 0.25 s. Before the fix the same step grew with the input size: the 2 MB comment turned 0.19 s into 2.76 s for identical definitions, and going from 1000 to 8000 definitions multiplied the cost by 32 rather than 8, because the file grew with the definitions. The last row has no recursive calls, hence no annotation, and shows that this step was never slow on its own. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 6d7e348 commit b318ba5

4 files changed

Lines changed: 82 additions & 6 deletions

File tree

src/Lean/Elab/App.lean

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2270,12 +2270,11 @@ private def elabAppAux (f : Syntax) (namedArgs : Array NamedArg) (args : Array A
22702270

22712271
/--
22722272
We annotate recursive applications with their `Syntax` node to make sure we can produce error messages with
2273-
correct position information at `WF` and `Structural`.
2273+
correct position information at `WF` and `Structural`. `mkRecAppWithSyntax` stores a copy of the syntax that
2274+
does not reference the input string; otherwise, `Expr` traversals such as hash-consing would visit the whole
2275+
input string once per recursive application. The annotation must be erased before the definition is sent to
2276+
the kernel.
22742277
-/
2275-
-- TODO: It is overkill to store the whole `Syntax` object, and we have to make sure we erase it later.
2276-
-- We should store only the position information in the future.
2277-
-- Recall that we will need to have a compact way of storing position information in the future anyway, if we
2278-
-- want to support debugging information
22792278
private def annotateIfRec (stx : Syntax) (e : Expr) : TermElabM Expr := do
22802279
if (← read).saveRecAppSyntax then
22812280
let resultFn := e.getAppFn

src/Lean/Elab/RecAppSyntax.lean

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ Authors: Leonardo de Moura
66
module
77

88
prelude
9+
import Init.Data.String.Substring
910
public import Lean.Expr
1011

1112
public section
@@ -15,16 +16,42 @@ namespace Lean
1516
private def recAppKey := `_recApp
1617
private def recAppPosKey := `_recAppPos
1718

19+
private def detachSourceInfo : SourceInfo → SourceInfo
20+
| .original _ pos _ endPos => .synthetic pos endPos (canonical := true)
21+
| info => info
22+
23+
/--
24+
Copy of `stx` that does not reference the input string: every `SourceInfo.original` becomes a
25+
`SourceInfo.synthetic` with the same range, and identifiers get a fresh copy of their raw value.
26+
27+
The syntax stored at a recursive application lives inside an `Expr`, so `Expr` traversals that
28+
follow every pointer (e.g. `ShareCommon.shareCommon'`) visit it too. The `leading` and `trailing`
29+
substrings of `SourceInfo.original` and the `rawVal` of identifiers all share the whole input
30+
string, so storing `stx` itself would make such traversals visit, and hash, the input string once
31+
per substring. With thousands of recursive definitions in a file, this dominates elaboration time.
32+
33+
We keep the syntax tree instead of just its source range because error messages print the
34+
recursive call (see `PartialFixpoint`), and syntax produced by quotations may not have a position
35+
to recover the text from.
36+
-/
37+
private partial def detachSyntax : Syntax → Syntax
38+
| .node info kind args => .node (detachSourceInfo info) kind (args.map detachSyntax)
39+
| .atom info val => .atom (detachSourceInfo info) val
40+
| .ident info rawVal val pre => .ident (detachSourceInfo info) rawVal.toString.toRawSubstring val pre
41+
| .missing => .missing
42+
1843
/--
1944
We store the syntax at recursive applications to be able to generate better error messages
20-
when performing well-founded and structural recursion.
45+
when performing well-founded and structural recursion. The stored syntax is detached from the
46+
input string, see `detachSyntax`.
2147
2248
We additionally store the source position as an extra key, so that two recursive applications
2349
that are structurally identical as `Syntax` but originate from different source positions
2450
still produce distinct `MData`. Otherwise hashconsing or simplification can merge them and
2551
attribute an error to the wrong call site (issue #13444).
2652
-/
2753
def mkRecAppWithSyntax (e : Expr) (stx : Syntax) : Expr :=
54+
let stx := detachSyntax stx
2855
let m := KVMap.empty.insert recAppKey (.ofSyntax stx)
2956
let m := match stx.getPos? with
3057
| some p => m.insert recAppPosKey (.ofNat p.byteIdx)
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import Lean
2+
3+
/-!
4+
Benchmark: hash-consing of pre-definitions (`share common exprs`) on a file with many
5+
recursive definitions.
6+
7+
Recursive calls are annotated with their `Syntax`. The annotated expressions are traversed by
8+
`ShareCommon.shareCommon'`, which visits every object reachable from them. If the stored syntax
9+
kept `SourceInfo.original` infos, every one of them would point at the whole input string, and
10+
each recursive call would cost a hash of the whole file. The total would then be quadratic in
11+
the file size.
12+
13+
The input is generated as a string and elaborated with `Lean.Elab.process`, so the source infos
14+
point at the generated input, not at this file. The input ends with a large comment that makes
15+
the input much bigger than the declarations themselves. If the cost depends on the input size,
16+
the padded run is much slower than the unpadded one.
17+
-/
18+
19+
open Lean Elab
20+
21+
/--
22+
`numDefs` structurally recursive definitions on a small inductive type, followed by a comment
23+
of `padding` characters.
24+
-/
25+
def mkInput (numDefs padding : Nat) : String := Id.run do
26+
let mut s := "inductive N | Z | S (n : N)\n"
27+
for i in [0:numDefs] do
28+
s := s ++ s!"def g{i} : N → N\n | .Z => .Z\n | .S q => .S (g{i} q)\n"
29+
s := s ++ "/-\n" ++ "".pushn 'x' padding ++ "\n-/\n"
30+
return s
31+
32+
def runBench (numDefs padding : Nat) : CoreM Unit := do
33+
let input := mkInput numDefs padding
34+
let t0 ← IO.monoMsNow
35+
let (_, msgs) ← Lean.Elab.process input (← getEnv) {}
36+
let t1 ← IO.monoMsNow
37+
if msgs.hasErrors then
38+
for msg in msgs.toArray do
39+
IO.println (← msg.toString)
40+
throwError "unexpected errors"
41+
IO.println s!"measurement: defs_{numDefs}_pad_{padding} {(t1 - t0).toFloat / 1000.0} s"
42+
43+
#eval show CoreM Unit from do
44+
let bench := (← IO.getEnv "TEST_BENCH") == some "1"
45+
if bench then
46+
runBench 2000 0
47+
runBench 2000 4000000
48+
else
49+
runBench 20 100000
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
measurement: defs_20_pad_100000 ...

0 commit comments

Comments
 (0)