Optimize the classpath implementation

  - Avoid repeated conversion between package dotted names ("com.foo")
    and relative paths ("com/foo/") by threading a `PackageName`
    instance through the aggregate classpath lookup which caches
    the path name.
  - Avoid creating result buffers inside of each element of the
    aggregate classpath by introducing a callback based API that
    lets `AggregateClasspath` use a single buffer.

Author: retronym

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

@@ -12,7 +12,7 @@
 
 package scala.tools.nsc.backend.jvm
 
-import java.io.{BufferedOutputStream, DataOutputStream, FileOutputStream, IOException}
+import java.io.{DataOutputStream, IOException}
 import java.nio.ByteBuffer
 import java.nio.channels.{ClosedByInterruptException, FileChannel}
 import java.nio.charset.StandardCharsets

src/compiler/scala/tools/nsc/classpath/AggregateClassPath.scala

@@ -13,11 +13,11 @@
 package scala.tools.nsc.classpath
 
 import java.net.URL
+
 import scala.collection.mutable.ArrayBuffer
 import scala.reflect.internal.FatalError
 import scala.reflect.io.AbstractFile
-import scala.tools.nsc.util.ClassPath
-import scala.tools.nsc.util.ClassRepresentation
+import scala.tools.nsc.util.{ClassPath, ClassRepresentation, EfficientClassPath}
 
 /**
  * A classpath unifying multiple class- and sourcepath entries.
@@ -30,21 +30,21 @@ import scala.tools.nsc.util.ClassRepresentation
 case class AggregateClassPath(aggregates: Seq[ClassPath]) extends ClassPath {
   override def findClassFile(className: String): Option[AbstractFile] = {
     val (pkg, simpleClassName) = PackageNameUtils.separatePkgAndClassNames(className)
-    aggregatesForPackage(pkg).iterator.map(_.findClassFile(className)).collectFirst {
+    aggregatesForPackage(PackageName(pkg)).iterator.map(_.findClassFile(className)).collectFirst {
       case Some(x) => x
     }
   }
   private[this] val packageIndex: collection.mutable.Map[String, Seq[ClassPath]] = collection.mutable.Map()
-  private def aggregatesForPackage(pkg: String): Seq[ClassPath] = packageIndex.synchronized {
-    packageIndex.getOrElseUpdate(pkg, aggregates.filter(_.hasPackage(pkg)))
+  private def aggregatesForPackage(pkg: PackageName): Seq[ClassPath] = packageIndex.synchronized {
+    packageIndex.getOrElseUpdate(pkg.dottedString, aggregates.filter(_.hasPackage(pkg)))
   }
 
   // This method is performance sensitive as it is used by SBT's ExtractDependencies phase.
   override def findClass(className: String): Option[ClassRepresentation] = {
     val (pkg, simpleClassName) = PackageNameUtils.separatePkgAndClassNames(className)
 
     def findEntry(isSource: Boolean): Option[ClassRepresentation] = {
-      aggregatesForPackage(pkg).iterator.map(_.findClass(className)).collectFirst {
+      aggregatesForPackage(PackageName(pkg)).iterator.map(_.findClass(className)).collectFirst {
         case Some(s: SourceFileEntry) if isSource => s
         case Some(s: ClassFileEntry) if !isSource => s
       }
@@ -66,31 +66,41 @@ case class AggregateClassPath(aggregates: Seq[ClassPath]) extends ClassPath {
 
   override def asSourcePathString: String = ClassPath.join(aggregates map (_.asSourcePathString): _*)
 
-  override private[nsc] def packages(inPackage: String): Seq[PackageEntry] = {
+  override private[nsc] def packages(inPackage: PackageName): Seq[PackageEntry] = {
     val aggregatedPackages = aggregates.flatMap(_.packages(inPackage)).distinct
     aggregatedPackages
   }
 
-  override private[nsc] def classes(inPackage: String): Seq[ClassFileEntry] =
+  override private[nsc] def classes(inPackage: PackageName): Seq[ClassFileEntry] =
     getDistinctEntries(_.classes(inPackage))
 
-  override private[nsc] def sources(inPackage: String): Seq[SourceFileEntry] =
+  override private[nsc] def sources(inPackage: PackageName): Seq[SourceFileEntry] =
     getDistinctEntries(_.sources(inPackage))
 
-  override private[nsc] def hasPackage(pkg: String) = aggregates.exists(_.hasPackage(pkg))
-  override private[nsc] def list(inPackage: String): ClassPathEntries = {
-    val (packages, classesAndSources) = aggregates.map { cp =>
+  override private[nsc] def hasPackage(pkg: PackageName) = aggregates.exists(_.hasPackage(pkg))
+  override private[nsc] def list(inPackage: PackageName): ClassPathEntries = {
+    val packages: java.util.HashSet[PackageEntry] = new java.util.HashSet[PackageEntry]()
+    val classesAndSourcesBuffer = collection.mutable.ArrayBuffer[ClassRepresentation]()
+    aggregates.foreach { cp =>
       try {
-        cp.list(inPackage)
+        cp match {
+          case ecp: EfficientClassPath =>
+            ecp.list(inPackage, packages.add(_), classesAndSourcesBuffer += _)
+          case _ =>
+            val entries = cp.list(inPackage)
+            entries._1.foreach(entry => packages.add(entry))
+            classesAndSourcesBuffer ++= entries._2
+        }
       } catch {
         case ex: java.io.IOException =>
           val e = FatalError(ex.getMessage)
           e.initCause(ex)
           throw e
       }
-    }.unzip
-    val distinctPackages = packages.flatten.distinct
-    val distinctClassesAndSources = mergeClassesAndSources(classesAndSources)
+    }
+
+    val distinctPackages: Seq[PackageEntry] = if (packages == null) Nil else packages.toArray(new Array[PackageEntry](packages.size()))
+    val distinctClassesAndSources = mergeClassesAndSources(classesAndSourcesBuffer)
     ClassPathEntries(distinctPackages, distinctClassesAndSources)
   }
 
@@ -99,14 +109,12 @@ case class AggregateClassPath(aggregates: Seq[ClassPath]) extends ClassPath {
    * creates an entry containing both of them. If there would be more than one class or source
    * entries for the same class it always would use the first entry of each type found on a classpath.
    */
-  private def mergeClassesAndSources(entries: Seq[Seq[ClassRepresentation]]): Seq[ClassRepresentation] = {
+  private def mergeClassesAndSources(entries: Seq[ClassRepresentation]): Seq[ClassRepresentation] = {
     var count = 0
     val indices = collection.mutable.HashMap[String, Int]()
-    val mergedEntries = new ArrayBuffer[ClassRepresentation](1024)
-
+    val mergedEntries = new ArrayBuffer[ClassRepresentation](entries.size)
     for {
-      partOfEntries <- entries
-      entry <- partOfEntries
+      entry <- entries
     } {
       val name = entry.name
       if (indices contains name) {

src/compiler/scala/tools/nsc/classpath/ClassPath.scala

@@ -32,6 +32,21 @@ trait SourceFileEntry extends ClassRepresentation {
   def file: AbstractFile
 }
 
+case class PackageName(dottedString: String) {
+  def isRoot: Boolean = dottedString.isEmpty
+  val dirPathTrailingSlash: String = FileUtils.dirPath(dottedString) + "/"
+
+  def entryName(entry: String): String = {
+    if (isRoot) entry else {
+      val builder = new java.lang.StringBuilder(dottedString.length + 1 + entry.length)
+      builder.append(dottedString)
+      builder.append('.')
+      builder.append(entry)
+      builder.toString
+    }
+  }
+}
+
 trait PackageEntry {
   def name: String
 }
@@ -61,10 +76,10 @@ private[nsc] case class PackageEntryImpl(name: String) extends PackageEntry
 
 private[nsc] trait NoSourcePaths {
   final def asSourcePathString: String = ""
-  final private[nsc] def sources(inPackage: String): Seq[SourceFileEntry] = Seq.empty
+  final private[nsc] def sources(inPackage: PackageName): Seq[SourceFileEntry] = Seq.empty
 }
 
 private[nsc] trait NoClassPaths {
   final def findClassFile(className: String): Option[AbstractFile] = None
-  private[nsc] final def classes(inPackage: String): Seq[ClassFileEntry] = Seq.empty
+  private[nsc] final def classes(inPackage: PackageName): Seq[ClassFileEntry] = Seq.empty
 }

src/compiler/scala/tools/nsc/classpath/DirectoryClassPath.scala

@@ -18,7 +18,7 @@ import java.nio.file.{FileSystems, Files}
 import java.util
 
 import scala.reflect.io.{AbstractFile, PlainFile, PlainNioFile}
-import scala.tools.nsc.util.{ClassPath, ClassRepresentation}
+import scala.tools.nsc.util.{ClassPath, ClassRepresentation, EfficientClassPath}
 import FileUtils._
 import scala.collection.JavaConverters._
 import scala.reflect.internal.JDK9Reflectors
@@ -32,7 +32,7 @@ import scala.tools.nsc.classpath.PackageNameUtils.{packageContains, separatePkgA
  * when we have a name of a package.
  * It abstracts over the file representation to work with both JFile and AbstractFile.
  */
-trait DirectoryLookup[FileEntryType <: ClassRepresentation] extends ClassPath {
+trait DirectoryLookup[FileEntryType <: ClassRepresentation] extends EfficientClassPath {
   type F
 
   val dir: F
@@ -47,28 +47,26 @@ trait DirectoryLookup[FileEntryType <: ClassRepresentation] extends ClassPath {
   protected def createFileEntry(file: AbstractFile): FileEntryType
   protected def isMatchingFile(f: F): Boolean
 
-  private def getDirectory(forPackage: String): Option[F] = {
-    if (forPackage == ClassPath.RootPackage) {
+  private def getDirectory(forPackage: PackageName): Option[F] = {
+    if (forPackage.isRoot) {
       Some(dir)
     } else {
-      val packageDirName = FileUtils.dirPath(forPackage)
-      getSubDir(packageDirName)
+      getSubDir(forPackage.dirPathTrailingSlash)
     }
   }
-  override private[nsc] def hasPackage(pkg: String) = getDirectory(pkg).isDefined
+  override private[nsc] def hasPackage(pkg: PackageName) = getDirectory(pkg).isDefined
 
-  private[nsc] def packages(inPackage: String): Seq[PackageEntry] = {
+  private[nsc] def packages(inPackage: PackageName): Seq[PackageEntry] = {
     val dirForPackage = getDirectory(inPackage)
 
     val nestedDirs: Array[F] = dirForPackage match {
       case None => emptyFiles
       case Some(directory) => listChildren(directory, Some(isPackage))
     }
-    val prefix = PackageNameUtils.packagePrefix(inPackage)
-    nestedDirs.map(f => PackageEntryImpl(prefix + getName(f)))
+    nestedDirs.map(f => PackageEntryImpl(inPackage.entryName(getName(f))))
   }
 
-  protected def files(inPackage: String): Seq[FileEntryType] = {
+  protected def files(inPackage: PackageName): Seq[FileEntryType] = {
     val dirForPackage = getDirectory(inPackage)
     val files: Array[F] = dirForPackage match {
       case None => emptyFiles
@@ -77,22 +75,18 @@ trait DirectoryLookup[FileEntryType <: ClassRepresentation] extends ClassPath {
     files.map(f => createFileEntry(toAbstractFile(f)))
   }
 
-  private[nsc] def list(inPackage: String): ClassPathEntries = {
+  override private[nsc] def list(inPackage: PackageName, onPackageEntry: PackageEntry => Unit, onClassesAndSources: ClassRepresentation => Unit): Unit = {
     val dirForPackage = getDirectory(inPackage)
-    val files: Array[F] = dirForPackage match {
-      case None => emptyFiles
-      case Some(directory) => listChildren(directory)
-    }
-    val packagePrefix = PackageNameUtils.packagePrefix(inPackage)
-    val packageBuf = collection.mutable.ArrayBuffer.empty[PackageEntry]
-    val fileBuf = collection.mutable.ArrayBuffer.empty[FileEntryType]
-    for (file <- files) {
-      if (isPackage(file))
-        packageBuf += PackageEntryImpl(packagePrefix + getName(file))
-      else if (isMatchingFile(file))
-        fileBuf += createFileEntry(toAbstractFile(file))
+    dirForPackage match {
+      case None =>
+      case Some(directory) =>
+        for (file <- listChildren(directory)) {
+          if (isPackage(file))
+            onPackageEntry(PackageEntryImpl(inPackage.entryName(getName(file))))
+          else if (isMatchingFile(file))
+            onClassesAndSources(createFileEntry(toAbstractFile(file)))
+        }
     }
-    ClassPathEntries(packageBuf, fileBuf)
   }
 }
 
@@ -196,21 +190,21 @@ final class JrtClassPath(fs: java.nio.file.FileSystem) extends ClassPath with No
   }
 
   /** Empty string represents root package */
-  override private[nsc] def hasPackage(pkg: String) = packageToModuleBases.contains(pkg)
-  override private[nsc] def packages(inPackage: String): Seq[PackageEntry] = {
-    packageToModuleBases.keysIterator.filter(pack => packageContains(inPackage, pack)).map(PackageEntryImpl(_)).toVector
+  override private[nsc] def hasPackage(pkg: PackageName) = packageToModuleBases.contains(pkg.dottedString)
+  override private[nsc] def packages(inPackage: PackageName): Seq[PackageEntry] = {
+    packageToModuleBases.keysIterator.filter(pack => packageContains(inPackage.dottedString, pack)).map(PackageEntryImpl(_)).toVector
   }
-  private[nsc] def classes(inPackage: String): Seq[ClassFileEntry] = {
-    if (inPackage == "") Nil
+  private[nsc] def classes(inPackage: PackageName): Seq[ClassFileEntry] = {
+    if (inPackage.isRoot) Nil
     else {
-      packageToModuleBases.getOrElse(inPackage, Nil).flatMap(x =>
-        Files.list(x.resolve(inPackage.replace('.', '/'))).iterator().asScala.filter(_.getFileName.toString.endsWith(".class"))).map(x =>
+      packageToModuleBases.getOrElse(inPackage.dottedString, Nil).flatMap(x =>
+        Files.list(x.resolve(inPackage.dirPathTrailingSlash)).iterator().asScala.filter(_.getFileName.toString.endsWith(".class"))).map(x =>
         ClassFileEntryImpl(new PlainNioFile(x))).toVector
     }
   }
 
-  override private[nsc] def list(inPackage: String): ClassPathEntries =
-    if (inPackage == "") ClassPathEntries(packages(inPackage), Nil)
+  override private[nsc] def list(inPackage: PackageName): ClassPathEntries =
+    if (inPackage.isRoot) ClassPathEntries(packages(inPackage), Nil)
     else ClassPathEntries(packages(inPackage), classes(inPackage))
 
   def asURLs: Seq[URL] = Seq(new URL("jrt:/"))
@@ -262,21 +256,21 @@ final class CtSymClassPath(ctSym: java.nio.file.Path, release: Int) extends Clas
   }
 
   /** Empty string represents root package */
-  override private[nsc] def hasPackage(pkg: String) = packageIndex.contains(pkg)
-  override private[nsc] def packages(inPackage: String): Seq[PackageEntry] = {
-    packageIndex.keysIterator.filter(pack => packageContains(inPackage, pack)).map(PackageEntryImpl(_)).toVector
+  override private[nsc] def hasPackage(pkg: PackageName) = packageIndex.contains(pkg.dottedString)
+  override private[nsc] def packages(inPackage: PackageName): Seq[PackageEntry] = {
+    packageIndex.keysIterator.filter(pack => packageContains(inPackage.dottedString, pack)).map(PackageEntryImpl(_)).toVector
   }
-  private[nsc] def classes(inPackage: String): Seq[ClassFileEntry] = {
-    if (inPackage == "") Nil
+  private[nsc] def classes(inPackage: PackageName): Seq[ClassFileEntry] = {
+    if (inPackage.isRoot) Nil
     else {
-      val sigFiles = packageIndex.getOrElse(inPackage, Nil).iterator.flatMap(p =>
+      val sigFiles = packageIndex.getOrElse(inPackage.dottedString, Nil).iterator.flatMap(p =>
         Files.list(p).iterator().asScala.filter(_.getFileName.toString.endsWith(".sig")))
       sigFiles.map(f => ClassFileEntryImpl(new PlainNioFile(f))).toVector
     }
   }
 
-  override private[nsc] def list(inPackage: String): ClassPathEntries =
-    if (inPackage == "") ClassPathEntries(packages(inPackage), Nil)
+  override private[nsc] def list(inPackage: PackageName): ClassPathEntries =
+    if (inPackage.isRoot) ClassPathEntries(packages(inPackage), Nil)
     else ClassPathEntries(packages(inPackage), classes(inPackage))
 
   def asURLs: Seq[URL] = Nil
@@ -310,7 +304,7 @@ case class DirectoryClassPath(dir: File) extends JFileDirectoryLookup[ClassFileE
   protected def createFileEntry(file: AbstractFile): ClassFileEntryImpl = ClassFileEntryImpl(file)
   protected def isMatchingFile(f: File): Boolean = f.isClass
 
-  private[nsc] def classes(inPackage: String): Seq[ClassFileEntry] = files(inPackage)
+  private[nsc] def classes(inPackage: PackageName): Seq[ClassFileEntry] = files(inPackage)
 }
 
 case class DirectorySourcePath(dir: File) extends JFileDirectoryLookup[SourceFileEntryImpl] with NoClassPaths {
@@ -334,5 +328,5 @@ case class DirectorySourcePath(dir: File) extends JFileDirectoryLookup[SourceFil
     }
   }
 
-  private[nsc] def sources(inPackage: String): Seq[SourceFileEntry] = files(inPackage)
+  private[nsc] def sources(inPackage: PackageName): Seq[SourceFileEntry] = files(inPackage)
 }

src/compiler/scala/tools/nsc/classpath/VirtualDirectoryClassPath.scala

@@ -45,7 +45,7 @@ case class VirtualDirectoryClassPath(dir: VirtualDirectory) extends ClassPath wi
     Option(AbstractFileClassLoader.lookupPath(dir)(relativePath split '/', directory = false))
   }
 
-  private[nsc] def classes(inPackage: String): Seq[ClassFileEntry] = files(inPackage)
+  private[nsc] def classes(inPackage: PackageName): Seq[ClassFileEntry] = files(inPackage)
 
   protected def createFileEntry(file: AbstractFile): ClassFileEntryImpl = ClassFileEntryImpl(file)
   protected def isMatchingFile(f: AbstractFile): Boolean = f.isClass

src/compiler/scala/tools/nsc/classpath/ZipAndJarFileLookupFactory.scala

@@ -59,15 +59,15 @@ object ZipAndJarClassPathFactory extends ZipAndJarFileLookupFactory {
 
     override def findClassFile(className: String): Option[AbstractFile] = {
       val (pkg, simpleClassName) = PackageNameUtils.separatePkgAndClassNames(className)
-      file(pkg, simpleClassName + ".class").map(_.file)
+      file(PackageName(pkg), simpleClassName + ".class").map(_.file)
     }
     // This method is performance sensitive as it is used by SBT's ExtractDependencies phase.
     override def findClass(className: String): Option[ClassRepresentation] = {
       val (pkg, simpleClassName) = PackageNameUtils.separatePkgAndClassNames(className)
-      file(pkg, simpleClassName + ".class")
+      file(PackageName(pkg), simpleClassName + ".class")
     }
 
-    override private[nsc] def classes(inPackage: String): Seq[ClassFileEntry] = files(inPackage)
+    override private[nsc] def classes(inPackage: PackageName): Seq[ClassFileEntry] = files(inPackage)
 
     override protected def createFileEntry(file: FileZipArchive#Entry): ClassFileEntryImpl = ClassFileEntryImpl(file)
     override protected def isRequiredFileType(file: AbstractFile): Boolean = file.isClass
@@ -83,7 +83,7 @@ object ZipAndJarClassPathFactory extends ZipAndJarFileLookupFactory {
   private case class ManifestResourcesClassPath(file: ManifestResources) extends ClassPath with NoSourcePaths with Closeable {
     override def findClassFile(className: String): Option[AbstractFile] = {
       val (pkg, simpleClassName) = PackageNameUtils.separatePkgAndClassNames(className)
-      classes(pkg).find(_.name == simpleClassName).map(_.file)
+      classes(PackageName(pkg)).find(_.name == simpleClassName).map(_.file)
     }
 
     override def asClassPathStrings: Seq[String] = Seq(file.path)
@@ -135,22 +135,21 @@ object ZipAndJarClassPathFactory extends ZipAndJarFileLookupFactory {
       packages
     }
 
-    override private[nsc] def packages(inPackage: String): Seq[PackageEntry] = cachedPackages.get(inPackage) match {
+    override private[nsc] def packages(inPackage: PackageName): Seq[PackageEntry] = cachedPackages.get(inPackage.dottedString) match {
       case None => Seq.empty
       case Some(PackageFileInfo(_, subpackages)) =>
-        val prefix = PackageNameUtils.packagePrefix(inPackage)
-        subpackages.map(packageFile => PackageEntryImpl(prefix + packageFile.name))
+        subpackages.map(packageFile => PackageEntryImpl(inPackage.entryName(packageFile.name)))
     }
 
-    override private[nsc] def classes(inPackage: String): Seq[ClassFileEntry] = cachedPackages.get(inPackage) match {
+    override private[nsc] def classes(inPackage: PackageName): Seq[ClassFileEntry] = cachedPackages.get(inPackage.dottedString) match {
       case None => Seq.empty
       case Some(PackageFileInfo(pkg, _)) =>
         (for (file <- pkg if file.isClass) yield ClassFileEntryImpl(file))(collection.breakOut)
     }
 
 
-    override private[nsc] def hasPackage(pkg: String) = cachedPackages.contains(pkg)
-    override private[nsc] def list(inPackage: String): ClassPathEntries = ClassPathEntries(packages(inPackage), classes(inPackage))
+    override private[nsc] def hasPackage(pkg: PackageName) = cachedPackages.contains(pkg.dottedString)
+    override private[nsc] def list(inPackage: PackageName): ClassPathEntries = ClassPathEntries(packages(inPackage), classes(inPackage))
   }
 
   private object ManifestResourcesClassPath {
@@ -183,7 +182,7 @@ object ZipAndJarSourcePathFactory extends ZipAndJarFileLookupFactory {
 
     override def asSourcePathString: String = asClassPathString
 
-    override private[nsc] def sources(inPackage: String): Seq[SourceFileEntry] = files(inPackage)
+    override private[nsc] def sources(inPackage: PackageName): Seq[SourceFileEntry] = files(inPackage)
 
     override protected def createFileEntry(file: FileZipArchive#Entry): SourceFileEntryImpl = SourceFileEntryImpl(file)
     override protected def isRequiredFileType(file: AbstractFile): Boolean = file.isScalaOrJavaSource