From 67c151f1821eb2124ac70c70bf1d974ca0e5b364 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 28 Feb 2017 15:19:25 +0100 Subject: [PATCH 1/2] Split up cache for ClassBTypes in two, according to origin Split up the `classBTypeFromInternalName` map in two, depending if the type was created from a symbol or classfile. This allows adding an additional assertion: when getting a `ClassBType` for a symbol that's being compiled in the current run, we want to assert that an existing, cached type was created from that symbol, and not from a (matching, but old) classfile on the classpath. Note that creating `ClassBTypes` from symbols is the ordinary case. In particular, bytecode generation creats these types for every class being emitted. Types created from classfiles are only used for type references not present in the soruce but introduced by the inliner. At this later stage, all types for classes being compiled are already created (and cached). --- .../tools/nsc/backend/jvm/BCodeHelpers.scala | 10 +++--- .../scala/tools/nsc/backend/jvm/BTypes.scala | 30 ++++++++-------- .../nsc/backend/jvm/BTypesFromSymbols.scala | 35 +++++++++++-------- .../tools/nsc/backend/jvm/CoreBTypes.scala | 2 +- .../backend/jvm/analysis/BackendUtils.scala | 2 +- .../jvm/opt/BTypesFromClassfileTest.scala | 5 ++- .../nsc/backend/jvm/opt/CallGraphTest.scala | 5 +-- .../nsc/backend/jvm/opt/InlineInfoTest.scala | 7 ++-- .../jvm/opt/InlinerIllegalAccessTest.scala | 17 ++++++++- .../nsc/backend/jvm/opt/InlinerTest.scala | 3 +- 10 files changed, 73 insertions(+), 43 deletions(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala index a74c70a68491..32f8816c5796 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala +++ b/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") diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala index 3e3229d2c3a5..31daa09ab54d 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala @@ -64,17 +64,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 +175,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 +188,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)) }) } @@ -847,7 +849,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 @@ -868,7 +870,7 @@ abstract class BTypes { checkInfoConsistency() } - classBTypeFromInternalName(internalName) = this + cache(internalName) = this private def checkInfoConsistency(): Unit = { if (info.isLeft) return diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala index f7ee36c1ba0a..6a742376ee71 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala @@ -115,17 +115,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) + } + } } } @@ -626,8 +631,8 @@ 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 c.info = Right(ClassInfo( @@ -643,8 +648,8 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { 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, diff --git a/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala index acb950929f1c..83912cf88f41 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala +++ b/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. diff --git a/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala b/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala index 90da570f0173..720f794c3b0e 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala @@ -81,7 +81,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. diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.scala index 42a2c417a0b2..56a78d9dc5f0 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.scala @@ -27,7 +27,10 @@ class BTypesFromClassfileTest extends BytecodeTesting { duringBackend(global.scalaPrimitives.init()) // needed: it's only done when running the backend, and we don't actually run the compiler duringBackend(bTypes.initializeCoreBTypes()) - def clearCache() = bTypes.classBTypeFromInternalName.clear() + def clearCache() = { + bTypes.classBTypeCacheFromSymbol.clear() + bTypes.classBTypeCacheFromClassfile.clear() + } def sameBType(fromSym: ClassBType, fromClassfile: ClassBType, checked: Set[InternalName] = Set.empty): Set[InternalName] = { if (checked(fromSym.internalName)) checked diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala index a74e73afc984..97da902ef1dc 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala @@ -22,7 +22,8 @@ class CallGraphTest extends BytecodeTesting { import compiler._ import global.genBCode.bTypes val notPerRun: List[Clearable] = List( - bTypes.classBTypeFromInternalName, + bTypes.classBTypeCacheFromSymbol, + bTypes.classBTypeCacheFromClassfile, bTypes.byteCodeRepository.compilingClasses, bTypes.byteCodeRepository.parsedClasses, bTypes.callGraph.callsites) @@ -145,7 +146,7 @@ class CallGraphTest extends BytecodeTesting { val m = getAsmMethod(c, "m") val List(fn) = callsInMethod(m) val forNameMeth = byteCodeRepository.methodNode("java/lang/Class", "forName", "(Ljava/lang/String;)Ljava/lang/Class;").get._1 - val classTp = classBTypeFromInternalName("java/lang/Class") + val classTp = cachedClassBType("java/lang/Class").get val r = callGraph.callsites(m)(fn) checkCallsite(fn, m, forNameMeth, classTp, safeToInline = false, atInline = false, atNoInline = false) } diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlineInfoTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlineInfoTest.scala index 6f098e1432a0..4ea628c70eb0 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlineInfoTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlineInfoTest.scala @@ -21,7 +21,8 @@ class InlineInfoTest extends BytecodeTesting { override def compilerArgs = "-opt:l:classpath" def notPerRun: List[Clearable] = List( - bTypes.classBTypeFromInternalName, + bTypes.classBTypeCacheFromSymbol, + bTypes.classBTypeCacheFromClassfile, bTypes.byteCodeRepository.compilingClasses, bTypes.byteCodeRepository.parsedClasses) notPerRun foreach global.perRunCaches.unrecordCache @@ -52,7 +53,7 @@ class InlineInfoTest extends BytecodeTesting { """.stripMargin val classes = compile(code) - val fromSyms = classes.map(c => global.genBCode.bTypes.classBTypeFromInternalName(c.name).info.get.inlineInfo) + val fromSyms = classes.map(c => global.genBCode.bTypes.cachedClassBType(c.name).get.info.get.inlineInfo) val fromAttrs = classes.map(c => { assert(c.attrs.asScala.exists(_.isInstanceOf[InlineInfoAttribute]), c.attrs) @@ -71,7 +72,7 @@ class InlineInfoTest extends BytecodeTesting { |} """.stripMargin compileClasses("class C { new A }", javaCode = List((jCode, "A.java"))) - val info = global.genBCode.bTypes.classBTypeFromInternalName("A").info.get.inlineInfo + val info = global.genBCode.bTypes.cachedClassBType("A").get.info.get.inlineInfo assertEquals(info.methodInfos, Map( "bar()I" -> MethodInlineInfo(true,false,false), "()V" -> MethodInlineInfo(false,false,false), diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala index bf9da0f48f5c..079cacdaf54f 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala @@ -23,6 +23,11 @@ class InlinerIllegalAccessTest extends BytecodeTesting { def assertEmpty(ins: List[AbstractInsnNode]) = for (i <- ins) throw new AssertionError(textify(i)) + def clearClassBTypeCaches(): Unit = { + classBTypeCacheFromSymbol.clear() + classBTypeCacheFromClassfile.clear() + } + @Test def typeAccessible(): Unit = { val code = @@ -56,7 +61,7 @@ class InlinerIllegalAccessTest extends BytecodeTesting { check(eClass, assertEmpty) // C is public, so accessible in E byteCodeRepository.parsedClasses.clear() - classBTypeFromInternalName.clear() + clearClassBTypeCaches() cClass.access &= ~ACC_PUBLIC // ftw addToRepo(allClasses) @@ -72,6 +77,11 @@ class InlinerIllegalAccessTest extends BytecodeTesting { } // MatchError otherwise }) + + // ensure the caches are empty at the end for the next test to run (`check` caches types by + // calling `classBTypeFromParsedClassfile`). note that per-run caches are cleared at the end + // of a compilation, not the beginning. + clearClassBTypeCaches() } @Test @@ -190,5 +200,10 @@ class InlinerIllegalAccessTest extends BytecodeTesting { // privated method accesses can only be inlined in the same class for (m <- Set(rdC, rhC)) check(m, cCl, cCl, assertEmpty) for (m <- Set(rdC, rhC); c <- allClasses.tail) check(m, cCl, c, cOrDOwner) + + // ensure the caches are empty at the end for the next test to run (`check` caches types by + // calling `classBTypeFromParsedClassfile`). note that per-run caches are cleared at the end + // of a compilation, not the beginning. + clearClassBTypeCaches() } } diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala index 7be88816d50f..36089ac1dcbe 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala @@ -27,7 +27,8 @@ class InlinerTest extends BytecodeTesting { import global.genBCode.bTypes // allows inspecting the caches after a compilation run def notPerRun: List[Clearable] = List( - bTypes.classBTypeFromInternalName, + bTypes.classBTypeCacheFromSymbol, + bTypes.classBTypeCacheFromClassfile, bTypes.byteCodeRepository.compilingClasses, bTypes.byteCodeRepository.parsedClasses, bTypes.callGraph.callsites) From 5abd5b859f2ca5edd0971b7632de81d47eda54d9 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 28 Feb 2017 15:29:33 +0100 Subject: [PATCH 2/2] Make ClassBType components lazy, prevent forcing symbols unnecessarily Make the `nestedClasses` and `nestedInfo` fields lazy to ensure the corresponding symbol infos are only forced if necessary. --- .../nsc/backend/jvm/BCodeSkelBuilder.scala | 2 +- .../scala/tools/nsc/backend/jvm/BTypes.scala | 63 ++++++++++++++++--- .../nsc/backend/jvm/BTypesFromSymbols.scala | 22 ++++--- .../backend/jvm/analysis/BackendUtils.scala | 2 +- .../tools/nsc/backend/jvm/BTypesTest.scala | 13 ++++ .../jvm/opt/BTypesFromClassfileTest.scala | 30 +++++++-- 6 files changed, 109 insertions(+), 23 deletions(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala index 03df1c76fa8f..58bdc79624fd 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala +++ b/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 ---------------- */ diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala index 31daa09ab54d..5d2c2fe46fc2 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala +++ b/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. @@ -223,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) { @@ -248,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 } @@ -895,7 +900,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) + } } /** @@ -923,17 +930,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, @@ -1066,9 +1073,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. diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala index 6a742376ee71..f87c3d7a59e4 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala @@ -360,7 +360,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 @@ -432,7 +432,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. @@ -442,9 +442,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) @@ -634,13 +640,13 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { 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 }) @@ -654,8 +660,8 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { superClass = Some(sbScalaBeanInfoRef), interfaces = Nil, flags = javaFlags(mainClass), - nestedClasses = Nil, - nestedInfo = None, + nestedClasses = Lazy(Nil), + nestedInfo = Lazy(None), inlineInfo = EmptyInlineInfo)) c }) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala b/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala index 720f794c3b0e..d15de851dbf1 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala @@ -324,7 +324,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 diff --git a/test/junit/scala/tools/nsc/backend/jvm/BTypesTest.scala b/test/junit/scala/tools/nsc/backend/jvm/BTypesTest.scala index 58df4691e473..c31979156c6e 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/BTypesTest.scala +++ b/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() { diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.scala index 56a78d9dc5f0..b504f4d0ab40 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/BTypesFromClassfileTest.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.nsc.backend.jvm.BTypes.InternalName import scala.tools.nsc.backend.jvm.BackendReporting._ @@ -52,7 +53,7 @@ class BTypesFromClassfileTest extends BytecodeTesting { // Nested class symbols can undergo makeNotPrivate (ExplicitOuter). But this is only applied // for symbols of class symbols that are being compiled, not those read from a pickle. // So a class may be public in bytecode, but the symbol still says private. - if (fromSym.nestedInfo.isEmpty) fromSym.flags == fromClassfile.flags + if (fromSym.nestedInfo.force.isEmpty) fromSym.flags == fromClassfile.flags else (fromSym.flags | ACC_PRIVATE | ACC_PUBLIC) == (fromClassfile.flags | ACC_PRIVATE | ACC_PUBLIC) }, s"class flags differ\n$fromSym\n$fromClassfile") @@ -70,19 +71,38 @@ class BTypesFromClassfileTest extends BytecodeTesting { // and anonymous classes as members of the outer class. But not for unpickled symbols). // The fromClassfile info has all nested classes, including anonymous and local. So we filter // them out: member classes are identified by having the `outerName` defined. - val memberClassesFromClassfile = fromClassfile.nestedClasses.filter(_.info.get.nestedInfo.get.outerName.isDefined) + val memberClassesFromClassfile = fromClassfile.nestedClasses.force.filter(_.info.get.nestedInfo.force.get.outerName.isDefined) // Sorting is required: the backend sorts all InnerClass entries by internalName before writing // them to the classfile (to make it deterministic: the entries are collected in a Set during // code generation). - val chk3 = sameBTypes(fromSym.nestedClasses.sortBy(_.internalName), memberClassesFromClassfile.sortBy(_.internalName), chk2) - sameBTypes(fromSym.nestedInfo.map(_.enclosingClass), fromClassfile.nestedInfo.map(_.enclosingClass), chk3) + val chk3 = sameBTypes(fromSym.nestedClasses.force.sortBy(_.internalName), memberClassesFromClassfile.sortBy(_.internalName), chk2) + sameBTypes(fromSym.nestedInfo.force.map(_.enclosingClass), fromClassfile.nestedInfo.force.map(_.enclosingClass), chk3) + } + + // Force all lazy components of a `ClassBType`. + def forceLazy(classBType: ClassBType, done: mutable.Set[ClassBType] = mutable.Set.empty): Unit = { + if (!done(classBType)) { + done += classBType + val i = classBType.info.get + i.superClass.foreach(forceLazy(_, done)) + i.interfaces.foreach(forceLazy(_, done)) + i.nestedClasses.force.foreach(forceLazy(_, done)) + i.nestedInfo.force.foreach(n => forceLazy(n.enclosingClass, done)) + } } def check(classSym: Symbol): Unit = duringBackend { clearCache() val fromSymbol = classBTypeFromSymbol(classSym) + // We need to force all lazy components to ensure that all referenced `ClassBTypes` are + // constructed from symbols. If we keep them lazy, a `ClassBType` might be constructed from a + // classfile first and then picked up cachedClassBType and added to the `fromSymbol` type. + forceLazy(fromSymbol) + clearCache() - val fromClassfile = bTypes.classBTypeFromParsedClassfile(fromSymbol.internalName) + val fromClassfile = classBTypeFromParsedClassfile(fromSymbol.internalName) + forceLazy(fromClassfile) + sameBType(fromSymbol, fromClassfile) }