Skip to content

Commit 8f9f5f0

Browse files
authored
Make 'core.Renamer' quicker by simplifying scope handling (#940)
I don't know what the nested list is for, but we don't seem to use it anywhere. Therefore I'm removing the nesting and using mutable hashmaps: it's on a hot path! Renamer then goes from 2.4-3.4s to 0.6-0.4s :) [measured a few times via profiling] (for reference, before #938, it was somewhere between 3 and 4 seconds)
1 parent 53b4587 commit 8f9f5f0

File tree

3 files changed

+36
-23
lines changed

3 files changed

+36
-23
lines changed

effekt/jvm/src/test/scala/effekt/core/RenamerTests.scala

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,4 +176,23 @@ class RenamerTests extends CoreTests {
176176
assertRenamingPreservesAlpha(code)
177177
assertRenamingMakesDefsUnique(code)
178178
}
179+
test("shadowing let bindings inside a def") {
180+
val code =
181+
""" module main
182+
|
183+
| def main = { () =>
184+
| def foo = { () =>
185+
| let x = 1
186+
| return x: Int
187+
| }
188+
| let x = 2
189+
| def bar = { () =>
190+
| let x = 3
191+
| return x: Int
192+
| }
193+
| return x:Int
194+
| }
195+
|""".stripMargin
196+
assertRenamingPreservesAlpha(code)
197+
}
179198
}

effekt/jvm/src/test/scala/effekt/core/TestRenamer.scala

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,4 @@ class TestRenamer(names: Names = Names(Map.empty), prefix: String = "") extends
109109
suffix = 0
110110
rewrite(s)
111111
}
112-
}
113-
114-
object TestRenamer {
115-
def rename(b: Block): Block = Renamer().rewrite(b)
116-
def rename(b: BlockLit): (BlockLit, Map[Id, Id]) =
117-
val renamer = Renamer()
118-
val res = renamer.rewrite(b)
119-
(res, renamer.renamed)
120-
}
112+
}

effekt/shared/src/main/scala/effekt/core/Renamer.scala

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
package effekt.core
22

3+
import scala.collection.mutable
4+
35
import effekt.{ core, symbols }
4-
import effekt.context.Context
56

67
/**
78
* Freshens bound names in a given Core term.
@@ -16,32 +17,33 @@ import effekt.context.Context
1617
*/
1718
class Renamer(names: Names = Names(Map.empty), prefix: String = "") extends core.Tree.Rewrite {
1819

19-
// list of scopes that map bound symbols to their renamed variants.
20-
private var scopes: List[Map[Id, Id]] = List.empty
20+
// Local renamings: map of bound symbols to their renamed variants in a given scope.
21+
private var scope: Map[Id, Id] = Map.empty
2122

22-
// Here we track ALL renamings
23-
var renamed: Map[Id, Id] = Map.empty
23+
// All renamings: map of bound symbols to their renamed variants, globally!
24+
val renamed: mutable.HashMap[Id, Id] = mutable.HashMap.empty
2425

2526
def freshIdFor(id: Id): Id =
2627
if prefix.isEmpty then Id(id) else Id(id.name.rename { _current => prefix })
2728

2829
def withBindings[R](ids: List[Id])(f: => R): R =
29-
val before = scopes
30+
val scopeBefore = scope
3031
try {
31-
val newScope = ids.map { x => x -> freshIdFor(x) }.toMap
32-
scopes = newScope :: scopes
33-
renamed = renamed ++ newScope
32+
ids.foreach { x =>
33+
val fresh = freshIdFor(x)
34+
scope = scope + (x -> fresh)
35+
renamed.put(x, fresh)
36+
}
37+
3438
f
35-
} finally { scopes = before }
39+
} finally { scope = scopeBefore }
3640

3741
/** Alias for withBindings(List(id)){...} */
3842
def withBinding[R](id: Id)(f: => R): R = withBindings(List(id))(f)
3943

4044
// free variables are left untouched
4145
override def id: PartialFunction[core.Id, core.Id] = {
42-
case id => scopes.collectFirst {
43-
case bnds if bnds.contains(id) => bnds(id)
44-
}.getOrElse(id)
46+
id => scope.getOrElse(id, id)
4547
}
4648

4749
override def stmt: PartialFunction[Stmt, Stmt] = {
@@ -107,7 +109,7 @@ class Renamer(names: Names = Names(Map.empty), prefix: String = "") extends core
107109

108110
object Renamer {
109111
def rename(b: Block): Block = Renamer().rewrite(b)
110-
def rename(b: BlockLit): (BlockLit, Map[Id, Id]) =
112+
def rename(b: BlockLit): (BlockLit, mutable.HashMap[Id, Id]) =
111113
val renamer = Renamer()
112114
val res = renamer.rewrite(b)
113115
(res, renamer.renamed)

0 commit comments

Comments
 (0)