Skip to content

Commit 2a666d8

Browse files
Revert parts of the refactoring
@DarkDimius I think you have mesured the wrong with this 20% slowdown. By not sharing instances Optimistion instead of allocating new ones you where also sharing the mutable.Hashmap instances. The failure on compileStdLib is actaully a proper timeout: keeping these Hashmaps around when compiling all of stdlib prevented GC from doing it job, and (I'm guessing) spent a lot of time reshufuling the Hashmap. I also removed your `private val useFuel = false` back to a simple runtime setting check. I moved the debuging code in a `printIfDifferent` which only adds 3 extra bytecode instructions to the body of the TreeMap.transform (going from 70 to 73).
1 parent 195d5e6 commit 2a666d8

File tree

1 file changed

+26
-34
lines changed

1 file changed

+26
-34
lines changed

compiler/src/dotty/tools/dotc/transform/localopt/Simplify.scala

Lines changed: 26 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class Simplify extends MiniPhaseTransform with IdentityDenotTransformer {
3232
* The order of optimizations is tuned to converge faster.
3333
* Reordering them may require quadratically more rounds to finish.
3434
*/
35-
private val beforeErasure: List[Optimisation] =
35+
private def beforeErasure: List[Optimisation] =
3636
new InlineCaseIntrinsics ::
3737
new RemoveUnnecessaryNullChecks ::
3838
new InlineOptions ::
@@ -49,7 +49,7 @@ class Simplify extends MiniPhaseTransform with IdentityDenotTransformer {
4949
Nil
5050

5151
/** See comment on beforeErasure */
52-
private val afterErasure: List[Optimisation] =
52+
private def afterErasure: List[Optimisation] =
5353
new Valify(this) ::
5454
new Devalify ::
5555
new Jumpjump ::
@@ -67,26 +67,10 @@ class Simplify extends MiniPhaseTransform with IdentityDenotTransformer {
6767
*/
6868
var fuel: Int = -1
6969

70-
71-
/** Using fuel stops any inlining and prevents optimizations from triggering.
72-
* on my tests it gives 20% slowdown, so it is going to be disabled in public builds.
73-
*/
74-
private final val useFuel = false
75-
76-
private var optimisations: List[Optimisation] = _
77-
7870
override def prepareForUnit(tree: Tree)(implicit ctx: Context) = {
7971
val maxFuel = ctx.settings.YoptFuel.value
80-
if (!useFuel && maxFuel != ctx.settings.YoptFuel.default)
81-
ctx.error("Optimization fuel-debugging requires enabling in source, see Simplify.scala::useFuel")
8272
if (fuel < 0 && maxFuel > 0) // Both defaults are at -1
8373
fuel = maxFuel
84-
85-
optimisations = {
86-
val o = if (ctx.erasedTypes) afterErasure else beforeErasure
87-
val p = ctx.settings.YoptPhases.value
88-
if (p.isEmpty) o else o.filter(x => p.contains(x.name))
89-
}
9074
this
9175
}
9276

@@ -95,12 +79,17 @@ class Simplify extends MiniPhaseTransform with IdentityDenotTransformer {
9579
val ctx0 = ctx
9680
if (ctx.settings.optimise.value && !tree.symbol.is(Label)) {
9781
implicit val ctx: Context = ctx0.withOwner(tree.symbol(ctx0))
82+
val optimisations = {
83+
val o = if (ctx.erasedTypes) afterErasure else beforeErasure
84+
val p = ctx.settings.YoptPhases.value
85+
if (p.isEmpty) o else o.filter(x => p.contains(x.name))
86+
}
9887

9988
var rhs0 = tree.rhs
10089
var rhs1: Tree = null
10190
while (rhs1 ne rhs0) {
10291
rhs1 = rhs0
103-
// val context = ctx.withOwner(tree.symbol)
92+
val context = ctx.withOwner(tree.symbol)
10493
optimisations.foreach { optimisation => // TODO: fuse for performance
10594
// Visit
10695
rhs0.foreachSubTree(optimisation.visitor)
@@ -110,21 +99,7 @@ class Simplify extends MiniPhaseTransform with IdentityDenotTransformer {
11099
override def transform(tree: Tree)(implicit ctx: Context): Tree = {
111100
val innerCtx = if (tree.isDef && tree.symbol.exists) ctx.withOwner(tree.symbol) else ctx
112101
val childOptimizedTree = super.transform(tree)(innerCtx)
113-
114-
if (useFuel && fuel == 0)
115-
childOptimizedTree
116-
else {
117-
val fullyOptimizedTree = optimisation.transformer(ctx)(childOptimizedTree)
118-
119-
if (useFuel && (tree ne fullyOptimizedTree)) {
120-
if (fuel > 0) fuel -= 1
121-
if (fuel != -1 && fuel < 10) {
122-
println(s"${tree.symbol} was simplified by ${optimisation.name} (fuel=$fuel): ${tree.show}")
123-
println(s"became after ${optimisation.name}: (fuel=$fuel) ${fullyOptimizedTree.show}")
124-
}
125-
}
126-
fullyOptimizedTree
127-
}
102+
printIfDifferent(childOptimizedTree, optimisation.transformer(ctx)(childOptimizedTree), optimisation)
128103
}
129104
}.transform(rhs0)
130105
}
@@ -133,6 +108,23 @@ class Simplify extends MiniPhaseTransform with IdentityDenotTransformer {
133108
else tree
134109
} else tree
135110
}
111+
112+
private def printIfDifferent(tree1: Tree, tree2: => Tree, opt: Optimisation)(implicit ctx: Context): Tree = {
113+
if (fuel == -1)
114+
tree2 // Does nothing when fuel is disabled.
115+
else if (fuel == 0)
116+
tree1 // No more fuel? No more transformations for you!
117+
else { // Print the trees if different and consume fuel accordingly.
118+
if (tree1 ne tree2) {
119+
if (fuel > 0) fuel -= 1
120+
if (fuel != -1) {
121+
println(s"${tree1.symbol} was simplified by ${opt.name} (fuel=$fuel): ${tree1.show}")
122+
println(s"became after ${opt.name}: (fuel=$fuel) ${tree2.show}")
123+
}
124+
}
125+
tree2
126+
}
127+
}
136128
}
137129

138130
object Simplify {

0 commit comments

Comments
 (0)