Separate hash maps in the code repo for classes being compiled or not

Store classes being compiled in a separate hash map. This allows
efficiently traversing all classes being compiled. It also simplifies
limiting the size of the cache of class nodes parsed from classfiles.

Also change the cache of class nodes parsed from classfiles to LRU
instead of FIFO.

6 files changed, 54 insertions, 24 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala
index 8fc976a1c5..dd1d70c65f 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala
@@ -10,6 +10,7 @@ package opt
 import scala.tools.asm
 import asm.tree._
 import scala.collection.convert.decorateAsScala._
+import scala.collection.concurrent
 import scala.tools.asm.Attribute
 import scala.tools.nsc.backend.jvm.BackendReporting._
 import scala.tools.nsc.io.AbstractFile
@@ -29,38 +30,47 @@ class ByteCodeRepository[BT <: BTypes](val classPath: ClassFileLookup[AbstractFi
   import btypes._
 
   /**
-   * Cache for parsed ClassNodes. Also stores the source of the bytecode: [[Classfile]] if read
-   * from `classPath`, [[CompilationUnit]] if the bytecode corresponds to a class being compiled.
+   * ClassNodes for classes being compiled in the current compilation run.
+   */
+  val compilingClasses: concurrent.Map[InternalName, ClassNode] = recordPerRunCache(concurrent.TrieMap.empty)
+
+  /**
+   * Cache for parsed ClassNodes.
    * The `Long` field encodes the age of the node in the map, which allows removing old entries when
    * the map grows too large (see limitCacheSize).
    * For Java classes in mixed compilation, the map contains an error message: no ClassNode is
    * generated by the backend and also no classfile that could be parsed.
    */
-  val classes: collection.concurrent.Map[InternalName, Either[ClassNotFound, (ClassNode, Source, Long)]] = recordPerRunCache(collection.concurrent.TrieMap.empty)
+  val parsedClasses: concurrent.Map[InternalName, Either[ClassNotFound, (ClassNode, Long)]] = recordPerRunCache(concurrent.TrieMap.empty)
 
   private val maxCacheSize = 1500
   private val targetSize   = 500
 
-  private val idCounter = new AtomicLong(0)
+  private object lruCounter extends AtomicLong(0l) with collection.generic.Clearable {
+    def clear(): Unit = { this.set(0l) }
+  }
+  recordPerRunCache(lruCounter)
 
   /**
    * Prevent the code repository from growing too large. Profiling reveals that the average size
    * of a ClassNode is about 30 kb. I observed having 17k+ classes in the cache, i.e., 500 mb.
-   *
-   * We can only remove classes with `Source == Classfile`, those can be parsed again if requested.
    */
   private def limitCacheSize(): Unit = {
-    if (classes.count(c => c._2.isRight && c._2.right.get._2 == Classfile) > maxCacheSize) {
-      val removeId = idCounter.get - targetSize
-      val toRemove = classes.iterator.collect({
-        case (name, Right((_, Classfile, id))) if id < removeId => name
-      }).toList
-      toRemove foreach classes.remove
+    if (parsedClasses.size > maxCacheSize) {
+      // OK if multiple threads get here
+      val minimalLRU = parsedClasses.valuesIterator.collect({
+        case Right((_, lru)) => lru
+      }).toList.sorted(Ordering.Long.reverse).drop(targetSize).headOption.getOrElse(Long.MaxValue)
+      parsedClasses retain {
+        case (_, Right((_, lru))) => lru > minimalLRU
+        case _ => false
+      }
     }
   }
 
   def add(classNode: ClassNode, source: Source) = {
-    classes(classNode.name) = Right((classNode, source, idCounter.incrementAndGet()))
+    if (source == CompilationUnit) compilingClasses(classNode.name) = classNode
+    else parsedClasses(classNode.name) = Right((classNode, lruCounter.incrementAndGet()))
   }
 
   /**
@@ -68,18 +78,32 @@ class ByteCodeRepository[BT <: BTypes](val classPath: ClassFileLookup[AbstractFi
    * parsed from the classfile on the compile classpath.
    */
   def classNodeAndSource(internalName: InternalName): Either[ClassNotFound, (ClassNode, Source)] = {
-    val r = classes.getOrElseUpdate(internalName, {
-      limitCacheSize()
-      parseClass(internalName).map((_, Classfile, idCounter.incrementAndGet()))
+    classNode(internalName) map (n => {
+      val source = if (compilingClasses contains internalName) CompilationUnit else Classfile
+      (n, source)
     })
-    r.map(v => (v._1, v._2))
   }
 
   /**
    * The class node for an internal name. If the class node is not yet available, it is parsed from
    * the classfile on the compile classpath.
    */
-  def classNode(internalName: InternalName): Either[ClassNotFound, ClassNode] = classNodeAndSource(internalName).map(_._1)
+  def classNode(internalName: InternalName): Either[ClassNotFound, ClassNode] = {
+    compilingClasses.get(internalName).map(Right(_)) getOrElse {
+      val r = parsedClasses.get(internalName) match {
+        case Some(l @ Left(_)) => l
+        case Some(r @ Right((classNode, _))) =>
+          parsedClasses(internalName) = Right((classNode, lruCounter.incrementAndGet()))
+          r
+        case None =>
+          limitCacheSize()
+          val res = parseClass(internalName).map((_, lruCounter.incrementAndGet()))
+          parsedClasses(internalName) = res
+          res
+      }
+      r.map(_._1)
+    }
+  }
 
   /**
    * The field node for a field matching `name` and `descriptor`, accessed in class `classInternalName`.
@@ -90,7 +114,6 @@ class ByteCodeRepository[BT <: BTypes](val classPath: ClassFileLookup[AbstractFi
    */
   def fieldNode(classInternalName: InternalName, name: String, descriptor: String): Either[FieldNotFound, (FieldNode, InternalName)] = {
     def fieldNodeImpl(parent: InternalName): Either[FieldNotFound, (FieldNode, InternalName)] = {
-      def msg = s"The field node $name$descriptor could not be found in class $classInternalName or any of its superclasses."
       classNode(parent) match {
         case Left(e)  => Left(FieldNotFound(name, descriptor, classInternalName, Some(e)))
         case Right(c) =>
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 45ef810796..715db3f8c2 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/CallGraphTest.scala
@@ -28,7 +28,7 @@ class CallGraphTest {
   import compiler.genBCode.bTypes._
 
   // allows inspecting the caches after a compilation run
-  val notPerRun: List[Clearable] = List(classBTypeFromInternalName, byteCodeRepository.classes, callGraph.callsites)
+  val notPerRun: List[Clearable] = List(classBTypeFromInternalName, byteCodeRepository.compilingClasses, byteCodeRepository.parsedClasses, callGraph.callsites)
   notPerRun foreach compiler.perRunCaches.unrecordCache
 
   def compile(code: String, allowMessage: StoreReporter#Info => Boolean): List[ClassNode] = {
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/ClosureOptimizerTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/ClosureOptimizerTest.scala
index 69eed1f75d..258813ea68 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/ClosureOptimizerTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/ClosureOptimizerTest.scala
@@ -29,7 +29,7 @@ import scala.collection.convert.decorateAsScala._
 import scala.tools.testing.ClearAfterClass
 
 object ClosureOptimizerTest extends ClearAfterClass.Clearable {
-  var compiler = newCompiler(extraArgs = "-Yopt:l:classpath -Yopt-warnings")
+  var compiler = newCompiler(extraArgs = "-Yopt:l:classpath -Yopt-warnings:_")
   def clear(): Unit = { compiler = null }
 }
 
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 5ccb940415..c25933e63e 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlineInfoTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlineInfoTest.scala
@@ -22,7 +22,10 @@ object InlineInfoTest extends ClearAfterClass.Clearable {
   var compiler = newCompiler(extraArgs = "-Ybackend:GenBCode -Yopt:l:classpath")
   def clear(): Unit = { compiler = null }
 
-  def notPerRun: List[Clearable] = List(compiler.genBCode.bTypes.classBTypeFromInternalName, compiler.genBCode.bTypes.byteCodeRepository.classes)
+  def notPerRun: List[Clearable] = List(
+    compiler.genBCode.bTypes.classBTypeFromInternalName,
+    compiler.genBCode.bTypes.byteCodeRepository.compilingClasses,
+    compiler.genBCode.bTypes.byteCodeRepository.parsedClasses)
   notPerRun foreach compiler.perRunCaches.unrecordCache
 }
 
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 7ed0e13226..f1be44a094 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerIllegalAccessTest.scala
@@ -67,7 +67,7 @@ class InlinerIllegalAccessTest extends ClearAfterClass {
     check(dClass, assertEmpty)
     check(eClass, assertEmpty) // C is public, so accessible in E
 
-    byteCodeRepository.classes.clear()
+    byteCodeRepository.parsedClasses.clear()
     classBTypeFromInternalName.clear()
 
     cClass.access &= ~ACC_PUBLIC // ftw
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 baba1f3c95..c6313a84d2 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala
@@ -33,7 +33,11 @@ object InlinerTest extends ClearAfterClass.Clearable {
   var compiler = newCompiler(extraArgs = args)
 
   // allows inspecting the caches after a compilation run
-  def notPerRun: List[Clearable] = List(compiler.genBCode.bTypes.classBTypeFromInternalName, compiler.genBCode.bTypes.byteCodeRepository.classes, compiler.genBCode.bTypes.callGraph.callsites)
+  def notPerRun: List[Clearable] = List(
+    compiler.genBCode.bTypes.classBTypeFromInternalName,
+    compiler.genBCode.bTypes.byteCodeRepository.compilingClasses,
+    compiler.genBCode.bTypes.byteCodeRepository.parsedClasses,
+    compiler.genBCode.bTypes.callGraph.callsites)
   notPerRun foreach compiler.perRunCaches.unrecordCache
 
   def clear(): Unit = { compiler = null }