From 6fd9569c3d3e6bd2aced0e69aed07a8bec2420e5 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sat, 30 Sep 2017 15:57:00 +0200 Subject: [PATCH 1/2] Optimize hasRefinedMethod even more It turns out that this method takes time because getDeclaredMethods take time, so we avoid calling it when possible. --- .../tools/dotc/transform/TreeTransform.scala | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeTransform.scala b/compiler/src/dotty/tools/dotc/transform/TreeTransform.scala index 0111c19292c9..5bffd92da656 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeTransform.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeTransform.scala @@ -196,10 +196,12 @@ object TreeTransforms { } } - @sharable val NoTransform = new TreeTransform { + private class NoTreeTransform extends TreeTransform { def phase = unsupported("phase") } + @sharable val NoTransform: TreeTransform = new NoTreeTransform + type Mutator[T] = (TreeTransform, T, Context) => TreeTransform class TransformerInfo(val transformers: Array[TreeTransform], val nx: NXTransformations, val group: TreeTransformer) @@ -209,15 +211,29 @@ object TreeTransforms { * @see NXTransformations.index for format of plan */ class NXTransformations { + private val clsMethodsCache = new java.util.IdentityHashMap[Class[_], Array[java.lang.reflect.Method]] + // TODO: We spend too much time here. See if we can call it less or make it faster, + // e.g. by checking `cls.getMethod(name, ...).getDeclaringClass != classOf[TreeTransform]` instead. private def hasRedefinedMethod(cls: Class[_], name: String): Boolean = { - val clsMethods = cls.getDeclaredMethods + if (cls.eq(classOf[TreeTransform]) || cls.eq(classOf[NoTreeTransform]) || + cls.eq(classOf[MiniPhaseTransform])) + return false + + // Class#getDeclaredMethods is slow, so we cache its output + var clsMethods = clsMethodsCache.get(cls) + if (clsMethods eq null) { + clsMethods = cls.getDeclaredMethods + clsMethodsCache.put(cls, clsMethods) + } + var i = clsMethods.length - 1 while (i >= 0) { if (clsMethods(i).getName == name) - return cls != classOf[TreeTransform] + return true i -= 1 } + hasRedefinedMethod(cls.getSuperclass, name) } From 128afccbea4597c05f7186624e44649b623f8018 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sat, 30 Sep 2017 03:01:50 +0200 Subject: [PATCH 2/2] Micro-optimization: avoid a lazy val in a transformDefDef --- .../dotty/tools/dotc/transform/Memoize.scala | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Memoize.scala b/compiler/src/dotty/tools/dotc/transform/Memoize.scala index d9769a1886dc..733791da5a4a 100644 --- a/compiler/src/dotty/tools/dotc/transform/Memoize.scala +++ b/compiler/src/dotty/tools/dotc/transform/Memoize.scala @@ -101,19 +101,8 @@ import Decorators._ cpy.installAfter(thisTransform) } - lazy val field = sym.field.orElse(newField).asTerm - - def adaptToField(tree: Tree): Tree = - if (tree.isEmpty) tree else tree.ensureConforms(field.info.widen) - val NoFieldNeeded = Lazy | Deferred | JavaDefined | (if (ctx.settings.YnoInline.value) EmptyFlags else Inline) - def isErasableBottomField(cls: Symbol): Boolean = { - // TODO: For Scala.js, return false if this field is in a js.Object unless it is an ErasedPhantomClass. - !field.isVolatile && - ((cls eq defn.NothingClass) || (cls eq defn.NullClass) || (cls eq defn.BoxedUnitClass) || (cls eq defn.ErasedPhantomClass)) - } - def erasedBottomTree(sym: Symbol) = { if (sym eq defn.NothingClass) Throw(Literal(Constant(null))) else if (sym eq defn.NullClass) Literal(Constant(null)) @@ -125,7 +114,18 @@ import Decorators._ } } - if (sym.is(Accessor, butNot = NoFieldNeeded)) + if (sym.is(Accessor, butNot = NoFieldNeeded)) { + val field = sym.field.orElse(newField).asTerm + + def adaptToField(tree: Tree): Tree = + if (tree.isEmpty) tree else tree.ensureConforms(field.info.widen) + + def isErasableBottomField(cls: Symbol): Boolean = { + // TODO: For Scala.js, return false if this field is in a js.Object unless it is an ErasedPhantomClass. + !field.isVolatile && + ((cls eq defn.NothingClass) || (cls eq defn.NullClass) || (cls eq defn.BoxedUnitClass) || (cls eq defn.ErasedPhantomClass)) + } + if (sym.isGetter) { var rhs = tree.rhs.changeOwnerAfter(sym, field, thisTransform) if (isWildcardArg(rhs)) rhs = EmptyTree @@ -151,6 +151,7 @@ import Decorators._ } else tree // curiously, some accessors from Scala2 have ' ' suffixes. They count as // neither getters nor setters + } else tree } }