Merge pull request scala#5739 from lrytz/lazyBTypes

Lazy ClassBTypes

Author: retronym

src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala

@@ -276,12 +276,14 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
   final class CClassWriter(flags: Int) extends asm.ClassWriter(flags) {
 
     /**
-     * This method is thread-safe: it depends only on the BTypes component, which does not depend
-     * on global. TODO @lry move to a different place where no global is in scope, on bTypes.
+     * This method is used by asm when computing stack map frames. It is thread-safe: it depends
+     * only on the BTypes component, which does not depend on global.
+     * TODO @lry move to a different place where no global is in scope, on bTypes.
      */
     override def getCommonSuperClass(inameA: String, inameB: String): String = {
-      val a = classBTypeFromInternalName(inameA)
-      val b = classBTypeFromInternalName(inameB)
+      // All types that appear in a class node need to have their ClassBType cached, see [[cachedClassBType]].
+      val a = cachedClassBType(inameA).get
+      val b = cachedClassBType(inameB).get
       val lub = a.jvmWiseLUB(b).get
       val lubName = lub.internalName
       assert(lubName != "scala/Any")

src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala

@@ -79,7 +79,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
     def tpeTK(tree: Tree): BType = typeToBType(tree.tpe)
 
     def log(msg: => AnyRef) {
-      global synchronized { global.log(msg) }
+      frontendLock synchronized { global.log(msg) }
     }
 
     /* ---------------- helper utils for generating classes and fields ---------------- */

src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala

@@ -32,6 +32,11 @@ import scala.tools.nsc.settings.ScalaSettings
 abstract class BTypes {
   import BTypes.InternalName
 
+  // Stages after code generation in the backend (optimizations, classfile writing) are prepared
+  // to run in parallel on multiple classes. This object should be used for synchronizing operations
+  // that may access the compiler frontend during these late stages.
+  val frontendLock: AnyRef = new Object()
+
   val backendUtils: BackendUtils[this.type]
 
   // Some core BTypes are required here, in class BType, where no Global instance is available.
@@ -64,17 +69,19 @@ abstract class BTypes {
   def compilerSettings: ScalaSettings
 
   /**
-   * A map from internal names to ClassBTypes. Every ClassBType is added to this map on its
-   * construction.
+   * Every ClassBType is cached on construction and accessible through this method.
    *
-   * This map is used when computing stack map frames. The asm.ClassWriter invokes the method
+   * The cache is used when computing stack map frames. The asm.ClassWriter invokes the method
    * `getCommonSuperClass`. In this method we need to obtain the ClassBType for a given internal
-   * name. The method assumes that every class type that appears in the bytecode exists in the map.
-   *
-   * Concurrent because stack map frames are computed when in the class writer, which might run
-   * on multiple classes concurrently.
+   * name. The method assumes that every class type that appears in the bytecode exists in the map
    */
-  val classBTypeFromInternalName: concurrent.Map[InternalName, ClassBType] = recordPerRunCache(TrieMap.empty)
+  def cachedClassBType(internalName: InternalName): Option[ClassBType] =
+    classBTypeCacheFromSymbol.get(internalName).orElse(classBTypeCacheFromClassfile.get(internalName))
+
+  // Concurrent maps because stack map frames are computed when in the class writer, which
+  // might run on multiple classes concurrently.
+  val classBTypeCacheFromSymbol: concurrent.Map[InternalName, ClassBType] = recordPerRunCache(TrieMap.empty)
+  val classBTypeCacheFromClassfile: concurrent.Map[InternalName, ClassBType] = recordPerRunCache(TrieMap.empty)
 
   /**
    * Store the position of every MethodInsnNode during code generation. This allows each callsite
@@ -173,8 +180,8 @@ abstract class BTypes {
    * be found in the `byteCodeRepository`, the `info` of the resulting ClassBType is undefined.
    */
   def classBTypeFromParsedClassfile(internalName: InternalName): ClassBType = {
-    classBTypeFromInternalName.getOrElse(internalName, {
-      val res = ClassBType(internalName)
+    cachedClassBType(internalName).getOrElse({
+      val res = ClassBType(internalName)(classBTypeCacheFromClassfile)
       byteCodeRepository.classNode(internalName) match {
         case Left(msg) => res.info = Left(NoClassBTypeInfoMissingBytecode(msg)); res
         case Right(c)  => setClassInfoFromClassNode(c, res)
@@ -186,8 +193,8 @@ abstract class BTypes {
    * Construct the [[ClassBType]] for a parsed classfile.
    */
   def classBTypeFromClassNode(classNode: ClassNode): ClassBType = {
-    classBTypeFromInternalName.getOrElse(classNode.name, {
-      setClassInfoFromClassNode(classNode, ClassBType(classNode.name))
+    cachedClassBType(classNode.name).getOrElse({
+      setClassInfoFromClassNode(classNode, ClassBType(classNode.name)(classBTypeCacheFromClassfile))
     })
   }
 
@@ -221,13 +228,13 @@ abstract class BTypes {
       })
     }
 
-    val nestedClasses: List[ClassBType] = classNode.innerClasses.asScala.collect({
+    def nestedClasses: List[ClassBType] = classNode.innerClasses.asScala.collect({
       case i if nestedInCurrentClass(i) => classBTypeFromParsedClassfile(i.name)
     })(collection.breakOut)
 
     // if classNode is a nested class, it has an innerClass attribute for itself. in this
     // case we build the NestedInfo.
-    val nestedInfo = classNode.innerClasses.asScala.find(_.name == classNode.name) map {
+    def nestedInfo = classNode.innerClasses.asScala.find(_.name == classNode.name) map {
       case innerEntry =>
         val enclosingClass =
           if (innerEntry.outerName != null) {
@@ -246,7 +253,7 @@ abstract class BTypes {
 
     val interfaces: List[ClassBType] = classNode.interfaces.asScala.map(classBTypeFromParsedClassfile)(collection.breakOut)
 
-    classBType.info = Right(ClassInfo(superClass, interfaces, flags, nestedClasses, nestedInfo, inlineInfo))
+    classBType.info = Right(ClassInfo(superClass, interfaces, flags, Lazy(nestedClasses), Lazy(nestedInfo), inlineInfo))
     classBType
   }
 
@@ -857,7 +864,7 @@ abstract class BTypes {
    * a missing info. In order not to crash the compiler unnecessarily, the inliner does not force
    * infos using `get`, but it reports inliner warnings for missing infos that prevent inlining.
    */
-  final case class ClassBType(internalName: InternalName) extends RefBType {
+  final case class ClassBType(internalName: InternalName)(cache: mutable.Map[InternalName, ClassBType]) extends RefBType {
     /**
      * Write-once variable allows initializing a cyclic graph of infos. This is required for
      * nested classes. Example: for the definition `class A { class B }` we have
@@ -878,7 +885,7 @@ abstract class BTypes {
       checkInfoConsistency()
     }
 
-    classBTypeFromInternalName(internalName) = this
+    cache(internalName) = this
 
     private def checkInfoConsistency(): Unit = {
       if (info.isLeft) return
@@ -903,7 +910,9 @@ abstract class BTypes {
         s"Invalid interfaces in $this: ${info.get.interfaces}"
       )
 
-      assert(info.get.nestedClasses.forall(c => ifInit(c)(_.isNestedClass.get)), info.get.nestedClasses)
+      info.get.nestedClasses.onForce { cs =>
+        assert(cs.forall(c => ifInit(c)(_.isNestedClass.get)), cs)
+      }
     }
 
     /**
@@ -931,17 +940,17 @@ abstract class BTypes {
 
     def isPublic: Either[NoClassBTypeInfo, Boolean] = info.map(i => (i.flags & asm.Opcodes.ACC_PUBLIC) != 0)
 
-    def isNestedClass: Either[NoClassBTypeInfo, Boolean] = info.map(_.nestedInfo.isDefined)
+    def isNestedClass: Either[NoClassBTypeInfo, Boolean] = info.map(_.nestedInfo.force.isDefined)
 
     def enclosingNestedClassesChain: Either[NoClassBTypeInfo, List[ClassBType]] = {
       isNestedClass.flatMap(isNested => {
         // if isNested is true, we know that info.get is defined, and nestedInfo.get is also defined.
-        if (isNested) info.get.nestedInfo.get.enclosingClass.enclosingNestedClassesChain.map(this :: _)
+        if (isNested) info.get.nestedInfo.force.get.enclosingClass.enclosingNestedClassesChain.map(this :: _)
         else Right(Nil)
       })
     }
 
-    def innerClassAttributeEntry: Either[NoClassBTypeInfo, Option[InnerClassEntry]] = info.map(i => i.nestedInfo map {
+    def innerClassAttributeEntry: Either[NoClassBTypeInfo, Option[InnerClassEntry]] = info.map(i => i.nestedInfo.force map {
       case NestedInfo(_, outerName, innerName, isStaticNestedClass) =>
         InnerClassEntry(
           internalName,
@@ -1074,9 +1083,49 @@ abstract class BTypes {
    * @param inlineInfo    Information about this class for the inliner.
    */
   final case class ClassInfo(superClass: Option[ClassBType], interfaces: List[ClassBType], flags: Int,
-                             nestedClasses: List[ClassBType], nestedInfo: Option[NestedInfo],
+                             nestedClasses: Lazy[List[ClassBType]], nestedInfo: Lazy[Option[NestedInfo]],
                              inlineInfo: InlineInfo)
 
+  object Lazy {
+    def apply[T <: AnyRef](t: => T): Lazy[T] = new Lazy[T](() => t)
+  }
+
+  final class Lazy[T <: AnyRef](t: () => T) {
+    private var value: T = null.asInstanceOf[T]
+
+    private var function = {
+      val tt = t // prevent allocating a field for t
+      () => { value = tt() }
+    }
+
+    override def toString = if (value == null) "<?>" else value.toString
+
+    def onForce(f: T => Unit): Unit = {
+      if (value != null) f(value)
+      else frontendLock.synchronized {
+        if (value != null) f(value)
+        else {
+          val prev = function
+          function = () => {
+            prev()
+            f(value)
+          }
+        }
+      }
+    }
+
+    def force: T = {
+      if (value != null) value
+      else frontendLock.synchronized {
+        if (value == null) {
+          function()
+          function = null
+        }
+        value
+      }
+    }
+  }
+
   /**
    * Information required to add a class to an InnerClass table.
    * The spec summary above explains what information is required for the InnerClass entry.

src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala

@@ -118,17 +118,22 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
     else if (classSym == NullClass) srNullRef
     else {
       val internalName = classSym.javaBinaryNameString
-      classBTypeFromInternalName.getOrElse(internalName, {
-        // The new ClassBType is added to the map in its constructor, before we set its info. This
-        // allows initializing cyclic dependencies, see the comment on variable ClassBType._info.
-        val res = ClassBType(internalName)
-        if (completeSilentlyAndCheckErroneous(classSym)) {
-          res.info = Left(NoClassBTypeInfoClassSymbolInfoFailedSI9111(classSym.fullName))
-          res
-        } else {
-          setClassInfo(classSym, res)
-        }
-      })
+      cachedClassBType(internalName) match {
+        case Some(bType) =>
+          if (currentRun.compiles(classSym))
+            assert(classBTypeCacheFromSymbol.contains(internalName), s"ClassBType for class being compiled was already created from a classfile: ${classSym.fullName}")
+          bType
+        case None =>
+          // The new ClassBType is added to the map in its constructor, before we set its info. This
+          // allows initializing cyclic dependencies, see the comment on variable ClassBType._info.
+          val res = ClassBType(internalName)(classBTypeCacheFromSymbol)
+          if (completeSilentlyAndCheckErroneous(classSym)) {
+            res.info = Left(NoClassBTypeInfoClassSymbolInfoFailedSI9111(classSym.fullName))
+            res
+          } else {
+            setClassInfo(classSym, res)
+          }
+      }
     }
   }
 
@@ -358,7 +363,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
      * declared but not otherwise referenced in C (from the bytecode or a method / field signature).
      * We collect them here.
      */
-    val nestedClassSymbols = {
+    lazy val nestedClassSymbols = {
       val linkedClass = exitingPickler(classSym.linkedClassOfClass) // linkedCoC does not work properly in late phases
 
       // The lambdalift phase lifts all nested classes to the enclosing class, so if we collect
@@ -430,7 +435,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
      * for A contain both the class B and the module class B.
      * Here we get rid of the module class B, making sure that the class B is present.
      */
-    val nestedClassSymbolsNoJavaModuleClasses = nestedClassSymbols.filter(s => {
+    def nestedClassSymbolsNoJavaModuleClasses = nestedClassSymbols.filter(s => {
       if (s.isJavaDefined && s.isModuleClass) {
         // We could also search in nestedClassSymbols for s.linkedClassOfClass, but sometimes that
         // returns NoSymbol, so it doesn't work.
@@ -440,9 +445,15 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
       } else true
     })
 
-    val nestedClasses = nestedClassSymbolsNoJavaModuleClasses.map(classBTypeFromSymbol)
+    val nestedClasses = {
+      val ph = phase
+      Lazy(enteringPhase(ph)(nestedClassSymbolsNoJavaModuleClasses.map(classBTypeFromSymbol)))
+    }
 
-    val nestedInfo = buildNestedInfo(classSym)
+    val nestedInfo = {
+      val ph = phase
+      Lazy(enteringPhase(ph)(buildNestedInfo(classSym)))
+    }
 
     val inlineInfo = buildInlineInfo(classSym, classBType.internalName)
 
@@ -632,31 +643,31 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
   def mirrorClassClassBType(moduleClassSym: Symbol): ClassBType = {
     assert(isTopLevelModuleClass(moduleClassSym), s"not a top-level module class: $moduleClassSym")
     val internalName = moduleClassSym.javaBinaryNameString.stripSuffix(nme.MODULE_SUFFIX_STRING)
-    classBTypeFromInternalName.getOrElse(internalName, {
-      val c = ClassBType(internalName)
+    cachedClassBType(internalName).getOrElse({
+      val c = ClassBType(internalName)(classBTypeCacheFromSymbol)
       // class info consistent with BCodeHelpers.genMirrorClass
-      val nested = exitingPickler(memberClassesForInnerClassTable(moduleClassSym)) map classBTypeFromSymbol
+      val nested = Lazy(exitingPickler(memberClassesForInnerClassTable(moduleClassSym)) map classBTypeFromSymbol)
       c.info = Right(ClassInfo(
         superClass = Some(ObjectRef),
         interfaces = Nil,
         flags = asm.Opcodes.ACC_SUPER | asm.Opcodes.ACC_PUBLIC | asm.Opcodes.ACC_FINAL,
         nestedClasses = nested,
-        nestedInfo = None,
+        nestedInfo = Lazy(None),
         inlineInfo = EmptyInlineInfo.copy(isEffectivelyFinal = true))) // no method inline infos needed, scala never invokes methods on the mirror class
       c
     })
   }
 
   def beanInfoClassClassBType(mainClass: Symbol): ClassBType = {
     val internalName = mainClass.javaBinaryNameString + "BeanInfo"
-    classBTypeFromInternalName.getOrElse(internalName, {
-      val c = ClassBType(internalName)
+    cachedClassBType(internalName).getOrElse({
+      val c = ClassBType(internalName)(classBTypeCacheFromSymbol)
       c.info = Right(ClassInfo(
         superClass = Some(sbScalaBeanInfoRef),
         interfaces = Nil,
         flags = javaFlags(mainClass),
-        nestedClasses = Nil,
-        nestedInfo = None,
+        nestedClasses = Lazy(Nil),
+        nestedInfo = Lazy(None),
         inlineInfo = EmptyInlineInfo))
       c
     })

src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala

@@ -22,7 +22,7 @@ import scala.tools.nsc.backend.jvm.BTypes.InternalName
  * constructor will actually go through the proxy. The lazy vals make sure the instance is assigned
  * in the proxy before the fields are initialized.
  *
- * Note: if we did not re-create the core BTypes on each compiler run, BType.classBTypeFromInternalNameMap
+ * Note: if we did not re-create the core BTypes on each compiler run, BType.classBTypeCacheFromSymbol
  * could not be a perRunCache anymore: the classes defined here need to be in that map, they are
  * added when the ClassBTypes are created. The per run cache removes them, so they would be missing
  * in the second run.

src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala

@@ -93,7 +93,7 @@ class BackendUtils[BT <: BTypes](val btypes: BT) {
 
     // Make sure to reference the ClassBTypes of all types that are used in the code generated
     // here (e.g. java/util/Map) are initialized. Initializing a ClassBType adds it to the
-    // `classBTypeFromInternalName` map. When writing the classfile, the asm ClassWriter computes
+    // `cachedClassBType` maps. When writing the classfile, the asm ClassWriter computes
     // stack map frames and invokes the `getCommonSuperClass` method. This method expects all
     // ClassBTypes mentioned in the source code to exist in the map.
 
@@ -355,7 +355,7 @@ class BackendUtils[BT <: BTypes](val btypes: BT) {
     }
 
     visitInternalName(classNode.name)
-    innerClasses ++= classBTypeFromParsedClassfile(classNode.name).info.get.nestedClasses
+    innerClasses ++= classBTypeFromParsedClassfile(classNode.name).info.get.nestedClasses.force
 
     visitInternalName(classNode.superName)
     classNode.interfaces.asScala foreach visitInternalName

test/junit/scala/tools/nsc/backend/jvm/BTypesTest.scala

@@ -6,6 +6,7 @@ import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
 
+import scala.collection.mutable
 import scala.tools.asm.Opcodes
 import scala.tools.testing.BytecodeTesting
 
@@ -80,6 +81,18 @@ class BTypesTest extends BytecodeTesting {
     }
   }
 
+  @Test
+  def lazyForceTest(): Unit = {
+    val res = new mutable.StringBuilder()
+    val l = Lazy({res append "1"; "hi"})
+    l.onForce(v => res append s"-2:$v")
+    l.onForce(v => res append s"-3:$v:${l.force}") // `force` within `onForce` returns the value
+    assertEquals("<?>", l.toString)
+    assertEquals("hi", l.force)
+    assertEquals("hi", l.toString)
+    assertEquals("1-2:hi-3:hi:hi", res.toString)
+  }
+
   // TODO @lry do more tests
   @Test
   def maxTypeTest() {