Merge pull request #9557 from retronym/faster/zipfs

JAR reading: more accurate and widespread caching, less read contention
scala · May 18, 2021 · 21a8a7a · 21a8a7a
2 parents df9bb3e + d0396cd
commit 21a8a7a
Show file tree

Hide file tree

Showing 9 changed files with 103 additions and 71 deletions.
diff --git a/project/MimaFilters.scala b/project/MimaFilters.scala
@@ -28,6 +28,11 @@ object MimaFilters extends AutoPlugin {
 
     // #9166 add missing serialVersionUID
     ProblemFilters.exclude[MissingFieldProblem]("*.serialVersionUID"),
+
+    // private[scala] Internal API
+    ProblemFilters.exclude[IncompatibleMethTypeProblem]("scala.reflect.io.FileZipArchive#LeakyEntry.this"),
+    ProblemFilters.exclude[IncompatibleMethTypeProblem]("scala.reflect.io.FileZipArchive#LeakyEntry.this"),
+    ProblemFilters.exclude[MissingClassProblem]("scala.reflect.io.FileZipArchive$zipFilePool$"),
   )
 
   override val buildSettings = Seq(

diff --git a/src/compiler/scala/tools/nsc/classpath/DirectoryClassPath.scala b/src/compiler/scala/tools/nsc/classpath/DirectoryClassPath.scala
@@ -14,7 +14,6 @@ package scala.tools.nsc.classpath
 
 import java.io.{Closeable, File}
 import java.net.URL
-import java.nio.file.{FileSystems, Files}
 import java.util
 
 import scala.reflect.io.{AbstractFile, PlainFile, PlainNioFile}
@@ -130,6 +129,8 @@ trait JFileDirectoryLookup[FileEntryType <: ClassRepresentation] extends Directo
 
 object JrtClassPath {
   import java.nio.file._, java.net.URI
+  private val jrtClassPathCache = new FileBasedCache[Unit, JrtClassPath]()
+  private val ctSymClassPathCache = new FileBasedCache[Unit, CtSymClassPath]()
   def apply(release: Option[String], closeableRegistry: CloseableRegistry): Option[ClassPath] = {
     import scala.util.Properties._
     if (!isJavaAtLeast("9")) None
@@ -148,8 +149,7 @@ object JrtClassPath {
             val ctSym = Paths.get(javaHome).resolve("lib").resolve("ct.sym")
             if (Files.notExists(ctSym)) None
             else {
-              val classPath = new CtSymClassPath(ctSym, v.toInt)
-              closeableRegistry.registerClosable(classPath)
+              val classPath = ctSymClassPathCache.getOrCreate((), ctSym :: Nil, () => new CtSymClassPath(ctSym, v.toInt), closeableRegistry, true)
               Some(classPath)
             }
           } catch {
@@ -158,7 +158,8 @@ object JrtClassPath {
         case _ =>
           try {
             val fs = FileSystems.getFileSystem(URI.create("jrt:/"))
-            Some(new JrtClassPath(fs))
+            val classPath = jrtClassPathCache.getOrCreate((), Nil, () => new JrtClassPath(fs), closeableRegistry, false)
+            Some(classPath)
           } catch {
             case _: ProviderNotFoundException | _: FileSystemNotFoundException => None
           }

diff --git a/src/compiler/scala/tools/nsc/classpath/ZipAndJarFileLookupFactory.scala b/src/compiler/scala/tools/nsc/classpath/ZipAndJarFileLookupFactory.scala
@@ -14,16 +14,18 @@ package scala.tools.nsc.classpath
 
 import java.io.{Closeable, File}
 import java.net.URL
-import java.nio.file.Files
+import java.nio.file.{Files, InvalidPathException}
 import java.nio.file.attribute.{BasicFileAttributes, FileTime}
+import java.nio.file.spi.FileSystemProvider
 import java.util.{Timer, TimerTask}
 import java.util.concurrent.atomic.AtomicInteger
-
+import java.util.zip.ZipError
 import scala.annotation.tailrec
 import scala.reflect.io.{AbstractFile, FileZipArchive, ManifestResources}
 import scala.tools.nsc.util.{ClassPath, ClassRepresentation}
 import scala.tools.nsc.{CloseableRegistry, Settings}
 import FileUtils._
+import scala.reflect.internal.FatalError
 import scala.tools.nsc.io.Jar
 
 /**
@@ -32,21 +34,23 @@ import scala.tools.nsc.io.Jar
  * when there are a lot of projects having a lot of common dependencies.
  */
 sealed trait ZipAndJarFileLookupFactory {
-  private val cache = new FileBasedCache[ClassPath with Closeable]
+  case class ZipSettings(releaseValue: Option[String])
+  private val cache = new FileBasedCache[ZipSettings, ClassPath with Closeable]
 
   def create(zipFile: AbstractFile, settings: Settings, closeableRegistry: CloseableRegistry): ClassPath = {
     val disabled = (settings.YdisableFlatCpCaching.value && !settings.YforceFlatCpCaching.value) || zipFile.file == null
+    val zipSettings = ZipSettings(settings.releaseValue)
     cache.checkCacheability(zipFile.toURL :: Nil, checkStamps = true, disableCache = disabled) match {
       case Left(_) =>
-        val result: ClassPath with Closeable = createForZipFile(zipFile, settings.releaseValue)
+        val result: ClassPath with Closeable = createForZipFile(zipFile, zipSettings)
         closeableRegistry.registerClosable(result)
         result
       case Right(Seq(path)) =>
-        cache.getOrCreate(List(path), () => createForZipFile(zipFile, settings.releaseValue), closeableRegistry, checkStamps = true)
+        cache.getOrCreate(zipSettings, List(path), () => createForZipFile(zipFile, zipSettings), closeableRegistry, checkStamps = true)
     }
   }
 
-  protected def createForZipFile(zipFile: AbstractFile, release: Option[String]): ClassPath with Closeable
+  protected def createForZipFile(zipFile: AbstractFile, zipSettings: ZipSettings): ClassPath with Closeable
 }
 
 /**
@@ -158,9 +162,9 @@ object ZipAndJarClassPathFactory extends ZipAndJarFileLookupFactory {
     case class PackageInfo(packageName: String, subpackages: List[AbstractFile])
   }
 
-  override protected def createForZipFile(zipFile: AbstractFile, release: Option[String]): ClassPath with Closeable =
+  override protected def createForZipFile(zipFile: AbstractFile, zipSettings: ZipSettings): ClassPath with Closeable =
     if (zipFile.file == null) createWithoutUnderlyingFile(zipFile)
-    else ZipArchiveClassPath(zipFile.file, release)
+    else ZipArchiveClassPath(zipFile.file, zipSettings.releaseValue)
 
   private def createWithoutUnderlyingFile(zipFile: AbstractFile) = zipFile match {
     case manifestRes: ManifestResources =>
@@ -189,13 +193,13 @@ object ZipAndJarSourcePathFactory extends ZipAndJarFileLookupFactory {
     override protected def isRequiredFileType(file: AbstractFile): Boolean = file.isScalaOrJavaSource
   }
 
-  override protected def createForZipFile(zipFile: AbstractFile, release: Option[String]): ClassPath with Closeable = ZipArchiveSourcePath(zipFile.file)
+  override protected def createForZipFile(zipFile: AbstractFile, zipSettings: ZipSettings): ClassPath with Closeable = ZipArchiveSourcePath(zipFile.file)
 }
 
-final class FileBasedCache[T] {
+final class FileBasedCache[K, T] {
   import java.nio.file.Path
   private case class Stamp(lastModified: FileTime, size: Long, fileKey: Object)
-  private case class Entry(stamps: Seq[Stamp], t: T) {
+  private case class Entry(k: K, stamps: Seq[Stamp], t: T) {
     val referenceCount: AtomicInteger = new AtomicInteger(1)
     var timerTask: TimerTask = null
     def cancelTimer(): Unit = {
@@ -205,9 +209,9 @@ final class FileBasedCache[T] {
       }
     }
   }
-  private val cache = collection.mutable.Map.empty[Seq[Path], Entry]
+  private val cache = collection.mutable.Map.empty[(K, Seq[Path]), Entry]
 
-  private def referenceCountDecrementer(e: Entry, paths: Seq[Path]): Closeable = {
+  private def referenceCountDecrementer(e: Entry, key: (K, Seq[Path])): Closeable = {
     // Cancel the deferred close timer (if any) that was started when the reference count
     // last dropped to zero.
     e.cancelTimer()
@@ -227,7 +231,7 @@ final class FileBasedCache[T] {
                       override def run(): Unit = {
                         cache.synchronized {
                           if (e.referenceCount.compareAndSet(0, -1)) {
-                            cache.remove(paths)
+                            cache.remove(key)
                             cl.close()
                           }
                         }
@@ -259,7 +263,7 @@ final class FileBasedCache[T] {
     }
   }
 
-  def getOrCreate(paths: Seq[Path], create: () => T, closeableRegistry: CloseableRegistry, checkStamps: Boolean): T = cache.synchronized {
+  def getOrCreate(k: K, paths: Seq[Path], create: () => T, closeableRegistry: CloseableRegistry, checkStamps: Boolean): T = cache.synchronized {
     val stamps = if (!checkStamps) Nil else paths.map { path =>
       try {
       val attrs = Files.readAttributes(path, classOf[BasicFileAttributes])
@@ -273,14 +277,15 @@ final class FileBasedCache[T] {
           Stamp(FileTime.fromMillis(0), -1, new Object)
       }
     }
+    val key = (k, paths)
 
-    cache.get(paths) match {
-      case Some(e@Entry(cachedStamps, cached)) =>
+    cache.get(key) match {
+      case Some(e@Entry(k1, cachedStamps, cached)) =>
         if (!checkStamps || cachedStamps == stamps) {
           // Cache hit
           val count = e.referenceCount.incrementAndGet()
           assert(count > 0, (stamps, count))
-          closeableRegistry.registerClosable(referenceCountDecrementer(e, paths))
+          closeableRegistry.registerClosable(referenceCountDecrementer(e, (k1, paths)))
           cached
         } else {
           // Cache miss: we found an entry but the underlying files have been modified
@@ -293,17 +298,17 @@ final class FileBasedCache[T] {
               }
           }
           val value = create()
-          val entry = Entry(stamps, value)
-          cache.put(paths, entry)
-          closeableRegistry.registerClosable(referenceCountDecrementer(entry, paths))
+          val entry = Entry(k, stamps, value)
+          cache.put(key, entry)
+          closeableRegistry.registerClosable(referenceCountDecrementer(entry, key))
           value
         }
       case _ =>
         // Cache miss
         val value = create()
-        val entry = Entry(stamps, value)
-        cache.put(paths, entry)
-        closeableRegistry.registerClosable(referenceCountDecrementer(entry, paths))
+        val entry = Entry(k, stamps, value)
+        cache.put(key, entry)
+        closeableRegistry.registerClosable(referenceCountDecrementer(entry, key))
         value
     }
   }

diff --git a/src/compiler/scala/tools/nsc/plugins/Plugin.scala b/src/compiler/scala/tools/nsc/plugins/Plugin.scala
@@ -112,7 +112,7 @@ object Plugin {
 
   val PluginXML = "scalac-plugin.xml"
 
-  private[nsc] val pluginClassLoadersCache = new FileBasedCache[ScalaClassLoader.URLClassLoader]()
+  private[nsc] val pluginClassLoadersCache = new FileBasedCache[Unit, ScalaClassLoader.URLClassLoader]()
 
   type AnyClass = Class[_]
 

diff --git a/src/compiler/scala/tools/nsc/plugins/Plugins.scala b/src/compiler/scala/tools/nsc/plugins/Plugins.scala
@@ -106,7 +106,7 @@ trait Plugins { global: Global =>
         closeableRegistry.registerClosable(loader)
         loader
       case Right(paths) =>
-        cache.getOrCreate(classpath.map(_.jfile.toPath()), newLoader, closeableRegistry, checkStamps)
+        cache.getOrCreate((), classpath.map(_.jfile.toPath()), newLoader, closeableRegistry, checkStamps)
     }
   }
 

diff --git a/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala b/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala
@@ -199,15 +199,16 @@ abstract class SymbolLoaders {
     }
   }
   private def nameOf(classRep: ClassRepresentation): TermName = {
-    while(true) {
-      val len = classRep.nameChars(nameCharBuffer)
-      if (len == -1) nameCharBuffer = new Array[Char](nameCharBuffer.length * 2)
-      else return newTermName(nameCharBuffer, 0, len)
+    val name = classRep.name
+    val nameLength = name.length
+    if (nameLength <= nameCharBuffer.length) {
+      name.getChars(0, nameLength, nameCharBuffer, 0)
+      newTermName(nameCharBuffer, 0, nameLength)
+    } else {
+      newTermName(name)
     }
-    throw new IllegalStateException()
   }
-  private var nameCharBuffer = new Array[Char](256)
-
+  private val nameCharBuffer = new Array[Char](512)
 
   /**
    * A lazy type that completes itself by calling parameter doComplete.

diff --git a/src/compiler/scala/tools/nsc/typechecker/Macros.scala b/src/compiler/scala/tools/nsc/typechecker/Macros.scala
@@ -96,7 +96,7 @@ trait Macros extends MacroRuntimes with Traces with Helpers {
         closeableRegistry.registerClosable(loader)
         loader
       case Right(paths) =>
-        cache.getOrCreate(paths, newLoader, closeableRegistry, checkStamps)
+        cache.getOrCreate((), paths, newLoader, closeableRegistry, checkStamps)
     }
   }
 
@@ -973,7 +973,7 @@ trait Macros extends MacroRuntimes with Traces with Helpers {
 
 object Macros {
   final val macroClassLoadersCache =
-    new scala.tools.nsc.classpath.FileBasedCache[ScalaClassLoader.URLClassLoader]()
+    new scala.tools.nsc.classpath.FileBasedCache[Unit, ScalaClassLoader.URLClassLoader]()
 }
 
 trait MacrosStats {

diff --git a/src/compiler/scala/tools/nsc/util/ClassPath.scala b/src/compiler/scala/tools/nsc/util/ClassPath.scala
@@ -204,17 +204,6 @@ object ClassPath {
 trait ClassRepresentation {
   def fileName: String
   def name: String
-  /** Low level way to extract the entry name without allocation. */
-  final def nameChars(buffer: Array[Char]): Int = {
-    val ix = fileName.lastIndexOf('.')
-    val nameLength = if (ix < 0) fileName.length else ix
-    if (nameLength > buffer.length)
-      -1
-    else {
-      fileName.getChars(0, fileName.lastIndexOf('.'), buffer, 0)
-      nameLength
-    }
-  }
   def binary: Option[AbstractFile]
   def source: Option[AbstractFile]
 }