Skip to content

Commit c1f821b

Browse files
jvicanjvican
committed
Fix typesafehub#113: Implement SafeLazy in Java
`SafeLazy` has been traditionally implemented in `zincApiInfo` because it is part of the sbt API and is accessible to all the subprojects that depend on it. Before this commit, `SafeLazy` was a runtime dependency (using reflection) of the compiler bridge. In this regard, Zinc was assuming that the sbt API was accessible at runtime and therefore invoked it to use an implementation of lazy that would remove references to the thunks once they've been forced. This was done to free memory as soon as possible since those thunks usually depend on classes of compiler internals and would not be GC'ed otherwise. However, the compiler bridge is not supposed to depend on sbt APIs since its code is compiled by the Scala compiler that the user picks in SBT. Its only dependency is the compiler interface, which is implemented in Java and compiled beforehand with javac. This commit removes the runtime dependency of the compiler bridge to the sbt API and avoids the method invocations using reflection. This was done for the following reasons: * Simplicity. It is not obvious why `SafeLazy` is invoked reflectively. See sbt/zinc#113. * Performance. Even though the JVM should make this simple use of reflection fast, there's a very small overhead of using reflection in the compiler bridge because `lzy` is (most likely) hot. The fix consists of a Java implementation of `SafeLazy` that uses the non-thread-safe lazy val implementation described [here](http://docs.scala-lang.org/sips/pending/improved-lazy-val-initialization.html). It is complemented with a proxy written in Scala that will create an indirection layer for things like by-name and strict evaluation. This implementation of lazy val assumes that `SafeLazy` will never be called asynchronously. If this is the case, it's up to the Zinc maintainer to make sure that safe publishing is implemented at the call-site or to change the implementation to avoid races and uninitialized fields.
1 parent 16e1670 commit c1f821b

File tree

14 files changed

+125
-75
lines changed

14 files changed

+125
-75
lines changed

Diff for: ‎internal/compiler-bridge/src-2.10/main/scala/xsbt/ExtractAPI.scala

+9-12
Original file line numberDiff line numberDiff line change
@@ -136,18 +136,15 @@ class ExtractAPI[GlobalType <: Global](
136136
def renaming(symbol: Symbol): Option[String] = renameTo.get(symbol)
137137
}
138138

139-
// call back to the xsbti.SafeLazy class in main sbt code to construct a SafeLazy instance
140-
// we pass a thunk, whose class is loaded by the interface class loader (this class's loader)
141-
// SafeLazy ensures that once the value is forced, the thunk is nulled out and so
142-
// references to the thunk's classes are not retained. Specifically, it allows the interface classes
143-
// (those in this subproject) to be garbage collected after compilation.
144-
private[this] val safeLazy = Class.forName("xsbti.SafeLazy").getMethod("apply", classOf[xsbti.F0[_]])
145-
private def lzy[S <: AnyRef](s: => S): xsbti.api.Lazy[S] =
146-
{
147-
val z = safeLazy.invoke(null, Message(s)).asInstanceOf[xsbti.api.Lazy[S]]
148-
pending += z
149-
z
150-
}
139+
/**
140+
* Construct a lazy instance from a by-name parameter that will null out references to once
141+
* the value is forced and therefore references to thunk's classes will be garbage collected.
142+
*/
143+
private def lzy[S <: AnyRef](s: => S): xsbti.api.Lazy[S] = {
144+
val lazyImpl = xsbti.api.SafeLazy.apply(Message(s))
145+
pending += lazyImpl
146+
lazyImpl
147+
}
151148

152149
/**
153150
* Force all lazy structures. This is necessary so that we see the symbols/types at this phase and

Diff for: ‎internal/compiler-bridge/src/main/scala/xsbt/ExtractAPI.scala

+10-13
Original file line numberDiff line numberDiff line change
@@ -136,18 +136,15 @@ class ExtractAPI[GlobalType <: Global](
136136
def renaming(symbol: Symbol): Option[String] = renameTo.get(symbol)
137137
}
138138

139-
// call back to the xsbti.SafeLazy class in main sbt code to construct a SafeLazy instance
140-
// we pass a thunk, whose class is loaded by the interface class loader (this class's loader)
141-
// SafeLazy ensures that once the value is forced, the thunk is nulled out and so
142-
// references to the thunk's classes are not retained. Specifically, it allows the interface classes
143-
// (those in this subproject) to be garbage collected after compilation.
144-
private[this] val safeLazy = Class.forName("xsbti.SafeLazy").getMethod("apply", classOf[xsbti.F0[_]])
145-
private def lzy[S <: AnyRef](s: => S): xsbti.api.Lazy[S] =
146-
{
147-
val z = safeLazy.invoke(null, Message(s)).asInstanceOf[xsbti.api.Lazy[S]]
148-
pending += z
149-
z
150-
}
139+
/**
140+
* Construct a lazy instance from a by-name parameter that will null out references to once
141+
* the value is forced and therefore references to thunk's classes will be garbage collected.
142+
*/
143+
private def lzy[S <: AnyRef](s: => S): xsbti.api.Lazy[S] = {
144+
val lazyImpl = xsbti.api.SafeLazy.apply(Message(s))
145+
pending += lazyImpl
146+
lazyImpl
147+
}
151148

152149
/**
153150
* Force all lazy structures. This is necessary so that we see the symbols/types at this phase and
@@ -624,4 +621,4 @@ class ExtractAPI[GlobalType <: Global](
624621
implicit def compat(ann: AnnotationInfo): IsStatic = new IsStatic(ann)
625622
annotations.filter(_.isStatic)
626623
}
627-
}
624+
}

Diff for: ‎internal/compiler-interface/src/main/java/xsbti/api/SafeLazy.java

+58
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package xsbti.api;
2+
3+
/**
4+
* Implement a Scala `lazy val` in Java for the facing sbt interface.
5+
*
6+
* It holds a reference to a thunk that is lazily evaluated and then
7+
* its reference is clear to avoid memory leaks in memory-intensive code.
8+
* It needs to be defined in [[xsbti]] or a subpackage, see
9+
* [[xsbti.api.Lazy]] or [[xsbti.F0]] for similar definitions.
10+
*/
11+
public class SafeLazy {
12+
13+
/* We do not use conversions from and to Scala functions because [[xsbti]]
14+
* cannot hold any reference to Scala code nor the Scala library. */
15+
16+
/** Return a sbt [[xsbti.api.Lazy]] from a given Scala parameterless function. */
17+
public static <T> xsbti.api.Lazy<T> apply(xsbti.F0<T> sbtThunk) {
18+
return new Impl<T>(sbtThunk);
19+
}
20+
21+
/** Return a sbt [[xsbti.api.Lazy]] from a strict value. */
22+
public static <T> xsbti.api.Lazy<T> strict(T value) {
23+
// Convert strict parameter to sbt function returning it
24+
return apply(new xsbti.F0<T>() {
25+
public T apply() {
26+
return value;
27+
}
28+
});
29+
}
30+
31+
private static final class Impl<T> extends xsbti.api.AbstractLazy<T> {
32+
private xsbti.F0<T> thunk;
33+
private T result;
34+
private boolean flag = false;
35+
36+
Impl(xsbti.F0<T> thunk) {
37+
this.thunk = thunk;
38+
}
39+
40+
/**
41+
* Return cached result or force lazy evaluation.
42+
*
43+
* Don't call it in a multi-threaded environment.
44+
*/
45+
public T get() {
46+
if (flag) return result;
47+
else {
48+
result = thunk.apply();
49+
flag = true;
50+
// Clear reference so that thunk is GC'ed
51+
thunk = null;
52+
return result;
53+
}
54+
}
55+
}
56+
}
57+
58+

Diff for: ‎internal/zinc-apiinfo/src/main/scala/sbt/internal/inc/ClassToAPI.scala

+6-7
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ import java.lang.annotation.Annotation
77
import annotation.tailrec
88
import inc.classfile.ClassFile
99
import xsbti.api
10-
import xsbti.SafeLazy
11-
import SafeLazy.strict
10+
import xsbti.api.SafeLazyProxy
1211
import collection.mutable
1312
import sbt.io.IO
1413

@@ -78,10 +77,10 @@ object ClassToAPI {
7877
val name = classCanonicalName(c)
7978
val tpe = if (Modifier.isInterface(c.getModifiers)) Trait else ClassDef
8079
lazy val (static, instance) = structure(c, enclPkg, cmap)
81-
val cls = new api.ClassLike(name, acc, mods, annots, tpe, strict(Empty), lzy(instance, cmap), emptyStringArray, children.toArray,
80+
val cls = new api.ClassLike(name, acc, mods, annots, tpe, lzyS(Empty), lzy(instance, cmap), emptyStringArray, children.toArray,
8281
topLevel, typeParameters(typeParameterTypes(c)))
8382
val clsDef = new api.ClassLikeDef(name, acc, mods, annots, typeParameters(typeParameterTypes(c)), tpe)
84-
val stat = new api.ClassLike(name, acc, mods, annots, Module, strict(Empty), lzy(static, cmap), emptyStringArray, emptyTypeArray,
83+
val stat = new api.ClassLike(name, acc, mods, annots, Module, lzyS(Empty), lzy(static, cmap), emptyStringArray, emptyTypeArray,
8584
topLevel, emptyTypeParameterArray)
8685
val statDef = new api.ClassLikeDef(name, acc, mods, annots, emptyTypeParameterArray, Module)
8786
val defs = cls :: stat :: Nil
@@ -112,8 +111,8 @@ object ClassToAPI {
112111
private[this] def classFileForClass(c: Class[_]): ClassFile =
113112
classfile.Parser.apply(IO.classfileLocation(c))
114113

115-
private[this] def lzyS[T <: AnyRef](t: T): xsbti.api.Lazy[T] = lzy(t)
116-
def lzy[T <: AnyRef](t: => T): xsbti.api.Lazy[T] = xsbti.SafeLazy(t)
114+
@inline private[this] def lzyS[T <: AnyRef](t: T): xsbti.api.Lazy[T] = SafeLazyProxy.strict(t)
115+
@inline final def lzy[T <: AnyRef](t: => T): xsbti.api.Lazy[T] = SafeLazyProxy(t)
117116
private[this] def lzy[T <: AnyRef](t: => T, cmap: ClassMap): xsbti.api.Lazy[T] = {
118117
val s = lzy(t)
119118
cmap.lz += s
@@ -127,7 +126,7 @@ object ClassToAPI {
127126
private val emptySimpleTypeArray = new Array[xsbti.api.SimpleType](0)
128127
private val lzyEmptyTpeArray = lzyS(emptyTypeArray)
129128
private val lzyEmptyDefArray = lzyS(new Array[xsbti.api.ClassDefinition](0))
130-
private val lzyEmptyStructure = strict(new xsbti.api.Structure(lzyEmptyTpeArray, lzyEmptyDefArray, lzyEmptyDefArray))
129+
private val lzyEmptyStructure = lzyS(new xsbti.api.Structure(lzyEmptyTpeArray, lzyEmptyDefArray, lzyEmptyDefArray))
131130

132131
private def allSuperTypes(t: Type): Seq[Type] =
133132
{

Diff for: ‎internal/zinc-apiinfo/src/main/scala/xsbt/api/APIUtil.scala

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package xsbt.api
22

3-
import xsbti.SafeLazy
43
import xsbti.api._
54
import scala.collection.mutable.HashSet
65

@@ -76,7 +75,7 @@ object APIUtil {
7675
new xsbti.api.ClassLike(name, new Public, emptyModifiers, Array.empty,
7776
definitionType, lzy(emptyType), lzy(emptyStructure), Array.empty, Array.empty, true, Array.empty)
7877

79-
private[this] def lzy[T <: AnyRef](t: T): Lazy[T] = SafeLazy.strict(t)
78+
private[this] def lzy[T <: AnyRef](t: T): Lazy[T] = SafeLazyProxy.strict(t)
8079

8180
private[this] val emptyType = new EmptyType
8281
}

Diff for: ‎internal/zinc-apiinfo/src/main/scala/xsbt/api/SafeLazyProxy.scala

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package xsbti.api
2+
3+
/**
4+
* Proxy `SafeLazy` functionality from the Java implementation
5+
* implementation in [[xsbt.api.SafeLazy]] to Scala helpers.
6+
*
7+
* The implementation of these helpers are not reused between each
8+
* other because they create intermediate anonymous functions and
9+
* the price of a new object in this hot path is not worth it.
10+
*/
11+
object SafeLazyProxy {
12+
/**
13+
* Return a lazy implementation of a Scala by-name parameter.
14+
*/
15+
def apply[T](s: => T): Lazy[T] = {
16+
val sbtThunk = new xsbti.F0[T] { def apply() = s }
17+
SafeLazy.apply(sbtThunk)
18+
}
19+
20+
/**
21+
* Return a lazy implementation of a strict value.
22+
*/
23+
def strict[T](s: T): Lazy[T] = {
24+
val sbtThunk = new xsbti.F0[T] { def apply() = s }
25+
SafeLazy.apply(sbtThunk)
26+
}
27+
}

Diff for: ‎internal/zinc-apiinfo/src/main/scala/xsbti/SafeLazy.scala

-24
This file was deleted.

Diff for: ‎internal/zinc-core/src/main/scala/sbt/internal/inc/APIs.scala

+1-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package internal
66
package inc
77

88
import xsbti.api._
9-
import xsbti.SafeLazy
109
import APIs.getAPI
1110
import xsbt.api.{ APIUtil, SameAPI }
1211

@@ -48,7 +47,7 @@ object APIs {
4847
val emptyCompilation = new xsbti.api.Compilation(-1, Array())
4948
val emptyNameHashes = new xsbti.api.NameHashes(Array.empty, Array.empty)
5049
val emptyCompanions = new xsbti.api.Companions(emptyAPI, emptyAPI)
51-
val emptyAnalyzedClass = new xsbti.api.AnalyzedClass(emptyCompilation, emptyName, SafeLazy(emptyCompanions), emptyAPIHash,
50+
val emptyAnalyzedClass = new xsbti.api.AnalyzedClass(emptyCompilation, emptyName, SafeLazyProxy(emptyCompanions), emptyAPIHash,
5251
emptyNameHashes, false)
5352
def getAPI[T](map: Map[T, AnalyzedClass], className: T): AnalyzedClass = map.getOrElse(className, emptyAnalyzedClass)
5453
}

Diff for: ‎internal/zinc-core/src/main/scala/sbt/internal/inc/Compile.scala

+3-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import sbt.internal.inc.Analysis.{ LocalProduct, NonLocalProduct }
99
import xsbt.api.{ APIUtil, HashAPI, NameHashing }
1010
import xsbti.api._
1111
import xsbti.compile.{ CompileAnalysis, DependencyChanges, IncOptions, MultipleOutput, Output, SingleOutput }
12-
import xsbti.{ Position, Problem, SafeLazy, Severity }
12+
import xsbti.{ Position, Problem, Severity }
1313
import sbt.util.Logger
1414
import sbt.util.Logger.{ m2o, problem }
1515
import java.io.File
@@ -274,7 +274,8 @@ private final class AnalysisCallback(
274274
val hasMacro: Boolean = macroClasses.contains(name)
275275
val (companions, apiHash) = companionsWithHash(name)
276276
val nameHashes = nameHashesForCompanions(name)
277-
val ac = new AnalyzedClass(compilation, name, SafeLazy(companions), apiHash, nameHashes, hasMacro)
277+
val safeCompanions = SafeLazyProxy(companions)
278+
val ac = new AnalyzedClass(compilation, name, safeCompanions, apiHash, nameHashes, hasMacro)
278279
ac
279280
}
280281

Diff for: ‎internal/zinc-core/src/test/scala/sbt/inc/TestAnalysisCallback.scala

+1-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package inc
55
import java.io.File
66

77
import scala.collection.mutable.{ ArrayBuffer, HashMap }
8-
import xsbti.SafeLazy
98
import xsbti.api._
109
import xsbti.api.DependencyContext._
1110
import xsbt.api.{ HashAPI, NameHashing, APIUtil }
@@ -105,7 +104,7 @@ class TestAnalysisCallback(
105104
val hasMacro: Boolean = macroClasses.contains(name)
106105
val (companions, apiHash) = companionsWithHash(name)
107106
val nameHashes = nameHashesForCompanions(name)
108-
val ac = new AnalyzedClass(compilation, name, SafeLazy(companions), apiHash, nameHashes, hasMacro)
107+
val ac = new AnalyzedClass(compilation, name, SafeLazyProxy(companions), apiHash, nameHashes, hasMacro)
109108
ac
110109
}
111110

Diff for: ‎internal/zinc-core/src/test/scala/sbt/inc/TestCaseGenerators.scala

+2-3
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import Gen._
1010

1111
import sbt.internal.util.Relation
1212
import xsbti.api._
13-
import xsbti.SafeLazy
1413
import xsbti.api.DependencyContext._
1514

1615
/**
@@ -82,7 +81,7 @@ object TestCaseGenerators {
8281
private[this] def makeCompanions(name: String): Companions =
8382
new Companions(makeClassLike(name, DefinitionType.ClassDef), makeClassLike(name, DefinitionType.Module))
8483

85-
private[this] def lzy[T <: AnyRef](x: T) = SafeLazy.strict(x)
84+
private[this] def lzy[T <: AnyRef](x: T) = SafeLazyProxy.strict(x)
8685

8786
def genNameHash(defn: String): Gen[xsbti.api.NameHash] =
8887
const(new xsbti.api.NameHash(defn, defn.hashCode()))
@@ -113,7 +112,7 @@ object TestCaseGenerators {
113112
apiHash <- arbitrary[Int]
114113
hasMacro <- arbitrary[Boolean]
115114
nameHashes <- genNameHashes(Seq(name))
116-
} yield new AnalyzedClass(new Compilation(startTime, Array()), name, SafeLazy(makeCompanions(name)), apiHash, nameHashes, hasMacro)
115+
} yield new AnalyzedClass(new Compilation(startTime, Array()), name, SafeLazyProxy(makeCompanions(name)), apiHash, nameHashes, hasMacro)
117116

118117
def genClasses(all_defns: Seq[String]): Gen[Seq[AnalyzedClass]] =
119118
Gen.sequence[List[AnalyzedClass], AnalyzedClass](all_defns.map(genClass))

Diff for: ‎internal/zinc-persist/src/main/scala/sbt/internal/inc/FileBasedStore.scala

-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ package inc
88
import java.io._
99
import java.util.zip.{ ZipInputStream, ZipEntry }
1010
import sbt.io.{ IO, Using }
11-
import xsbti.SafeLazy
1211
import xsbti.compile.{ CompileAnalysis, MiniSetup }
1312
import xsbti.api.Companions
1413
import scala.util.control.Exception.allCatch

Diff for: ‎internal/zinc-persist/src/main/scala/sbt/internal/inc/TextAnalysisFormat.scala

+6-5
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ package inc
44

55
import java.io._
66
import sbt.internal.util.Relation
7-
import xsbti.{ T2, SafeLazy }
8-
import xsbti.api.{ AnalyzedClass, Compilation, Companions, NameHashes, Lazy }
7+
import xsbti.T2
8+
import xsbti.api.{ AnalyzedClass, Compilation, Companions, NameHashes, Lazy, SafeLazyProxy }
99
import xsbti.compile.{ CompileAnalysis, MultipleOutput, SingleOutput, MiniOptions, MiniSetup, FileHash }
1010
import javax.xml.bind.DatatypeConverter
1111
import java.net.URI
@@ -273,6 +273,7 @@ object TextAnalysisFormat {
273273
FormatTimer.close("sbinary write")
274274
}
275275

276+
@inline final def lzy[T](t: => T) = SafeLazyProxy(t)
276277
def read(in: BufferedReader, companionsStore: Option[CompanionsStore]): APIs = {
277278
val internal = readMap(in)(Headers.internal, identity[String], stringToAnalyzedClass)
278279
val external = readMap(in)(Headers.external, identity[String], stringToAnalyzedClass)
@@ -281,11 +282,11 @@ object TextAnalysisFormat {
281282
companionsStore match {
282283
case Some(companionsStore) =>
283284
val companions: Lazy[(Map[String, Companions], Map[String, Companions])] =
284-
SafeLazy(companionsStore.getUncaught())
285+
lzy(companionsStore.getUncaught())
285286

286287
APIs(
287-
internal map { case (k, v) => k -> v.withApi(SafeLazy(companions.get._1(k))) },
288-
external map { case (k, v) => k -> v.withApi(SafeLazy(companions.get._2(k))) }
288+
internal map { case (k, v) => k -> v.withApi(lzy(companions.get._1(k))) },
289+
external map { case (k, v) => k -> v.withApi(lzy(companions.get._2(k))) }
289290
)
290291
case _ => APIs(internal, external)
291292
}

Diff for: ‎internal/zinc-persist/src/main/scala/xsbt/api/AnalyzedClassFormat.scala

+1-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
*/
44
package xsbt.api
55

6-
import xsbti.SafeLazy
76
import xsbti.api._
87
import sbinary._
98
import sbinary.DefaultProtocol._
@@ -16,7 +15,7 @@ object AnalyzedClassFormats {
1615
a => (a.compilation, a.name, a.apiHash, a.nameHashes, a.hasMacro),
1716
(x: (Compilation, String, Int, NameHashes, Boolean)) => x match {
1817
case (compilation: Compilation, name: String, apiHash: Int, nameHashes: NameHashes, hasMacro: Boolean) =>
19-
new AnalyzedClass(compilation, name, SafeLazy(emptyCompanions), apiHash, nameHashes, hasMacro)
18+
new AnalyzedClass(compilation, name, SafeLazyProxy(emptyCompanions), apiHash, nameHashes, hasMacro)
2019
}
2120
)
2221
}

0 commit comments

Comments
 (0)