From fad1584d1b4c518c5637eca516b4f67c9461a642 Mon Sep 17 00:00:00 2001 From: Szymon Rodziewicz Date: Wed, 1 Feb 2023 17:39:54 +0100 Subject: [PATCH 1/5] Fix static lazy field holder for GraalVM --- .../lazyvals/InitializedAccessInt.scala | 30 +++++++++++++++++++ .../lazyvals/InitializedObject.scala | 22 ++++++++++++++ .../tools/benchmarks/lazyvals/LazyVals.scala | 18 +++++++++++ .../tools/backend/jvm/BCodeSkelBuilder.scala | 2 +- .../backend/jvm/DottyBackendInterface.scala | 7 +++-- .../dotty/tools/dotc/transform/LazyVals.scala | 11 ++----- 6 files changed, 77 insertions(+), 13 deletions(-) create mode 100644 bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/InitializedAccessInt.scala create mode 100644 bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/InitializedObject.scala diff --git a/bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/InitializedAccessInt.scala b/bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/InitializedAccessInt.scala new file mode 100644 index 000000000000..2a115ad63496 --- /dev/null +++ b/bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/InitializedAccessInt.scala @@ -0,0 +1,30 @@ +package dotty.tools.benchmarks.lazyvals + +import org.openjdk.jmh.annotations.* +import org.openjdk.jmh.infra.Blackhole +import LazyVals.LazyIntHolder +import java.util.concurrent.TimeUnit + +@BenchmarkMode(Array(Mode.AverageTime)) +@Fork(2) +@Threads(1) +@Warmup(iterations = 5) +@Measurement(iterations = 5) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@State(Scope.Benchmark) +class InitializedAccessInt { + + var holder: LazyIntHolder = _ + + @Setup + def prepare: Unit = { + holder = new LazyIntHolder + holder.value + } + + @Benchmark + def measureInitialized(bh: Blackhole) = { + bh.consume(holder) + bh.consume(holder.value) + } +} diff --git a/bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/InitializedObject.scala b/bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/InitializedObject.scala new file mode 100644 index 000000000000..672cc4bf6544 --- /dev/null +++ b/bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/InitializedObject.scala @@ -0,0 +1,22 @@ +package dotty.tools.benchmarks.lazyvals + +import org.openjdk.jmh.annotations.* +import org.openjdk.jmh.infra.Blackhole +import LazyVals.ObjectHolder +import java.util.concurrent.TimeUnit + +@BenchmarkMode(Array(Mode.AverageTime)) +@Fork(2) +@Threads(1) +@Warmup(iterations = 5) +@Measurement(iterations = 5) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@State(Scope.Benchmark) +class InitializedObject { + + @Benchmark + def measureInitialized(bh: Blackhole) = { + bh.consume(ObjectHolder) + bh.consume(ObjectHolder.value) + } +} diff --git a/bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/LazyVals.scala b/bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/LazyVals.scala index 0afd93d086be..68379f9e142c 100644 --- a/bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/LazyVals.scala +++ b/bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/LazyVals.scala @@ -50,4 +50,22 @@ object LazyVals { } } } + + class LazyIntHolder { + lazy val value: Int = { + (System.nanoTime() % 1000).toInt + } + } + + object ObjectHolder { + lazy val value: String = { + System.nanoTime() % 5 match { + case 0 => "abc" + case 1 => "def" + case 2 => "ghi" + case 3 => "jkl" + case 4 => "mno" + } + } + } } diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 1885210a6687..9c1ff1f26763 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -151,7 +151,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { // !!! Part of this logic is duplicated in JSCodeGen.genCompilationUnit claszSymbol.info.decls.foreach { f => - if f.isField && !f.name.is(LazyBitMapName) then + if f.isField && !f.name.is(LazyBitMapName) && !f.name.is(LazyLocalName) then f.setFlag(JavaStatic) } diff --git a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala index ecdd0ae98803..f8f683a429f6 100644 --- a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala +++ b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala @@ -22,7 +22,7 @@ import dotty.tools.dotc.report import tpd._ import StdNames.nme -import NameKinds.LazyBitMapName +import NameKinds.{LazyBitMapName, LazyLocalName} import Names.Name class DottyBackendInterface(val outputDirectory: AbstractFile, val superCallsMap: ReadOnlyMap[Symbol, Set[ClassSymbol]])(using val ctx: Context) { @@ -129,10 +129,11 @@ object DottyBackendInterface { * the new lazy val encoding: https://github.com/lampepfl/dotty/issues/7140 */ def isStaticModuleField(using Context): Boolean = - sym.owner.isStaticModuleClass && sym.isField && !sym.name.is(LazyBitMapName) + sym.owner.isStaticModuleClass && sym.isField && !sym.name.is(LazyBitMapName) && !sym.name.is(LazyLocalName) def isStaticMember(using Context): Boolean = (sym ne NoSymbol) && - (sym.is(JavaStatic) || sym.isScalaStatic || sym.isStaticModuleField) + (sym.is(JavaStatic) || sym.isScalaStatic || sym.isStaticModuleField) + // guard against no sumbol cause this code is executed to select which call type(static\dynamic) to use to call array.clone /** diff --git a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala index 0a9a2b1214b2..0861350c30a9 100644 --- a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala +++ b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala @@ -466,13 +466,9 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer { val containerSymbol = newSymbol(claz, containerName, x.symbol.flags &~ containerFlagsMask | containerFlags | Private, defn.ObjectType, coord = x.symbol.coord).enteredAfter(this) containerSymbol.addAnnotation(Annotation(defn.VolatileAnnot, containerSymbol.span)) // private @volatile var _x: AnyRef containerSymbol.addAnnotations(x.symbol.annotations) // pass annotations from original definition - val stat = x.symbol.isStatic - if stat then - containerSymbol.setFlag(JavaStatic) + containerSymbol.removeAnnotation(defn.ScalaStaticAnnot) + containerSymbol.resetFlag(JavaStatic) val getOffset = - if stat then - Select(ref(defn.LazyValsModule), lazyNme.RLazyVals.getStaticFieldOffset) - else Select(ref(defn.LazyValsModule), lazyNme.RLazyVals.getOffsetStatic) val containerTree = ValDef(containerSymbol, nullLiteral) @@ -490,9 +486,6 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer { val offset = ref(offsetSymbol.nn) val swapOver = - if stat then - tpd.clsOf(x.symbol.owner.typeRef) - else This(claz) val (accessorDef, initMethodDef) = mkThreadSafeDef(x, claz, containerSymbol, offset, swapOver) From e466fa4541002f299bebf301e7872fcbba41a0ea Mon Sep 17 00:00:00 2001 From: Szymon Rodziewicz Date: Wed, 1 Feb 2023 18:19:19 +0100 Subject: [PATCH 2/5] No need to reset JavaStatic as its removed with the amsk --- compiler/src/dotty/tools/dotc/transform/LazyVals.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala index 0861350c30a9..8d3702190763 100644 --- a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala +++ b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala @@ -467,7 +467,6 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer { containerSymbol.addAnnotation(Annotation(defn.VolatileAnnot, containerSymbol.span)) // private @volatile var _x: AnyRef containerSymbol.addAnnotations(x.symbol.annotations) // pass annotations from original definition containerSymbol.removeAnnotation(defn.ScalaStaticAnnot) - containerSymbol.resetFlag(JavaStatic) val getOffset = Select(ref(defn.LazyValsModule), lazyNme.RLazyVals.getOffsetStatic) val containerTree = ValDef(containerSymbol, nullLiteral) From ef8e8553e6e653f95ca44a692279ead1ad7b71d8 Mon Sep 17 00:00:00 2001 From: Szymon Rodziewicz Date: Wed, 1 Feb 2023 18:25:07 +0100 Subject: [PATCH 3/5] Removing getStaticFieldOffset as it's not used anymore --- compiler/src/dotty/tools/dotc/transform/LazyVals.scala | 1 - library/src/scala/runtime/LazyVals.scala | 8 -------- 2 files changed, 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala index 8d3702190763..e4cb21a279d6 100644 --- a/compiler/src/dotty/tools/dotc/transform/LazyVals.scala +++ b/compiler/src/dotty/tools/dotc/transform/LazyVals.scala @@ -674,7 +674,6 @@ object LazyVals { val cas: TermName = N.cas.toTermName val getOffset: TermName = N.getOffset.toTermName val getOffsetStatic: TermName = "getOffsetStatic".toTermName - val getStaticFieldOffset: TermName = "getStaticFieldOffset".toTermName val getDeclaredField: TermName = "getDeclaredField".toTermName } val flag: TermName = "flag".toTermName diff --git a/library/src/scala/runtime/LazyVals.scala b/library/src/scala/runtime/LazyVals.scala index 5d1e8e74b89d..a75042671efa 100644 --- a/library/src/scala/runtime/LazyVals.scala +++ b/library/src/scala/runtime/LazyVals.scala @@ -142,14 +142,6 @@ object LazyVals { r } - def getStaticFieldOffset(field: java.lang.reflect.Field): Long = { - @nowarn - val r = unsafe.staticFieldOffset(field) - if (debug) - println(s"getStaticFieldOffset(${field.getDeclaringClass}, ${field.getName}) = $r") - r - } - def getOffsetStatic(field: java.lang.reflect.Field) = @nowarn val r = unsafe.objectFieldOffset(field) From 2bfbe7553098e050c8403557ff35a75902b8a0b7 Mon Sep 17 00:00:00 2001 From: Szymon Rodziewicz Date: Thu, 2 Feb 2023 16:45:19 +0100 Subject: [PATCH 4/5] Revert deletion of getStaticFieldOffset for now --- library/src/scala/runtime/LazyVals.scala | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/src/scala/runtime/LazyVals.scala b/library/src/scala/runtime/LazyVals.scala index a75042671efa..5d1e8e74b89d 100644 --- a/library/src/scala/runtime/LazyVals.scala +++ b/library/src/scala/runtime/LazyVals.scala @@ -142,6 +142,14 @@ object LazyVals { r } + def getStaticFieldOffset(field: java.lang.reflect.Field): Long = { + @nowarn + val r = unsafe.staticFieldOffset(field) + if (debug) + println(s"getStaticFieldOffset(${field.getDeclaringClass}, ${field.getName}) = $r") + r + } + def getOffsetStatic(field: java.lang.reflect.Field) = @nowarn val r = unsafe.objectFieldOffset(field) From 41cfb62df29b9cf893eb65034af169e0266a3632 Mon Sep 17 00:00:00 2001 From: Szymon Rodziewicz Date: Fri, 3 Feb 2023 15:21:56 +0100 Subject: [PATCH 5/5] Update printing tests to have matching AST --- .../printing/transformed/lazy-vals-new.check | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/printing/transformed/lazy-vals-new.check b/tests/printing/transformed/lazy-vals-new.check index 406417845c20..4b81cd457a38 100644 --- a/tests/printing/transformed/lazy-vals-new.check +++ b/tests/printing/transformed/lazy-vals-new.check @@ -10,19 +10,19 @@ package { @static private def (): Unit = { A.OFFSET$_m_0 = - scala.runtime.LazyVals.getStaticFieldOffset( + scala.runtime.LazyVals.getOffsetStatic( classOf[Object {...}].getDeclaredField("x$lzy1")) () } @static @static val OFFSET$_m_0: Long = - scala.runtime.LazyVals.getStaticFieldOffset( + scala.runtime.LazyVals.getOffsetStatic( classOf[Object {...}].getDeclaredField("x$lzy1")) private def writeReplace(): Object = new scala.runtime.ModuleSerializationProxy(classOf[A]) - @volatile private lazy var x$lzy1: Object = null + @volatile private lazy var x$lzy1: Object = null lazy def x(): Int = { - val result: Object = A#x$lzy1 + val result: Object = A.x$lzy1 if result.isInstanceOf[Int] then scala.Int.unbox(result) else if result.eq(scala.runtime.LazyVals.NullValue) then scala.Int.unbox(null) else scala.Int.unbox(A.x$lzyINIT1()) @@ -30,10 +30,10 @@ package { private def x$lzyINIT1(): Object = while do { - val current: Object = A#x$lzy1 + val current: Object = A.x$lzy1 if current.eq(null) then if - scala.runtime.LazyVals.objCAS(classOf[A], A.OFFSET$_m_0, null, + scala.runtime.LazyVals.objCAS(this, A.OFFSET$_m_0, null, scala.runtime.LazyVals.Evaluating) then { @@ -49,15 +49,15 @@ package { } finally if - scala.runtime.LazyVals.objCAS(classOf[A], A.OFFSET$_m_0, + scala.runtime.LazyVals.objCAS(this, A.OFFSET$_m_0, scala.runtime.LazyVals.Evaluating, result).unary_!() then { val lock: scala.runtime.LazyVals.LazyVals$Waiting = - A#x$lzy1.asInstanceOf[ + A.x$lzy1.asInstanceOf[ scala.runtime.LazyVals.LazyVals$Waiting] - scala.runtime.LazyVals.objCAS(classOf[A], A.OFFSET$_m_0, - lock, result) + scala.runtime.LazyVals.objCAS(this, A.OFFSET$_m_0, lock, + result) lock.countDown() } else () @@ -71,8 +71,8 @@ package { then if current.eq(scala.runtime.LazyVals.Evaluating) then { - scala.runtime.LazyVals.objCAS(classOf[A], A.OFFSET$_m_0, - current, new scala.runtime.LazyVals.LazyVals$Waiting()) + scala.runtime.LazyVals.objCAS(this, A.OFFSET$_m_0, current, + new scala.runtime.LazyVals.LazyVals$Waiting()) () } else