diff --git a/compiler/src/dotty/tools/dotc/classpath/AggregateClassPath.scala b/compiler/src/dotty/tools/dotc/classpath/AggregateClassPath.scala index c6b15d773614..a6d22a9719c8 100644 --- a/compiler/src/dotty/tools/dotc/classpath/AggregateClassPath.scala +++ b/compiler/src/dotty/tools/dotc/classpath/AggregateClassPath.scala @@ -19,32 +19,28 @@ import dotty.tools.io.{ AbstractFile, ClassPath, ClassRepresentation } */ case class AggregateClassPath(aggregates: Seq[ClassPath]) extends ClassPath { override def findClassFile(className: String): Option[AbstractFile] = { - @tailrec - def find(aggregates: Seq[ClassPath]): Option[AbstractFile] = - if (aggregates.nonEmpty) { - val classFile = aggregates.head.findClassFile(className) - if (classFile.isDefined) classFile - else find(aggregates.tail) - } else None - - find(aggregates) + val (pkg, _) = PackageNameUtils.separatePkgAndClassNames(className) + aggregatesForPackage(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))) } override def findClass(className: String): Option[ClassRepresentation] = { - @tailrec - def findEntry(aggregates: Seq[ClassPath], isSource: Boolean): Option[ClassRepresentation] = - if (aggregates.nonEmpty) { - val entry = aggregates.head.findClass(className) match { - case s @ Some(_: SourceFileEntry) if isSource => s - case s @ Some(_: ClassFileEntry) if !isSource => s - case _ => None - } - if (entry.isDefined) entry - else findEntry(aggregates.tail, isSource) - } else None - - val classEntry = findEntry(aggregates, isSource = false) - val sourceEntry = findEntry(aggregates, isSource = true) + val (pkg, _) = PackageNameUtils.separatePkgAndClassNames(className) + + def findEntry(isSource: Boolean): Option[ClassRepresentation] = { + aggregatesForPackage(pkg).iterator.map(_.findClass(className)).collectFirst { + case Some(s: SourceFileEntry) if isSource => s + case Some(s: ClassFileEntry) if !isSource => s + } + } + + val classEntry = findEntry(isSource = false) + val sourceEntry = findEntry(isSource = true) (classEntry, sourceEntry) match { case (Some(c: ClassFileEntry), Some(s: SourceFileEntry)) => Some(ClassAndSourceFilesEntry(c.file, s.file)) @@ -70,6 +66,7 @@ case class AggregateClassPath(aggregates: Seq[ClassPath]) extends ClassPath { override private[dotty] def sources(inPackage: String): Seq[SourceFileEntry] = getDistinctEntries(_.sources(inPackage)) + override private[dotty] def hasPackage(pkg: String) = aggregates.exists(_.hasPackage(pkg)) override private[dotty] def list(inPackage: String): ClassPathEntries = { val (packages, classesAndSources) = aggregates.map { cp => try { diff --git a/compiler/src/dotty/tools/dotc/classpath/DirectoryClassPath.scala b/compiler/src/dotty/tools/dotc/classpath/DirectoryClassPath.scala index 0a3496be4f50..91620151bbb6 100644 --- a/compiler/src/dotty/tools/dotc/classpath/DirectoryClassPath.scala +++ b/compiler/src/dotty/tools/dotc/classpath/DirectoryClassPath.scala @@ -45,6 +45,8 @@ trait DirectoryLookup[FileEntryType <: ClassRepresentation] extends ClassPath { } } + override private[dotty] def hasPackage(pkg: String) = getDirectory(pkg).isDefined + private[dotty] def packages(inPackage: String): Seq[PackageEntry] = { val dirForPackage = getDirectory(inPackage) val nestedDirs: Array[F] = dirForPackage match { @@ -161,6 +163,9 @@ final class JrtClassPath(fs: java.nio.file.FileSystem) extends ClassPath with No ps.map(p => (p.toString.stripPrefix("/packages/"), lookup(p))).toMap } + /** Empty string represents root package */ + override private[dotty] def hasPackage(pkg: String) = packageToModuleBases.contains(pkg) + override private[dotty] def packages(inPackage: String): Seq[PackageEntry] = { def matches(packageDottedName: String) = if (packageDottedName.contains(".")) diff --git a/compiler/src/dotty/tools/dotc/classpath/ZipAndJarFileLookupFactory.scala b/compiler/src/dotty/tools/dotc/classpath/ZipAndJarFileLookupFactory.scala index 648fb59497c0..2e3250bf174a 100644 --- a/compiler/src/dotty/tools/dotc/classpath/ZipAndJarFileLookupFactory.scala +++ b/compiler/src/dotty/tools/dotc/classpath/ZipAndJarFileLookupFactory.scala @@ -5,38 +5,31 @@ package dotty.tools.dotc.classpath import java.io.File import java.net.URL +import java.nio.file.Files +import java.nio.file.attribute.{BasicFileAttributes, FileTime} + import scala.annotation.tailrec -import dotty.tools.io.{AbstractFile, ClassPath, FileZipArchive, ManifestResources} -import dotty.tools.dotc.config.Settings +import dotty.tools.io.{AbstractFile, ClassPath, ClassRepresentation, FileZipArchive, ManifestResources} import dotty.tools.dotc.core.Contexts.Context import FileUtils._ /** * A trait providing an optional cache for classpath entries obtained from zip and jar files. - * It's possible to create such a cache assuming that entries in such files won't change (at - * least will be the same each time we'll load classpath during the lifetime of JVM process) - * - unlike class and source files in directories, which can be modified and recompiled. * It allows us to e.g. reduce significantly memory used by PresentationCompilers in Scala IDE * when there are a lot of projects having a lot of common dependencies. */ sealed trait ZipAndJarFileLookupFactory { - private val cache = collection.mutable.Map.empty[AbstractFile, ClassPath] + private val cache = new FileBasedCache[ClassPath] def create(zipFile: AbstractFile)(implicit ctx: Context): ClassPath = { - if (ctx.settings.YdisableFlatCpCaching.value) createForZipFile(zipFile) + if (ctx.settings.YdisableFlatCpCaching.value || zipFile.file == null) createForZipFile(zipFile) else createUsingCache(zipFile) } protected def createForZipFile(zipFile: AbstractFile): ClassPath - private def createUsingCache(zipFile: AbstractFile)(implicit ctx: Context): ClassPath = cache.synchronized { - def newClassPathInstance = { - if (ctx.settings.verbose.value || ctx.settings.Ylogcp.value) - println(s"$zipFile is not yet in the classpath cache") - createForZipFile(zipFile) - } - cache.getOrElseUpdate(zipFile, newClassPathInstance) - } + private def createUsingCache(zipFile: AbstractFile): ClassPath = + cache.getOrCreate(zipFile.file.toPath, () => createForZipFile(zipFile)) } /** @@ -50,7 +43,13 @@ object ZipAndJarClassPathFactory extends ZipAndJarFileLookupFactory { override def findClassFile(className: String): Option[AbstractFile] = { val (pkg, simpleClassName) = PackageNameUtils.separatePkgAndClassNames(className) - classes(pkg).find(_.name == simpleClassName).map(_.file) + file(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") } override private[dotty] def classes(inPackage: String): Seq[ClassFileEntry] = files(inPackage) @@ -133,6 +132,7 @@ object ZipAndJarClassPathFactory extends ZipAndJarFileLookupFactory { (for (file <- pkg if file.isClass) yield ClassFileEntryImpl(file))(collection.breakOut) } + override private[dotty] def hasPackage(pkg: String) = cachedPackages.contains(pkg) override private[dotty] def list(inPackage: String): ClassPathEntries = ClassPathEntries(packages(inPackage), classes(inPackage)) } @@ -173,3 +173,29 @@ object ZipAndJarSourcePathFactory extends ZipAndJarFileLookupFactory { override protected def createForZipFile(zipFile: AbstractFile): ClassPath = ZipArchiveSourcePath(zipFile.file) } + +final class FileBasedCache[T] { + private case class Stamp(lastModified: FileTime, fileKey: Object) + private val cache = collection.mutable.Map.empty[java.nio.file.Path, (Stamp, T)] + + def getOrCreate(path: java.nio.file.Path, create: () => T): T = cache.synchronized { + val attrs = Files.readAttributes(path, classOf[BasicFileAttributes]) + val lastModified = attrs.lastModifiedTime() + // only null on some platforms, but that's okay, we just use the last modified timestamp as our stamp + val fileKey = attrs.fileKey() + val stamp = Stamp(lastModified, fileKey) + cache.get(path) match { + case Some((cachedStamp, cached)) if cachedStamp == stamp => cached + case _ => + val value = create() + cache.put(path, (stamp, value)) + value + } + } + + def clear(): Unit = cache.synchronized { + // TODO support closing + // cache.valuesIterator.foreach(_.close()) + cache.clear() + } +} diff --git a/compiler/src/dotty/tools/dotc/classpath/ZipArchiveFileLookup.scala b/compiler/src/dotty/tools/dotc/classpath/ZipArchiveFileLookup.scala index 352a566097fe..2b7d039a6bb9 100644 --- a/compiler/src/dotty/tools/dotc/classpath/ZipArchiveFileLookup.scala +++ b/compiler/src/dotty/tools/dotc/classpath/ZipArchiveFileLookup.scala @@ -39,6 +39,14 @@ trait ZipArchiveFileLookup[FileEntryType <: ClassRepresentation] extends ClassPa entry <- dirEntry.iterator if isRequiredFileType(entry) } yield createFileEntry(entry) + protected def file(inPackage: String, name: String): Option[FileEntryType] = + for { + dirEntry <- findDirEntry(inPackage) + entry <- Option(dirEntry.lookupName(name, directory = false)) + if isRequiredFileType(entry) + } yield createFileEntry(entry) + + override private[dotty] def hasPackage(pkg: String) = findDirEntry(pkg).isDefined override private[dotty] def list(inPackage: String): ClassPathEntries = { val foundDirEntry = findDirEntry(inPackage) diff --git a/compiler/src/dotty/tools/io/ClassPath.scala b/compiler/src/dotty/tools/io/ClassPath.scala index b4cc426cfdd3..9b3ee03d167e 100644 --- a/compiler/src/dotty/tools/io/ClassPath.scala +++ b/compiler/src/dotty/tools/io/ClassPath.scala @@ -21,17 +21,42 @@ trait ClassPath { import dotty.tools.dotc.classpath._ def asURLs: Seq[URL] - /** Empty string represents root package */ + /* + * These methods are mostly used in the ClassPath implementation to implement the `list` and + * `findX` methods below. + * + * However, there are some other uses in the compiler, to implement `invalidateClassPathEntries`, + * which is used by the repl's `:require` (and maybe the spark repl, https://github.com/scala/scala/pull/4051). + * Using these methods directly is more efficient than calling `list`. + * + * The `inPackage` string is a full package name, e.g. "" or "scala.collection". + */ + + private[dotty] def hasPackage(pkg: String): Boolean private[dotty] def packages(inPackage: String): Seq[PackageEntry] private[dotty] def classes(inPackage: String): Seq[ClassFileEntry] private[dotty] def sources(inPackage: String): Seq[SourceFileEntry] - /** Allows to get entries for packages and classes merged with sources possibly in one pass. */ + /** + * Returns packages and classes (source or classfile) that are members of `inPackage` (not + * recursively). The `inPackage` string is a full package name, e.g., "scala.collection". + * + * This is the main method uses to find classes, see class `PackageLoader`. The + * `rootMirror.rootLoader` is created with `inPackage = ""`. + */ private[dotty] def list(inPackage: String): ClassPathEntries /** - * It returns both classes from class file and source files (as our base ClassRepresentation). - * So note that it's not so strictly related to findClassFile. + * Returns the class file and / or source file for a given external name, e.g., "java.lang.String". + * If there is both a class file and source file, the compiler can decide whether to read the + * class file or compile the source file. + * + * Internally this seems to be used only by `ScriptRunner`, but only to call `.isDefined`. That + * could probably be implemented differently. + * + * Externally, it is used by sbt's compiler interface: + * https://github.com/sbt/sbt/blob/v0.13.15/compile/interface/src/main/scala/xsbt/CompilerInterface.scala#L249 + * Jason has some improvements for that in the works (https://github.com/scala/bug/issues/10289#issuecomment-310022699) */ def findClass(className: String): Option[ClassRepresentation] = { // A default implementation which should be overridden, if we can create the more efficient @@ -43,6 +68,16 @@ trait ClassPath { foundClassFromClassFiles orElse findClassInSources } + + /** + * Returns the classfile for an external name, e.g., "java.lang.String". This method does not + * return source files. + * + * This method is used by the classfile parser. When parsing a Java class, its own inner classes + * are entered with a `ClassfileLoader` that parses the classfile returned by this method. + * It is also used in the backend, by the inliner, to obtain the bytecode when inlining from the + * classpath. It's also used by scalap. + */ def findClassFile(className: String): Option[AbstractFile] def asClassPathStrings: Seq[String] diff --git a/compiler/src/dotty/tools/io/ZipArchive.scala b/compiler/src/dotty/tools/io/ZipArchive.scala index b0aad16bb797..786163242b55 100644 --- a/compiler/src/dotty/tools/io/ZipArchive.scala +++ b/compiler/src/dotty/tools/io/ZipArchive.scala @@ -24,7 +24,7 @@ import scala.annotation.tailrec * ''Note: This library is considered experimental and should not be used unless you know what you are doing.'' */ object ZipArchive { - private[io] val closeZipFile = sys.props.get("scala.classpath.closeZip").map(_.toBoolean).getOrElse(false) + private[io] val closeZipFile = sys.props.get("scala.classpath.closeZip").exists(_.toBoolean) /** * @param file a File @@ -82,8 +82,8 @@ abstract class ZipArchive(override val file: JFile) extends AbstractFile with Eq override def isDirectory = true override def iterator: Iterator[Entry] = entries.valuesIterator override def lookupName(name: String, directory: Boolean): Entry = { - if (directory) entries(name + "/") - else entries(name) + if (directory) entries.get(name + "/").orNull + else entries.get(name).orNull } } diff --git a/compiler/test/dotty/tools/dotc/classpath/ZipAndJarFileLookupFactoryTest.scala b/compiler/test/dotty/tools/dotc/classpath/ZipAndJarFileLookupFactoryTest.scala new file mode 100644 index 000000000000..babbf97f20b2 --- /dev/null +++ b/compiler/test/dotty/tools/dotc/classpath/ZipAndJarFileLookupFactoryTest.scala @@ -0,0 +1,70 @@ +package dotty.tools.dotc.classpath + +import org.junit.Test +import java.nio.file._ +import java.nio.file.attribute.FileTime + +import dotty.tools.dotc.core.Contexts.{Context, ContextBase} +import dotty.tools.io.AbstractFile + + + +class ZipAndJarFileLookupFactoryTest { + @Test def cacheInvalidation(): Unit = { + if (scala.util.Properties.isWin) return // can't overwrite an open file on windows. + + val f = Files.createTempFile("test-", ".jar") + Files.delete(f) + + implicit val ctx: Context = new ContextBase().initialCtx + assert(!ctx.settings.YdisableFlatCpCaching.value) // we're testing with our JAR metadata caching enabled. + + def createCp = ZipAndJarClassPathFactory.create(AbstractFile.getFile(f.toFile)) + try { + createZip(f, Array(), "p1/C.class") + createZip(f, Array(), "p2/X.class") + createZip(f, Array(), "p3/Y.class") + val cp1 = createCp + assert(cp1.findClass("p1.C").isDefined) + + // We expect get a cache hit as the underlying zip hasn't changed + val cp2 = createCp + assert(cp2 eq cp1) + + // check things work after the cache hit + cp1.findClassFile("p2.X").get.toByteArray + + val lastMod1 = Files.getLastModifiedTime(f) + // Create a new zip at the same path with different contents and last modified + Files.delete(f) + createZip(f, Array(), "p1/D.class") + Files.setLastModifiedTime(f, FileTime.fromMillis(lastMod1.toMillis + 2000)) + + // Our classpath cache should create a new instance + val cp3 = createCp + assert(cp1 ne cp3, (System.identityHashCode(cp1), System.identityHashCode(cp3))) + // And that instance should see D, not C, in package p1. + assert(cp3.findClass("p1.C").isEmpty) + assert(cp3.findClass("p1.D").isDefined) + } finally Files.delete(f) + } + + def createZip(zipLocation: Path, content: Array[Byte], internalPath: String): Unit = { + val env = new java.util.HashMap[String, String]() + env.put("create", String.valueOf(Files.notExists(zipLocation))) + val fileUri = zipLocation.toUri + val zipUri = new java.net.URI("jar:" + fileUri.getScheme, fileUri.getPath, null) + val zipfs = FileSystems.newFileSystem(zipUri, env) + try { + try { + val internalTargetPath = zipfs.getPath(internalPath) + Files.createDirectories(internalTargetPath.getParent) + Files.write(internalTargetPath, content) + } finally { + if (zipfs != null) zipfs.close() + } + } finally { + zipfs.close() + } + } +} diff --git a/sbt-dotty/sbt-test/source-dependencies/binary/project/P.scala b/sbt-dotty/sbt-test/source-dependencies/binary/project/P.scala index a9df9a48619c..9cabc95a4e82 100644 --- a/sbt-dotty/sbt-test/source-dependencies/binary/project/P.scala +++ b/sbt-dotty/sbt-test/source-dependencies/binary/project/P.scala @@ -5,10 +5,6 @@ object B extends Build { lazy val dep = Project("dep", file("dep")) lazy val use = Project("use", file("use")) settings( - unmanagedJars in Compile <+= packageBin in (dep, Compile) map Attributed.blank, - - // Disable classpath caching since it does not invalidate modified jars - // (https://github.com/scala/bug/issues/10295) - scalacOptions += "-YdisableFlatCpCaching" + unmanagedJars in Compile <+= packageBin in (dep, Compile) map Attributed.blank ) }