diff options
| -rw-r--r-- | src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala | 3 | ||||
| -rw-r--r-- | src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala | 66 | ||||
| -rw-r--r-- | src/compiler/scala/tools/nsc/transform/Fields.scala | 10 | ||||
| -rw-r--r-- | src/compiler/scala/tools/nsc/transform/Mixin.scala | 77 | ||||
| -rw-r--r-- | src/reflect/scala/reflect/internal/TypeDebugging.scala | 2 | ||||
| -rw-r--r-- | test/files/run/sd242.scala | 13 | ||||
| -rw-r--r-- | test/files/run/t5293-map.scala | 88 | ||||
| -rw-r--r-- | test/files/run/t5293.scala | 83 | ||||
| -rw-r--r-- | test/files/run/t9946a.scala | 14 | ||||
| -rw-r--r-- | test/files/run/t9946b.scala | 12 | ||||
| -rw-r--r-- | test/files/run/t9946c.scala | 10 |
11 files changed, 132 insertions, 246 deletions
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala index 081830d61d..35ee5ba13d 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala @@ -325,8 +325,7 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) { insns.insertBefore(invocation, new InsnNode(DUP)) INVOKESPECIAL } - val isInterface = bodyOpcode == INVOKEINTERFACE - val bodyInvocation = new MethodInsnNode(bodyOpcode, lambdaBodyHandle.getOwner, lambdaBodyHandle.getName, lambdaBodyHandle.getDesc, isInterface) + val bodyInvocation = new MethodInsnNode(bodyOpcode, lambdaBodyHandle.getOwner, lambdaBodyHandle.getName, lambdaBodyHandle.getDesc, lambdaBodyHandle.isInterface) ownerMethod.instructions.insertBefore(invocation, bodyInvocation) val bodyReturnType = Type.getReturnType(lambdaBodyHandle.getDesc) diff --git a/src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala b/src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala index 120ee5c26e..a1923ead21 100644 --- a/src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala +++ b/src/compiler/scala/tools/nsc/transform/AccessorSynthesis.scala @@ -326,16 +326,7 @@ trait AccessorSynthesis extends Transform with ast.TreeDSL { * * This way the inliner should optimize the fast path because the method body is small enough. */ - def expandLazyClassMember(lazyVar: Symbol, lazyAccessor: Symbol, transformedRhs: Tree, nullables: Map[Symbol, List[Symbol]]): Tree = { - // use cast so that specialization can turn null.asInstanceOf[T] into null.asInstanceOf[Long] - def nullify(sym: Symbol) = - Select(thisRef, sym.accessedOrSelf) === gen.mkAsInstanceOf(NULL, sym.info.resultType) - - val nulls = nullables.getOrElse(lazyAccessor, Nil) map nullify - - if (nulls.nonEmpty) - log("nulling fields inside " + lazyAccessor + ": " + nulls) - + def expandLazyClassMember(lazyVar: global.Symbol, lazyAccessor: global.Symbol, transformedRhs: global.Tree): Tree = { val slowPathSym = slowPathFor(lazyAccessor) val rhsAtSlowDef = transformedRhs.changeOwner(lazyAccessor -> slowPathSym) @@ -346,7 +337,7 @@ trait AccessorSynthesis extends Transform with ast.TreeDSL { def needsInit = mkTest(lazyAccessor) val doInit = Block(List(storeRes), mkSetFlag(lazyAccessor)) // the slow part of double-checked locking (TODO: is this the most efficient pattern? https://github.come/scala/scala-dev/issues/204) - val slowPathRhs = Block(gen.mkSynchronized(thisRef)(If(needsInit, doInit, EmptyTree)) :: nulls, selectVar) + val slowPathRhs = Block(gen.mkSynchronized(thisRef)(If(needsInit, doInit, EmptyTree)) :: Nil, selectVar) // The lazy accessor delegates to the compute method if needed, otherwise just accesses the var (it was initialized previously) // `if ((bitmap&n & MASK) == 0) this.l$compute() else l$` @@ -358,59 +349,6 @@ trait AccessorSynthesis extends Transform with ast.TreeDSL { } } - /** Map lazy values to the fields they should null after initialization. */ - // TODO: fix - def lazyValNullables(clazz: Symbol, templStats: List[Tree]): Map[Symbol, List[Symbol]] = { - // if there are no lazy fields, take the fast path and save a traversal of the whole AST - if (!clazz.info.decls.exists(_.isLazy)) Map() - else { - // A map of single-use fields to the lazy value that uses them during initialization. - // Each field has to be private and defined in the enclosing class, and there must - // be exactly one lazy value using it. - // - // Such fields will be nulled after the initializer has memoized the lazy value. - val singleUseFields: Map[Symbol, List[Symbol]] = { - val usedIn = mutable.HashMap[Symbol, List[Symbol]]() withDefaultValue Nil - - object SingleUseTraverser extends Traverser { - override def traverse(tree: Tree) { - tree match { - // assignment targets don't count as a dereference -- only check the rhs - case Assign(_, rhs) => traverse(rhs) - case tree: RefTree if tree.symbol != NoSymbol => - val sym = tree.symbol - // println(s"$sym in ${sym.owner} from $currentOwner ($tree)") - if ((sym.hasAccessorFlag || (sym.isTerm && !sym.isMethod)) && sym.isPrivate && !sym.isLazy // non-lazy private field or its accessor - && !definitions.isPrimitiveValueClass(sym.tpe.resultType.typeSymbol) // primitives don't hang on to significant amounts of heap - && sym.owner == currentOwner.enclClass && !(currentOwner.isGetter && currentOwner.accessed == sym)) { - - // println("added use in: " + currentOwner + " -- " + tree) - usedIn(sym) ::= currentOwner - } - super.traverse(tree) - case _ => super.traverse(tree) - } - } - } - templStats foreach SingleUseTraverser.apply - // println("usedIn: " + usedIn) - - // only consider usages from non-transient lazy vals (SI-9365) - val singlyUsedIn = usedIn filter { case (_, member :: Nil) => member.isLazy && !member.accessed.hasAnnotation(TransientAttr) case _ => false } toMap - - // println("singlyUsedIn: " + singlyUsedIn) - singlyUsedIn - } - - val map = mutable.Map[Symbol, Set[Symbol]]() withDefaultValue Set() - // invert the map to see which fields can be nulled for each non-transient lazy val - for ((field, users) <- singleUseFields; lazyFld <- users) map(lazyFld) += field - - map.mapValues(_.toList sortBy (_.id)).toMap - } - } - - class SynthInitCheckedAccessorsIn(protected val clazz: Symbol) extends SynthCheckedAccessorsTreesInClass with CheckInitAccessorSymbolSynth { private object addInitBitsTransformer extends Transformer { private def checkedGetter(lhs: Tree)(pos: Position) = { diff --git a/src/compiler/scala/tools/nsc/transform/Fields.scala b/src/compiler/scala/tools/nsc/transform/Fields.scala index aa2ccd9788..f66e00ce1a 100644 --- a/src/compiler/scala/tools/nsc/transform/Fields.scala +++ b/src/compiler/scala/tools/nsc/transform/Fields.scala @@ -659,7 +659,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor val lazyVar = lazyVarOf(getter) val rhs = cast(Apply(selectSuper, Nil), lazyVar.info) - synthAccessorInClass.expandLazyClassMember(lazyVar, getter, rhs, Map.empty) + synthAccessorInClass.expandLazyClassMember(lazyVar, getter, rhs) } (afterOwnPhase { clazz.info.decls } toList) filter checkAndClearNeedsTrees map { @@ -715,7 +715,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor // note that `LazyAccessorTreeSynth` is pretty lightweight // (it's just a bunch of methods that all take a `clazz` parameter, which is thus stored as a field) val synthAccessorInClass = new SynthLazyAccessorsIn(currOwner) - synthAccessorInClass.expandLazyClassMember(lazyVarOf(statSym), statSym, transformedRhs, nullables.getOrElse(currOwner, Map.empty)) + synthAccessorInClass.expandLazyClassMember(lazyVarOf(statSym), statSym, transformedRhs) } // drop the val for (a) constant (pure & not-stored) and (b) not-stored (but still effectful) fields @@ -744,8 +744,6 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor if (stat.isTerm) atOwner(exprOwner)(transform(stat)) else transform(stat) - private val nullables = perRunCaches.newMap[Symbol, Map[Symbol, List[Symbol]]] - override def transformStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = { val addedStats = if (!currentOwner.isClass || currentOwner.isPackageClass) Nil @@ -756,10 +754,6 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor else thickets } - val inRealClass = currentOwner.isClass && !(currentOwner.isPackageClass || currentOwner.isTrait) - if (inRealClass) - nullables(currentOwner) = lazyValNullables(currentOwner, stats) - val newStats = stats mapConserve (if (exprOwner != currentOwner) transformTermsAtExprOwner(exprOwner) else transform) diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala index 582c51b90d..e62a12ce67 100644 --- a/src/compiler/scala/tools/nsc/transform/Mixin.scala +++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala @@ -10,6 +10,7 @@ package transform import symtab._ import Flags._ import scala.annotation.tailrec +import scala.collection.mutable abstract class Mixin extends InfoTransform with ast.TreeDSL with AccessorSynthesis { @@ -363,11 +364,13 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL with AccessorSynthes private val rootContext = erasure.NoContext.make(EmptyTree, rootMirror.RootClass, newScope) + private val nullables = mutable.AnyRefMap[Symbol, Map[Symbol, List[Symbol]]]() /** The first transform; called in a pre-order traversal at phase mixin * (that is, every node is processed before its children). * What transform does: * - For every non-trait class, add all mixed in members to the class info. + * - For every non-trait class, assign null to singly used private fields after use in lazy initialization. */ private def preTransform(tree: Tree): Tree = { val sym = tree.symbol @@ -381,12 +384,86 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL with AccessorSynthes else if (currentOwner.isTrait) publicizeTraitMethods(currentOwner) + if (!currentOwner.isTrait) + nullables(currentOwner) = lazyValNullables(currentOwner, body) + tree + case dd: DefDef if dd.symbol.name.endsWith(nme.LAZY_SLOW_SUFFIX) => + val fieldsToNull = nullables.getOrElse(sym.enclClass, Map()).getOrElse(sym, Nil) + if (fieldsToNull.isEmpty) dd + else { + deriveDefDef(dd) { + case blk@Block(stats, expr) => + assert(dd.symbol.originalOwner.isClass, dd.symbol) + def nullify(sym: Symbol) = + Select(gen.mkAttributedThis(sym.enclClass), sym.accessedOrSelf) === NULL + val stats1 = stats ::: fieldsToNull.map(nullify) + treeCopy.Block(blk, stats1, expr) + case tree => + devWarning("Unexpected tree shape in lazy slow path") + tree + } + } case _ => tree } } + /** Map lazy values to the fields they should null after initialization. */ + def lazyValNullables(clazz: Symbol, templStats: List[Tree]): Map[Symbol, List[Symbol]] = { + // if there are no lazy fields, take the fast path and save a traversal of the whole AST + if (!clazz.info.decls.exists(_.isLazy)) Map() + else { + // A map of single-use fields to the lazy value that uses them during initialization. + // Each field has to be private and defined in the enclosing class, and there must + // be exactly one lazy value using it. + // + // Such fields will be nulled after the initializer has memoized the lazy value. + val singleUseFields: Map[Symbol, List[Symbol]] = { + val usedIn = mutable.HashMap[Symbol, List[Symbol]]() withDefaultValue Nil + + object SingleUseTraverser extends Traverser { + override def traverse(tree: Tree) { + tree match { + // assignment targets don't count as a dereference -- only check the rhs + case Assign(_, rhs) => traverse(rhs) + case tree: RefTree if tree.symbol != NoSymbol => + val sym = tree.symbol + // println(s"$sym in ${sym.owner} from $currentOwner ($tree)") + if ((sym.hasAccessorFlag || (sym.isTerm && !sym.isMethod)) && sym.isPrivate && !sym.isLazy && !sym.isModule // non-lazy private field or its accessor + && !definitions.isPrimitiveValueClass(sym.tpe.resultType.typeSymbol) // primitives don't hang on to significant amounts of heap + && sym.owner == currentOwner.enclClass && !(currentOwner.isGetter && currentOwner.accessed == sym)) { + + // println("added use in: " + currentOwner + " -- " + tree) + usedIn(sym) ::= currentOwner + } + super.traverse(tree) + case _ => super.traverse(tree) + } + } + } + templStats foreach SingleUseTraverser.apply + // println("usedIn: " + usedIn) + + // only consider usages from non-transient lazy vals (SI-9365) + val singlyUsedIn = usedIn.filter { + case (_, member :: Nil) if member.name.endsWith(nme.LAZY_SLOW_SUFFIX) => + val lazyAccessor = member.owner.info.decl(member.name.stripSuffix(nme.LAZY_SLOW_SUFFIX)) + !lazyAccessor.accessedOrSelf.hasAnnotation(TransientAttr) + case _ => false + }.toMap + + // println("singlyUsedIn: " + singlyUsedIn) + singlyUsedIn + } + + val map = mutable.Map[Symbol, Set[Symbol]]() withDefaultValue Set() + // invert the map to see which fields can be nulled for each non-transient lazy val + for ((field, users) <- singleUseFields; lazyFld <- users) map(lazyFld) += field + + map.mapValues(_.toList sortBy (_.id)).toMap + } + } /** Add all new definitions to a non-trait class * diff --git a/src/reflect/scala/reflect/internal/TypeDebugging.scala b/src/reflect/scala/reflect/internal/TypeDebugging.scala index 63f897cd32..e9050b4e33 100644 --- a/src/reflect/scala/reflect/internal/TypeDebugging.scala +++ b/src/reflect/scala/reflect/internal/TypeDebugging.scala @@ -110,7 +110,7 @@ trait TypeDebugging { val hi_s = if (noPrint(hi)) "" else " <: " + ptTree(hi) lo_s + hi_s case _ if (t.symbol eq null) || (t.symbol eq NoSymbol) => to_s(t) - case _ => "" + t.symbol.tpe + case _ => "" + t.symbol.rawInfo.safeToString } def ptTypeParam(td: TypeDef): String = { val TypeDef(_, name, tparams, rhs) = td diff --git a/test/files/run/sd242.scala b/test/files/run/sd242.scala new file mode 100644 index 0000000000..acd51ec893 --- /dev/null +++ b/test/files/run/sd242.scala @@ -0,0 +1,13 @@ +trait T { + def test: Unit = { + byName("".toString) + () + } + + @inline + final def byName(action: => Unit) = action +} + +object Test extends App { + (new T {}).test +} diff --git a/test/files/run/t5293-map.scala b/test/files/run/t5293-map.scala deleted file mode 100644 index ad1bbcfe30..0000000000 --- a/test/files/run/t5293-map.scala +++ /dev/null @@ -1,88 +0,0 @@ - - - -import scala.collection.JavaConverters._ - - - -object Test extends App { - - def bench(label: String)(body: => Unit): Long = { - val start = System.nanoTime - - 0.until(10).foreach(_ => body) - - val end = System.nanoTime - - //println("%s: %s ms".format(label, (end - start) / 1000.0 / 1000.0)) - - end - start - } - - def benchJava(values: java.util.Map[Int, Int]) = { - bench("Java Map") { - val m = new java.util.HashMap[Int, Int] - - m.putAll(values) - } - } - - def benchScala(values: Iterable[(Int, Int)]) = { - bench("Scala Map") { - val m = new scala.collection.mutable.HashMap[Int, Int] - - m ++= values - } - } - - def benchScalaSorted(values: Iterable[(Int, Int)]) = { - bench("Scala Map sorted") { - val m = new scala.collection.mutable.HashMap[Int, Int] - - m ++= values.toArray.sorted - } - } - - def benchScalaPar(values: Iterable[(Int, Int)]) = { - bench("Scala ParMap") { - val m = new scala.collection.parallel.mutable.ParHashMap[Int, Int] map { x => x } - - m ++= values - } - } - - val total = 50000 - val values = (0 until total) zip (0 until total) - val map = scala.collection.mutable.HashMap.empty[Int, Int] - - map ++= values - - // warmup - for (x <- 0 until 5) { - benchJava(map.asJava) - benchScala(map) - benchScalaPar(map) - benchJava(map.asJava) - benchScala(map) - benchScalaPar(map) - } - - val javamap = benchJava(map.asJava) - val scalamap = benchScala(map) - val scalaparmap = benchScalaPar(map) - - // println(javamap) - // println(scalamap) - // println(scalaparmap) - - assert(scalamap < (javamap * 10), "scalamap: " + scalamap + " vs. javamap: " + javamap) - assert(scalaparmap < (javamap * 10), "scalaparmap: " + scalaparmap + " vs. javamap: " + javamap) -} - - - - - - - - diff --git a/test/files/run/t5293.scala b/test/files/run/t5293.scala deleted file mode 100644 index c42c967b42..0000000000 --- a/test/files/run/t5293.scala +++ /dev/null @@ -1,83 +0,0 @@ - - - -import scala.collection.JavaConverters._ - - - -object Test extends App { - - def bench(label: String)(body: => Unit): Long = { - val start = System.nanoTime - - 0.until(10).foreach(_ => body) - - val end = System.nanoTime - - //println("%s: %s ms".format(label, (end - start) / 1000.0 / 1000.0)) - - end - start - } - - def benchJava(values: java.util.Collection[Int]) = { - bench("Java Set") { - val set = new java.util.HashSet[Int] - - set.addAll(values) - } - } - - def benchScala(values: Iterable[Int]) = { - bench("Scala Set") { - val set = new scala.collection.mutable.HashSet[Int] - - set ++= values - } - } - - def benchScalaSorted(values: Iterable[Int]) = { - bench("Scala Set sorted") { - val set = new scala.collection.mutable.HashSet[Int] - - set ++= values.toArray.sorted - } - } - - def benchScalaPar(values: Iterable[Int]) = { - bench("Scala ParSet") { - val set = new scala.collection.parallel.mutable.ParHashSet[Int] map { x => x } - - set ++= values - } - } - - val values = 0 until 50000 - val set = scala.collection.mutable.HashSet.empty[Int] - - set ++= values - - // warmup - for (x <- 0 until 5) { - benchJava(set.asJava) - benchScala(set) - benchScalaPar(set) - benchJava(set.asJava) - benchScala(set) - benchScalaPar(set) - } - - val javaset = benchJava(set.asJava) - val scalaset = benchScala(set) - val scalaparset = benchScalaPar(set) - - assert(scalaset < (javaset * 8), "scalaset: " + scalaset + " vs. javaset: " + javaset) - assert(scalaparset < (javaset * 8), "scalaparset: " + scalaparset + " vs. javaset: " + javaset) -} - - - - - - - - diff --git a/test/files/run/t9946a.scala b/test/files/run/t9946a.scala new file mode 100644 index 0000000000..491fb31f7b --- /dev/null +++ b/test/files/run/t9946a.scala @@ -0,0 +1,14 @@ +package p1 { + object O { + private case class N(a: Any) + lazy val x: AnyRef = N + lazy val y: AnyRef = new { assert(N != null) } + } +} + +object Test { + def main(args: Array[String]): Unit = { + p1.O.x + p1.O.y + } +} diff --git a/test/files/run/t9946b.scala b/test/files/run/t9946b.scala new file mode 100644 index 0000000000..ac102a38f7 --- /dev/null +++ b/test/files/run/t9946b.scala @@ -0,0 +1,12 @@ +class Test(private val x: String) { + lazy val y = x.reverse +} +object Test { + def getX(t: Test) = t.x + def main(args: Array[String]): Unit = { + val t = new Test("foo") + assert(t.y == "oof", t.y) + assert(t.x == "foo", t.x) + } +} + diff --git a/test/files/run/t9946c.scala b/test/files/run/t9946c.scala new file mode 100644 index 0000000000..f9fe68d48f --- /dev/null +++ b/test/files/run/t9946c.scala @@ -0,0 +1,10 @@ +class Test(private[this] val x: String) { + lazy val y = x.reverse +} +object Test { + def main(args: Array[String]): Unit = { + val t = new Test("foo") + assert(t.y == "oof", t.y) + } +} + |