From 5a42fedb33c8e6cb1e92b373f58875f11e68466c Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 19 Aug 2016 15:22:22 +1000 Subject: [PATCH] Avoid boxes in local lazies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Avoid boxing the {Object, Int, ...}Ref itself by storing it in a val, not a var - Avoid box/unbox of primitive lazy expressions due which are added to "adapt" it to the erased type of the fictional syncronized method, by using a `return`. We have to add a dummy throw after the synchronized block, but this is elimnated by the always-on DCE in the code generator. ``` ⚡ qscalac -Xprint:mixin $(f "class C { def foo = { lazy val x = 42; x }}"); javap -private -c -cp . C [[syntax trees at end of mixin]] // a.scala package { class C extends Object { def foo(): Int = { lazy val x$lzy: scala.runtime.LazyInt = new scala.runtime.LazyInt(); C.this.x$1(x$lzy) }; final private[this] def x$1(x$lzy$1: scala.runtime.LazyInt): Int = { x$lzy$1.synchronized({ if (x$lzy$1.initialized().unary_!()) { x$lzy$1.initialized_=(true); x$lzy$1.value_=(42) }; return x$lzy$1.value() }); throw null }; def (): C = { C.super.(); () } } } Compiled from "a.scala" public class C { public int foo(); Code: 0: new #12 // class scala/runtime/LazyInt 3: dup 4: invokespecial #16 // Method scala/runtime/LazyInt."":()V 7: astore_1 8: aload_1 9: invokestatic #20 // Method x$1:(Lscala/runtime/LazyInt;)I 12: ireturn private static final int x$1(scala.runtime.LazyInt); Code: 0: aload_0 1: dup 2: astore_1 3: monitorenter 4: aload_0 5: invokevirtual #31 // Method scala/runtime/LazyInt.initialized:()Z 8: ifne 22 11: aload_0 12: iconst_1 13: invokevirtual #35 // Method scala/runtime/LazyInt.initialized_$eq:(Z)V 16: aload_0 17: bipush 42 19: invokevirtual #39 // Method scala/runtime/LazyInt.value_$eq:(I)V 22: aload_0 23: invokevirtual #42 // Method scala/runtime/LazyInt.value:()I 26: istore_2 27: goto 33 30: aload_1 31: monitorexit 32: athrow 33: aload_1 34: monitorexit 35: iload_2 36: ireturn Exception table: from to target type 4 30 30 Class java/lang/Throwable public C(); Code: 0: aload_0 1: invokespecial #43 // Method java/lang/Object."":()V 4: return } ``` --- src/compiler/scala/tools/nsc/transform/Fields.scala | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/compiler/scala/tools/nsc/transform/Fields.scala b/src/compiler/scala/tools/nsc/transform/Fields.scala index 8ae43b54d83e..73882ca8da0f 100644 --- a/src/compiler/scala/tools/nsc/transform/Fields.scala +++ b/src/compiler/scala/tools/nsc/transform/Fields.scala @@ -515,7 +515,7 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor val refClass = lazyHolders.getOrElse(lazyValType.typeSymbol, LazyRefClass) val refTpe = if (refClass != LazyRefClass) refClass.tpe else appliedType(refClass.typeConstructor, List(lazyValType)) - val flags = (lazyVal.flags & FieldFlags | ARTIFACT | MUTABLE) & ~(IMPLICIT | STABLE) // TODO: why include MUTABLE??? + val flags = (lazyVal.flags & FieldFlags | ARTIFACT) & ~(IMPLICIT | STABLE) val name = lazyVal.name.toTermName.append(nme.LAZY_LOCAL_SUFFIX_STRING) val holderSym = lazyVal.owner.newValue(name, lazyVal.pos, flags) setInfo refTpe @@ -529,15 +529,16 @@ abstract class Fields extends InfoTransform with ast.TreeDSL with TypingTransfor val valueSetter = if (isUnit) NoSymbol else valueGetter.setterIn(refClass) val setValue = if (isUnit) rhs else Apply(Select(Ident(holderSym), valueSetter), rhs :: Nil) val getValue = if (isUnit) UNIT else Apply(Select(Ident(holderSym), valueGetter), Nil) - gen.mkSynchronized(Ident(holderSym), + val syncBlock = gen.mkSynchronized(Ident(holderSym), Block(List( If(NOT(Ident(holderSym) DOT initializedGetter), Block(List( setInitialized), setValue), EmptyTree)), - getValue)) // must read the value within the synchronized block since it's not volatile + Return(getValue))) // must read the value within the synchronized block since it's not volatile // (there's no happens-before relation with the read of the volatile initialized field) + Block(syncBlock :: Nil, Throw(Literal(Constant(null)))) // TODO: double-checked locking }