Skip to content

Commit 673182e

Browse files
szymon-rdszymon-rd
authored
Fix static lazy field holder for GraalVM (#16800)
The new lazy vals implementation was not working in graalvm's native image. The generated holder for the lazy value was static post-compilation. It caused the generated code to perform CAS operate on bad addresses and throw Bus Error. Issue on oracle/graalvm: oracle/graal#5863 This PR removes the static modifier from the lazy val holder and makes it accessible via `MODULE$` instead. It fixes the CAS on GraalVM and does not have a performance impact, at least judging by the benchmark I've added to this PR.
2 parents a356581 + 917534b commit 673182e

File tree

7 files changed

+88
-26
lines changed

7 files changed

+88
-26
lines changed

‎bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/InitializedAccessInt.scala

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package dotty.tools.benchmarks.lazyvals
2+
3+
import org.openjdk.jmh.annotations.*
4+
import org.openjdk.jmh.infra.Blackhole
5+
import LazyVals.LazyIntHolder
6+
import java.util.concurrent.TimeUnit
7+
8+
@BenchmarkMode(Array(Mode.AverageTime))
9+
@Fork(2)
10+
@Threads(1)
11+
@Warmup(iterations = 5)
12+
@Measurement(iterations = 5)
13+
@OutputTimeUnit(TimeUnit.NANOSECONDS)
14+
@State(Scope.Benchmark)
15+
class InitializedAccessInt {
16+
17+
var holder: LazyIntHolder = _
18+
19+
@Setup
20+
def prepare: Unit = {
21+
holder = new LazyIntHolder
22+
holder.value
23+
}
24+
25+
@Benchmark
26+
def measureInitialized(bh: Blackhole) = {
27+
bh.consume(holder)
28+
bh.consume(holder.value)
29+
}
30+
}

‎bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/InitializedObject.scala

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package dotty.tools.benchmarks.lazyvals
2+
3+
import org.openjdk.jmh.annotations.*
4+
import org.openjdk.jmh.infra.Blackhole
5+
import LazyVals.ObjectHolder
6+
import java.util.concurrent.TimeUnit
7+
8+
@BenchmarkMode(Array(Mode.AverageTime))
9+
@Fork(2)
10+
@Threads(1)
11+
@Warmup(iterations = 5)
12+
@Measurement(iterations = 5)
13+
@OutputTimeUnit(TimeUnit.NANOSECONDS)
14+
@State(Scope.Benchmark)
15+
class InitializedObject {
16+
17+
@Benchmark
18+
def measureInitialized(bh: Blackhole) = {
19+
bh.consume(ObjectHolder)
20+
bh.consume(ObjectHolder.value)
21+
}
22+
}

‎bench-micro/src/main/scala/dotty/tools/benchmarks/lazyvals/LazyVals.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,22 @@ object LazyVals {
5050
}
5151
}
5252
}
53+
54+
class LazyIntHolder {
55+
lazy val value: Int = {
56+
(System.nanoTime() % 1000).toInt
57+
}
58+
}
59+
60+
object ObjectHolder {
61+
lazy val value: String = {
62+
System.nanoTime() % 5 match {
63+
case 0 => "abc"
64+
case 1 => "def"
65+
case 2 => "ghi"
66+
case 3 => "jkl"
67+
case 4 => "mno"
68+
}
69+
}
70+
}
5371
}

‎compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ trait BCodeSkelBuilder extends BCodeHelpers {
151151

152152
// !!! Part of this logic is duplicated in JSCodeGen.genCompilationUnit
153153
claszSymbol.info.decls.foreach { f =>
154-
if f.isField && !f.name.is(LazyBitMapName) then
154+
if f.isField && !f.name.is(LazyBitMapName) && !f.name.is(LazyLocalName) then
155155
f.setFlag(JavaStatic)
156156
}
157157

‎compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import dotty.tools.dotc.report
2222
import tpd._
2323

2424
import StdNames.nme
25-
import NameKinds.LazyBitMapName
25+
import NameKinds.{LazyBitMapName, LazyLocalName}
2626
import Names.Name
2727

2828
class DottyBackendInterface(val outputDirectory: AbstractFile, val superCallsMap: ReadOnlyMap[Symbol, Set[ClassSymbol]])(using val ctx: Context) {
@@ -129,10 +129,11 @@ object DottyBackendInterface {
129129
* the new lazy val encoding: https://github.com/lampepfl/dotty/issues/7140
130130
*/
131131
def isStaticModuleField(using Context): Boolean =
132-
sym.owner.isStaticModuleClass && sym.isField && !sym.name.is(LazyBitMapName)
132+
sym.owner.isStaticModuleClass && sym.isField && !sym.name.is(LazyBitMapName) && !sym.name.is(LazyLocalName)
133133

134134
def isStaticMember(using Context): Boolean = (sym ne NoSymbol) &&
135-
(sym.is(JavaStatic) || sym.isScalaStatic || sym.isStaticModuleField)
135+
(sym.is(JavaStatic) || sym.isScalaStatic || sym.isStaticModuleField)
136+
136137
// guard against no sumbol cause this code is executed to select which call type(static\dynamic) to use to call array.clone
137138

138139
/**

‎compiler/src/dotty/tools/dotc/transform/LazyVals.scala

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -466,13 +466,8 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
466466
val containerSymbol = newSymbol(claz, containerName, x.symbol.flags &~ containerFlagsMask | containerFlags | Private, defn.ObjectType, coord = x.symbol.coord).enteredAfter(this)
467467
containerSymbol.addAnnotation(Annotation(defn.VolatileAnnot, containerSymbol.span)) // private @volatile var _x: AnyRef
468468
containerSymbol.addAnnotations(x.symbol.annotations) // pass annotations from original definition
469-
val stat = x.symbol.isStatic
470-
if stat then
471-
containerSymbol.setFlag(JavaStatic)
469+
containerSymbol.removeAnnotation(defn.ScalaStaticAnnot)
472470
val getOffset =
473-
if stat then
474-
Select(ref(defn.LazyValsModule), lazyNme.RLazyVals.getStaticFieldOffset)
475-
else
476471
Select(ref(defn.LazyValsModule), lazyNme.RLazyVals.getOffsetStatic)
477472
val containerTree = ValDef(containerSymbol, nullLiteral)
478473

@@ -490,9 +485,6 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
490485
val offset = ref(offsetSymbol.nn)
491486

492487
val swapOver =
493-
if stat then
494-
tpd.clsOf(x.symbol.owner.typeRef)
495-
else
496488
This(claz)
497489

498490
val (accessorDef, initMethodDef) = mkThreadSafeDef(x, claz, containerSymbol, offset, swapOver)
@@ -682,7 +674,6 @@ object LazyVals {
682674
val cas: TermName = N.cas.toTermName
683675
val getOffset: TermName = N.getOffset.toTermName
684676
val getOffsetStatic: TermName = "getOffsetStatic".toTermName
685-
val getStaticFieldOffset: TermName = "getStaticFieldOffset".toTermName
686677
val getDeclaredField: TermName = "getDeclaredField".toTermName
687678
}
688679
val flag: TermName = "flag".toTermName

‎tests/printing/transformed/lazy-vals-new.check

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,30 +10,30 @@ package <empty> {
1010
@static private def <clinit>(): Unit =
1111
{
1212
A.OFFSET$_m_0 =
13-
scala.runtime.LazyVals.getStaticFieldOffset(
13+
scala.runtime.LazyVals.getOffsetStatic(
1414
classOf[Object {...}].getDeclaredField("x$lzy1"))
1515
()
1616
}
1717
@static @static val OFFSET$_m_0: Long =
18-
scala.runtime.LazyVals.getStaticFieldOffset(
18+
scala.runtime.LazyVals.getOffsetStatic(
1919
classOf[Object {...}].getDeclaredField("x$lzy1"))
2020
private def writeReplace(): Object =
2121
new scala.runtime.ModuleSerializationProxy(classOf[A])
22-
@volatile private lazy <static> var x$lzy1: Object = null
22+
@volatile private lazy var x$lzy1: Object = null
2323
lazy def x(): Int =
2424
{
25-
val result: Object = A#x$lzy1
25+
val result: Object = A.x$lzy1
2626
if result.isInstanceOf[Int] then scala.Int.unbox(result) else
2727
if result.eq(scala.runtime.LazyVals.NullValue) then
2828
scala.Int.unbox(null) else scala.Int.unbox(A.x$lzyINIT1())
2929
}
3030
private def x$lzyINIT1(): Object =
3131
while <empty> do
3232
{
33-
val current: Object = A#x$lzy1
33+
val current: Object = A.x$lzy1
3434
if current.eq(null) then
3535
if
36-
scala.runtime.LazyVals.objCAS(classOf[A], A.OFFSET$_m_0, null,
36+
scala.runtime.LazyVals.objCAS(this, A.OFFSET$_m_0, null,
3737
scala.runtime.LazyVals.Evaluating)
3838
then
3939
{
@@ -49,15 +49,15 @@ package <empty> {
4949
}
5050
finally
5151
if
52-
scala.runtime.LazyVals.objCAS(classOf[A], A.OFFSET$_m_0,
52+
scala.runtime.LazyVals.objCAS(this, A.OFFSET$_m_0,
5353
scala.runtime.LazyVals.Evaluating, result).unary_!()
5454
then
5555
{
5656
val lock: scala.runtime.LazyVals.LazyVals$Waiting =
57-
A#x$lzy1.asInstanceOf[
57+
A.x$lzy1.asInstanceOf[
5858
scala.runtime.LazyVals.LazyVals$Waiting]
59-
scala.runtime.LazyVals.objCAS(classOf[A], A.OFFSET$_m_0,
60-
lock, result)
59+
scala.runtime.LazyVals.objCAS(this, A.OFFSET$_m_0, lock,
60+
result)
6161
lock.countDown()
6262
}
6363
else ()
@@ -71,8 +71,8 @@ package <empty> {
7171
then
7272
if current.eq(scala.runtime.LazyVals.Evaluating) then
7373
{
74-
scala.runtime.LazyVals.objCAS(classOf[A], A.OFFSET$_m_0,
75-
current, new scala.runtime.LazyVals.LazyVals$Waiting())
74+
scala.runtime.LazyVals.objCAS(this, A.OFFSET$_m_0, current,
75+
new scala.runtime.LazyVals.LazyVals$Waiting())
7676
()
7777
}
7878
else

0 commit comments

Comments
 (0)