From f47c720d935042de7d746869f3c91b37c802b939 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Wed, 31 Oct 2018 17:37:47 +0100 Subject: [PATCH 1/2] Fix #5340: Improve runtime implementation of local lazy vals Reduce the amount of code generated for local lazy val by moving some of the generated code to the runtime library. This is a port of scala/scala#5374 --- .../dotty/tools/dotc/core/Definitions.scala | 19 +++- .../dotty/tools/dotc/transform/LazyVals.scala | 94 ++++++++++--------- .../backend/jvm/DottyBytecodeTests.scala | 22 +++++ library/src/dotty/runtime/LazyHolders.scala | 44 --------- tests/run/i5340.check | 8 ++ tests/run/i5340.scala | 23 +++++ 6 files changed, 121 insertions(+), 89 deletions(-) delete mode 100644 library/src/dotty/runtime/LazyHolders.scala create mode 100644 tests/run/i5340.check create mode 100644 tests/run/i5340.scala diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index d492b8ab6ed1..1a7dc1b4ff21 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -897,10 +897,25 @@ class Definitions { // ----- Symbol sets --------------------------------------------------- lazy val AbstractFunctionType: Array[TypeRef] = mkArityArray("scala.runtime.AbstractFunction", MaxImplementedFunctionArity, 0) - val AbstractFunctionClassPerRun: PerRun[Array[Symbol]] = new PerRun[Array[Symbol]](implicit ctx => AbstractFunctionType.map(_.symbol.asClass)) + val AbstractFunctionClassPerRun: PerRun[Array[Symbol]] = new PerRun(implicit ctx => AbstractFunctionType.map(_.symbol.asClass)) def AbstractFunctionClass(n: Int)(implicit ctx: Context): Symbol = AbstractFunctionClassPerRun()(ctx)(n) private lazy val ImplementedFunctionType = mkArityArray("scala.Function", MaxImplementedFunctionArity, 0) - def FunctionClassPerRun: PerRun[Array[Symbol]] = new PerRun[Array[Symbol]](implicit ctx => ImplementedFunctionType.map(_.symbol.asClass)) + def FunctionClassPerRun: PerRun[Array[Symbol]] = new PerRun(implicit ctx => ImplementedFunctionType.map(_.symbol.asClass)) + + val LazyHolder: PerRun[Map[Symbol, Symbol]] = new PerRun({ implicit ctx => + def holderImpl(holderType: String) = ctx.requiredClass("scala.runtime." + holderType) + Map[Symbol, Symbol]( + IntClass -> holderImpl("LazyInt"), + LongClass -> holderImpl("LazyLong"), + BooleanClass -> holderImpl("LazyBoolean"), + FloatClass -> holderImpl("LazyFloat"), + DoubleClass -> holderImpl("LazyDouble"), + ByteClass -> holderImpl("LazyByte"), + CharClass -> holderImpl("LazyChar"), + ShortClass -> holderImpl("LazyShort") + ) + .withDefaultValue(holderImpl("LazyRef")) + }) lazy val TupleType: Array[TypeRef] = mkArityArray("scala.Tuple", MaxTupleArity, 1) diff --git a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala index 87d35963dd40..c4b027a37eaa 100644 --- a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala +++ b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala @@ -129,50 +129,57 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer { Thicket(field, getter) } - /** Replace a local lazy val inside a method, - * with a LazyHolder from - * dotty.runtime(eg dotty.runtime.LazyInt) - */ + /** Desugar a local `lazy val x: Int = ` into: + * + * ``` + * val x$lzy = new scala.runtime.LazyInt() + * + * def x$lzycompute(): Int = x$lzy.synchronized { + * if (x$lzy.initialized()) x$lzy.value() + * else x$lzy.initialize() + * // TODO: Implement Unit-typed lazy val optimization described below + * // for a Unit-typed lazy val, this becomes `{ rhs ; x$lzy.initialize() }` + * // to avoid passing around BoxedUnit + * } + * + * def x(): Int = if (x$lzy.initialized()) x$lzy.value() else x$lzycompute() + * ``` + */ def transformLocalDef(x: ValOrDefDef)(implicit ctx: Context): Thicket = { - val valueInitter = x.rhs - val xname = x.name.asTermName - val holderName = LazyLocalName.fresh(xname) - val initName = LazyLocalInitName.fresh(xname) - val tpe = x.tpe.widen.resultType.widen - - val holderType = - if (tpe isRef defn.IntClass) "LazyInt" - else if (tpe isRef defn.LongClass) "LazyLong" - else if (tpe isRef defn.BooleanClass) "LazyBoolean" - else if (tpe isRef defn.FloatClass) "LazyFloat" - else if (tpe isRef defn.DoubleClass) "LazyDouble" - else if (tpe isRef defn.ByteClass) "LazyByte" - else if (tpe isRef defn.CharClass) "LazyChar" - else if (tpe isRef defn.ShortClass) "LazyShort" - else "LazyRef" - - - val holderImpl = ctx.requiredClass("dotty.runtime." + holderType) - - val holderSymbol = ctx.newSymbol(x.symbol.owner, holderName, containerFlags, holderImpl.typeRef, coord = x.pos) - val initSymbol = ctx.newSymbol(x.symbol.owner, initName, initFlags, MethodType(Nil, tpe), coord = x.pos) - val result = ref(holderSymbol).select(lazyNme.value).withPos(x.pos) - val flag = ref(holderSymbol).select(lazyNme.initialized) - val initer = valueInitter.changeOwnerAfter(x.symbol, initSymbol, this) - val initBody = - adaptToType( - ref(holderSymbol).select(defn.Object_synchronized).appliedTo( - adaptToType(mkNonThreadSafeDef(result, flag, initer, nullables = Nil), defn.ObjectType)), - tpe) - val initTree = DefDef(initSymbol, initBody) - val holderTree = ValDef(holderSymbol, New(holderImpl.typeRef, List())) - val methodBody = tpd.If(flag.ensureApplied, - result.ensureApplied, - ref(initSymbol).ensureApplied).ensureConforms(tpe) - - val methodTree = DefDef(x.symbol.asTerm, methodBody) - ctx.debuglog(s"found a lazy val ${x.show},\nrewrote with ${holderTree.show}") - Thicket(holderTree, initTree, methodTree) + val xname = x.name.asTermName + val tpe = x.tpe.widen.resultType.widen + + // val x$lzy = new scala.runtime.LazyInt() + val holderName = LazyLocalName.fresh(xname) + val holderImpl = defn.LazyHolder()(ctx)(tpe.typeSymbol) + val holderSymbol = ctx.newSymbol(x.symbol.owner, holderName, containerFlags, holderImpl.typeRef, coord = x.pos) + val holderTree = ValDef(holderSymbol, New(holderImpl.typeRef, Nil)) + + val holderRef = ref(holderSymbol) + val getValue = holderRef.select(lazyNme.value).ensureApplied.withPos(x.pos) + val initialized = holderRef.select(lazyNme.initialized).ensureApplied + + // def x$lzycompute(): Int = x$lzy.synchronized { + // if (x$lzy.initialized()) x$lzy.value() + // else x$lzy.initialize() + // } + val initName = LazyLocalInitName.fresh(xname) + val initSymbol = ctx.newSymbol(x.symbol.owner, initName, initFlags, MethodType(Nil, tpe), coord = x.pos) + val rhs = x.rhs.changeOwnerAfter(x.symbol, initSymbol, this) + val initialize = holderRef.select(lazyNme.initialize).appliedTo(rhs) + val initBody = + adaptToType( + holderRef.select(defn.Object_synchronized).appliedTo( + adaptToType(If(initialized, getValue, initialize), defn.ObjectType)), + tpe) + val initTree = DefDef(initSymbol, initBody) + + // def x(): Int = if (x$lzy.initialized()) x$lzy.value() else x$lzycompute() + val accessorBody = If(initialized, getValue, ref(initSymbol).ensureApplied).ensureConforms(tpe) + val accessor = DefDef(x.symbol.asTerm, accessorBody) + + ctx.debuglog(s"found a lazy val ${x.show},\nrewrote with ${holderTree.show}") + Thicket(holderTree, initTree, accessor) } @@ -458,6 +465,7 @@ object LazyVals { val result: TermName = "result".toTermName val value: TermName = "value".toTermName val initialized: TermName = "initialized".toTermName + val initialize: TermName = "initialize".toTermName val retry: TermName = "retry".toTermName } } diff --git a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala index 8c8d343f6f74..2e4502c47fa5 100644 --- a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala +++ b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala @@ -500,4 +500,26 @@ class TestBCode extends DottyBytecodeTest { liftReceiver) } } + + /** Test that the size of the lazy val initialiazer is under a certain threshold + * + * - Fix to #5340 reduced the size from 39 instructions to 34 + */ + @Test def i5340 = { + val source = + """class Test { + | def test = { + | lazy val x = 1 + | x + | } + |} + """.stripMargin + + checkBCode(source) { dir => + val clsIn = dir.lookupName("Test.class", directory = false).input + val clsNode = loadClassNode(clsIn) + val method = getMethod(clsNode, "x$lzyINIT1$1") + assertEquals(34, instructionsFromMethod(method).size) + } + } } diff --git a/library/src/dotty/runtime/LazyHolders.scala b/library/src/dotty/runtime/LazyHolders.scala deleted file mode 100644 index 1e31cda666ca..000000000000 --- a/library/src/dotty/runtime/LazyHolders.scala +++ /dev/null @@ -1,44 +0,0 @@ -package dotty.runtime - -/** - * Classes used as holders for local lazy vals - */ -class LazyInt { - var value: Int = _ - @volatile var initialized: Boolean = false -} - -class LazyLong { - var value: Long = _ - @volatile var initialized: Boolean = false -} - -class LazyBoolean { - var value: Boolean = _ - @volatile var initialized: Boolean = false -} - -class LazyDouble { - var value: Double = _ - @volatile var initialized: Boolean = false -} - -class LazyByte { - var value: Byte = _ - @volatile var initialized: Boolean = false -} - -class LazyRef { - var value: AnyRef = _ - @volatile var initialized: Boolean = false -} - -class LazyShort { - var value: Short = _ - @volatile var initialized: Boolean = false -} - -class LazyChar { - var value: Char = _ - @volatile var initialized: Boolean = false -} diff --git a/tests/run/i5340.check b/tests/run/i5340.check new file mode 100644 index 000000000000..ceefb4e58aec --- /dev/null +++ b/tests/run/i5340.check @@ -0,0 +1,8 @@ +Int +Long +Float +Double +Byte +Char +Short +String diff --git a/tests/run/i5340.scala b/tests/run/i5340.scala new file mode 100644 index 000000000000..4de264ce1f87 --- /dev/null +++ b/tests/run/i5340.scala @@ -0,0 +1,23 @@ +object Test { + def main(args: Array[String]): Unit = { + lazy val a: Int = { println("Int"); 1 } + lazy val b: Long = { println("Long"); 1L } + lazy val c: Float = { println("Float"); 1F } + lazy val d: Double = { println("Double"); 1.0 } + lazy val e: Byte = { println("Byte"); 1 } + lazy val f: Char = { println("Char"); '1' } + lazy val g: Short = { println("Short"); 1 } + lazy val h: String = { println("String"); "1" } + lazy val i: Unit = { println("Unit"); () } + + a + b + c + d + e + f + g + h + // i // FIXME: See #5350 + } +} From 3d33baef216d0d23a957306fbb59b4bb6fee8da3 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Thu, 1 Nov 2018 16:50:25 +0100 Subject: [PATCH 2/2] Fix #505: Remove unnecessary boxing of synchronized block `synchronized` is now treated as a magic method by the compiler and is no longer erased (similarly to `asInstanceOf` and `isInstanceOf`). Backend is updated to handle this new magic polymorphic method. --- .../backend/jvm/DottyBackendInterface.scala | 1 + .../dotty/tools/dotc/core/Definitions.scala | 2 +- .../dotty/tools/dotc/transform/LazyVals.scala | 9 +++---- .../backend/jvm/DottyBytecodeTests.scala | 26 ++++++++++++++++++- scala-backend | 2 +- tests/run/i505.scala | 15 +++++++++++ 6 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 tests/run/i505.scala diff --git a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala index b84814ea7a29..b0c930cdbb51 100644 --- a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala +++ b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala @@ -116,6 +116,7 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma val Throwable_Type: Type = defn.ThrowableType val Object_isInstanceOf: Symbol = defn.Any_isInstanceOf val Object_asInstanceOf: Symbol = defn.Any_asInstanceOf + val Object_synchronized: Symbol = defn.Object_synchronized val Object_equals: Symbol = defn.Any_equals val ArrayClass: Symbol = defn.ArrayClass val UnitClass: Symbol = defn.UnitClass diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 1a7dc1b4ff21..16009425a725 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -1074,7 +1074,7 @@ class Definitions { lazy val NoInitClasses: Set[Symbol] = NotRuntimeClasses + FunctionXXLClass def isPolymorphicAfterErasure(sym: Symbol): Boolean = - (sym eq Any_isInstanceOf) || (sym eq Any_asInstanceOf) + (sym eq Any_isInstanceOf) || (sym eq Any_asInstanceOf) || (sym eq Object_synchronized) def isTupleType(tp: Type)(implicit ctx: Context): Boolean = { val arity = tp.dealias.argInfos.length diff --git a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala index c4b027a37eaa..fa6243e865ac 100644 --- a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala +++ b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala @@ -167,11 +167,10 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer { val initSymbol = ctx.newSymbol(x.symbol.owner, initName, initFlags, MethodType(Nil, tpe), coord = x.pos) val rhs = x.rhs.changeOwnerAfter(x.symbol, initSymbol, this) val initialize = holderRef.select(lazyNme.initialize).appliedTo(rhs) - val initBody = - adaptToType( - holderRef.select(defn.Object_synchronized).appliedTo( - adaptToType(If(initialized, getValue, initialize), defn.ObjectType)), - tpe) + val initBody = holderRef + .select(defn.Object_synchronized) + .appliedToType(tpe) + .appliedTo(If(initialized, getValue, initialize).ensureConforms(tpe)) val initTree = DefDef(initSymbol, initBody) // def x(): Int = if (x$lzy.initialized()) x$lzy.value() else x$lzycompute() diff --git a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala index 2e4502c47fa5..d31f37cc32cd 100644 --- a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala +++ b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala @@ -504,6 +504,7 @@ class TestBCode extends DottyBytecodeTest { /** Test that the size of the lazy val initialiazer is under a certain threshold * * - Fix to #5340 reduced the size from 39 instructions to 34 + * - Fix to #505 reduced the size from 34 instructions to 32 */ @Test def i5340 = { val source = @@ -519,7 +520,30 @@ class TestBCode extends DottyBytecodeTest { val clsIn = dir.lookupName("Test.class", directory = false).input val clsNode = loadClassNode(clsIn) val method = getMethod(clsNode, "x$lzyINIT1$1") - assertEquals(34, instructionsFromMethod(method).size) + assertEquals(32, instructionsFromMethod(method).size) + } + } + + /** Test that synchronize blocks don't box */ + @Test def i505 = { + val source = + """class Test { + | def test: Int = synchronized(1) + |} + """.stripMargin + + checkBCode(source) { dir => + val clsIn = dir.lookupName("Test.class", directory = false).input + val clsNode = loadClassNode(clsIn) + val method = getMethod(clsNode, "test") + + val doBox = instructionsFromMethod(method).exists { + case Invoke(_, _, name, _, _) => + name == "boxToInteger" || name == "unboxToInt" + case _ => + false + } + assertFalse(doBox) } } } diff --git a/scala-backend b/scala-backend index 34af0ce0721a..9ef70dd9b9ee 160000 --- a/scala-backend +++ b/scala-backend @@ -1 +1 @@ -Subproject commit 34af0ce0721a2b7e19bab68c464d756ad7dc778f +Subproject commit 9ef70dd9b9eeec11cfb72dabb57c61198fa18a20 diff --git a/tests/run/i505.scala b/tests/run/i505.scala new file mode 100644 index 000000000000..90e91357ca1e --- /dev/null +++ b/tests/run/i505.scala @@ -0,0 +1,15 @@ +object Test { + def main(args: Array[String]): Unit = { + val a: Int = synchronized(1) + val b: Long = synchronized(1L) + val c: Boolean = synchronized(true) + val d: Float = synchronized(1f) + val e: Double = synchronized(1.0) + val f: Byte = synchronized(1.toByte) + val g: Char = synchronized('1') + val h: Short = synchronized(1.toShort) + val i: String = synchronized("Hello") + val j: List[Int] = synchronized(List(1)) + synchronized(()) + } +}