Skip to content

Commit 1b79cb3

Browse files
authored
Reduce IO calls in extractDependencies (#18266)
fix two hotspots in extractDependencies - checking if `depFile` is a class, (does file IO on `isDirectory`), and resolving sibling class file of a TASTy file (more file IO). To fix this, only check the file extension, and also avoid unnecessary resolving of sibling `.class` file (always failing) when we have a `.scala` file as the associated file. Second we can cache the associated class file of a TASTy file. Also removes the second lookup in checkTastyUUID
1 parent d5e5170 commit 1b79cb3

File tree

6 files changed

+84
-49
lines changed

6 files changed

+84
-49
lines changed

Diff for: compiler/src/dotty/tools/dotc/classpath/FileUtils.scala

+6-2
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,14 @@ object FileUtils {
1717
extension (file: AbstractFile) {
1818
def isPackage: Boolean = file.isDirectory && mayBeValidPackage(file.name)
1919

20-
def isClass: Boolean = !file.isDirectory && file.hasExtension("class") && !file.name.endsWith("$class.class")
20+
def isClass: Boolean = !file.isDirectory && hasClassExtension && !file.name.endsWith("$class.class")
2121
// FIXME: drop last condition when we stop being compatible with Scala 2.11
2222

23-
def isTasty: Boolean = !file.isDirectory && file.hasExtension("tasty")
23+
def hasClassExtension: Boolean = file.hasExtension("class")
24+
25+
def hasTastyExtension: Boolean = file.hasExtension("tasty")
26+
27+
def isTasty: Boolean = !file.isDirectory && hasTastyExtension
2428

2529
def isScalaBinary: Boolean = file.isClass || file.isTasty
2630

Diff for: compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala

+3-3
Original file line numberDiff line numberDiff line change
@@ -432,9 +432,9 @@ class TastyLoader(val tastyFile: AbstractFile) extends SymbolLoader {
432432

433433

434434
private def checkTastyUUID(tastyFile: AbstractFile, tastyBytes: Array[Byte])(using Context): Unit =
435-
var classfile = tastyFile.resolveSibling(tastyFile.name.stripSuffix(".tasty") + ".class")
436-
if classfile == null then
437-
classfile = tastyFile.resolveSibling(tastyFile.name.stripSuffix(".tasty") + "$.class")
435+
val classfile =
436+
val className = tastyFile.name.stripSuffix(".tasty")
437+
tastyFile.resolveSibling(className + ".class")
438438
if classfile != null then
439439
val tastyUUID = new TastyHeaderUnpickler(tastyBytes).readHeader()
440440
new ClassfileTastyUUIDParser(classfile)(ctx).checkTastyUUID(tastyUUID)

Diff for: compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala

+61-38
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import java.nio.file.Path
88
import java.util.{Arrays, EnumSet}
99

1010
import dotty.tools.dotc.ast.tpd
11-
import dotty.tools.dotc.classpath.FileUtils.{isTasty, isClass}
11+
import dotty.tools.dotc.classpath.FileUtils.{isTasty, hasClassExtension, hasTastyExtension}
1212
import dotty.tools.dotc.core.Contexts._
1313
import dotty.tools.dotc.core.Decorators._
1414
import dotty.tools.dotc.core.Flags._
@@ -21,7 +21,7 @@ import dotty.tools.dotc.core.Types._
2121
import dotty.tools.dotc.transform.SymUtils._
2222
import dotty.tools.dotc.util.{SrcPos, NoSourcePosition}
2323
import dotty.tools.io
24-
import dotty.tools.io.{AbstractFile, PlainFile, ZipArchive}
24+
import dotty.tools.io.{AbstractFile, PlainFile, ZipArchive, NoAbstractFile}
2525
import xsbti.UseScope
2626
import xsbti.api.DependencyContext
2727
import xsbti.api.DependencyContext._
@@ -421,58 +421,81 @@ class DependencyRecorder {
421421
usedNames.names.foreach:
422422
case (usedName, scopes) =>
423423
cb.usedName(className, usedName.toString, scopes)
424-
classDependencies.foreach(recordClassDependency(cb, _))
424+
val siblingClassfiles = new mutable.HashMap[PlainFile, Path]
425+
classDependencies.foreach(recordClassDependency(cb, _, siblingClassfiles))
425426
clear()
426427

427428
/** Clear all state. */
428-
def clear(): Unit =
429-
_usedNames.clear()
430-
_classDependencies.clear()
431-
lastOwner = NoSymbol
432-
lastDepSource = NoSymbol
433-
_responsibleForImports = NoSymbol
429+
def clear(): Unit =
430+
_usedNames.clear()
431+
_classDependencies.clear()
432+
lastOwner = NoSymbol
433+
lastDepSource = NoSymbol
434+
_responsibleForImports = NoSymbol
434435

435436
/** Handles dependency on given symbol by trying to figure out if represents a term
436437
* that is coming from either source code (not necessarily compiled in this compilation
437438
* run) or from class file and calls respective callback method.
438439
*/
439-
private def recordClassDependency(cb: interfaces.IncrementalCallback, dep: ClassDependency)(using Context): Unit = {
440+
private def recordClassDependency(cb: interfaces.IncrementalCallback, dep: ClassDependency,
441+
siblingClassfiles: mutable.Map[PlainFile, Path])(using Context): Unit = {
440442
val fromClassName = classNameAsString(dep.fromClass)
441443
val sourceFile = ctx.compilationUnit.source
442444

443-
def binaryDependency(file: Path, binaryClassName: String) =
444-
cb.binaryDependency(file, binaryClassName, fromClassName, sourceFile, dep.context)
445-
446-
def processExternalDependency(depFile: AbstractFile, binaryClassName: String) = {
447-
depFile match {
448-
case ze: ZipArchive#Entry => // The dependency comes from a JAR
449-
ze.underlyingSource match
450-
case Some(zip) if zip.jpath != null =>
451-
binaryDependency(zip.jpath, binaryClassName)
452-
case _ =>
453-
case pf: PlainFile => // The dependency comes from a class file, Zinc handles JRT filesystem
454-
binaryDependency(pf.jpath, binaryClassName)
455-
case _ =>
456-
internalError(s"Ignoring dependency $depFile of unknown class ${depFile.getClass}}", dep.fromClass.srcPos)
457-
}
458-
}
459-
460-
val depFile = dep.toClass.associatedFile
461-
if (depFile != null) {
445+
/**For a `.tasty` file, constructs a sibling class to the `jpath`.
446+
* Does not validate if it exists as a real file.
447+
*
448+
* Because classpath scanning looks for tasty files first, `dep.fromClass` will be
449+
* associated to a `.tasty` file. However Zinc records all dependencies either based on `.jar` or `.class` files,
450+
* where classes are in directories on the filesystem.
451+
*
452+
* So if the dependency comes from an upstream `.tasty` file and it was not packaged in a jar, then
453+
* we need to call this to resolve the classfile that will eventually exist at runtime.
454+
*
455+
* The way this works is that by the end of compilation analysis,
456+
* we should have called `cb.generatedNonLocalClass` with the same class file name.
457+
*
458+
* FIXME: we still need a way to resolve the correct classfile when we split tasty and classes between
459+
* different outputs (e.g. stdlib-bootstrapped).
460+
*/
461+
def cachedSiblingClass(pf: PlainFile): Path =
462+
siblingClassfiles.getOrElseUpdate(pf, {
463+
val jpath = pf.jpath
464+
jpath.getParent.resolve(jpath.getFileName.toString.stripSuffix(".tasty") + ".class")
465+
})
466+
467+
def binaryDependency(path: Path, binaryClassName: String) =
468+
cb.binaryDependency(path, binaryClassName, fromClassName, sourceFile, dep.context)
469+
470+
val depClass = dep.toClass
471+
val depFile = depClass.associatedFile
472+
if depFile != null then {
462473
// Cannot ignore inheritance relationship coming from the same source (see sbt/zinc#417)
463474
def allowLocal = dep.context == DependencyByInheritance || dep.context == LocalDependencyByInheritance
464-
val depClassFile =
465-
if depFile.isClass then depFile
466-
else depFile.resolveSibling(dep.toClass.binaryClassName + ".class")
467-
if (depClassFile != null) {
468-
// Dependency is external -- source is undefined
469-
processExternalDependency(depClassFile, dep.toClass.binaryClassName)
470-
} else if (allowLocal || depFile != sourceFile.file) {
475+
val isTasty = depFile.hasTastyExtension
476+
477+
def processExternalDependency() = {
478+
val binaryClassName = depClass.binaryClassName
479+
depFile match {
480+
case ze: ZipArchive#Entry => // The dependency comes from a JAR
481+
ze.underlyingSource match
482+
case Some(zip) if zip.jpath != null =>
483+
binaryDependency(zip.jpath, binaryClassName)
484+
case _ =>
485+
case pf: PlainFile => // The dependency comes from a class file, Zinc handles JRT filesystem
486+
binaryDependency(if isTasty then cachedSiblingClass(pf) else pf.jpath, binaryClassName)
487+
case _ =>
488+
internalError(s"Ignoring dependency $depFile of unknown class ${depFile.getClass}}", dep.fromClass.srcPos)
489+
}
490+
}
491+
492+
if isTasty || depFile.hasClassExtension then
493+
processExternalDependency()
494+
else if allowLocal || depFile != sourceFile.file then
471495
// We cannot ignore dependencies coming from the same source file because
472496
// the dependency info needs to propagate. See source-dependencies/trait-trait-211.
473-
val toClassName = classNameAsString(dep.toClass)
497+
val toClassName = classNameAsString(depClass)
474498
cb.classDependency(toClassName, fromClassName, dep.context)
475-
}
476499
}
477500
}
478501

Diff for: compiler/src/dotty/tools/io/AbstractFile.scala

+4-2
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,10 @@ abstract class AbstractFile extends Iterable[AbstractFile] {
9797
/** Returns the path of this abstract file in a canonical form. */
9898
def canonicalPath: String = if (jpath == null) path else jpath.normalize.toString
9999

100-
/** Checks extension case insensitively. */
100+
/** Checks extension case insensitively. TODO: change to enum */
101101
def hasExtension(other: String): Boolean = extension == other.toLowerCase
102+
103+
/** Returns the extension of this abstract file. TODO: store as an enum to avoid costly comparisons */
102104
val extension: String = Path.extension(name)
103105

104106
/** The absolute file, if this is a relative file. */
@@ -253,7 +255,7 @@ abstract class AbstractFile extends Iterable[AbstractFile] {
253255
/** Returns the sibling abstract file in the parent of this abstract file or directory.
254256
* If there is no such file, returns `null`.
255257
*/
256-
def resolveSibling(name: String): AbstractFile | Null =
258+
final def resolveSibling(name: String): AbstractFile | Null =
257259
container.lookupName(name, directory = false)
258260

259261
private def fileOrSubdirectoryNamed(name: String, isDir: Boolean): AbstractFile =

Diff for: compiler/src/dotty/tools/io/PlainFile.scala

+9-2
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,15 @@ class PlainFile(val givenPath: Path) extends AbstractFile {
102102
*/
103103
def lookupName(name: String, directory: Boolean): AbstractFile = {
104104
val child = givenPath / name
105-
if ((child.isDirectory && directory) || (child.isFile && !directory)) new PlainFile(child)
106-
else null
105+
if directory then
106+
if child.isDirectory /* IO! */ then
107+
new PlainFile(child)
108+
else
109+
null
110+
else if child.isFile /* IO! */ then
111+
new PlainFile(child)
112+
else
113+
null
107114
}
108115

109116
/** Does this abstract file denote an existing file? */

Diff for: compiler/src/dotty/tools/io/ZipArchive.scala

+1-2
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,7 @@ abstract class ZipArchive(override val jpath: JPath, release: Option[String]) ex
7272
// have to keep this name for compat with sbt's compiler-interface
7373
def getArchive: ZipFile = null
7474
override def underlyingSource: Option[ZipArchive] = Some(self)
75-
override def resolveSibling(name: String): AbstractFile =
76-
parent.lookupName(name, directory = false)
75+
override def container: Entry = parent
7776
override def toString: String = self.path + "(" + path + ")"
7877
}
7978

0 commit comments

Comments
 (0)