Skip to content

Reduce IO calls in extractDependencies #18266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions compiler/src/dotty/tools/dotc/classpath/FileUtils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,14 @@ object FileUtils {
extension (file: AbstractFile) {
def isPackage: Boolean = file.isDirectory && mayBeValidPackage(file.name)

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

def isTasty: Boolean = !file.isDirectory && file.hasExtension("tasty")
def hasClassExtension: Boolean = file.hasExtension("class")

def hasTastyExtension: Boolean = file.hasExtension("tasty")

def isTasty: Boolean = !file.isDirectory && hasTastyExtension

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

Expand Down
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala
Original file line number Diff line number Diff line change
Expand Up @@ -432,9 +432,9 @@ class TastyLoader(val tastyFile: AbstractFile) extends SymbolLoader {


private def checkTastyUUID(tastyFile: AbstractFile, tastyBytes: Array[Byte])(using Context): Unit =
var classfile = tastyFile.resolveSibling(tastyFile.name.stripSuffix(".tasty") + ".class")
if classfile == null then
classfile = tastyFile.resolveSibling(tastyFile.name.stripSuffix(".tasty") + "$.class")
val classfile =
val className = tastyFile.name.stripSuffix(".tasty")
tastyFile.resolveSibling(className + ".class")
if classfile != null then
val tastyUUID = new TastyHeaderUnpickler(tastyBytes).readHeader()
new ClassfileTastyUUIDParser(classfile)(ctx).checkTastyUUID(tastyUUID)
Expand Down
99 changes: 61 additions & 38 deletions compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import java.nio.file.Path
import java.util.{Arrays, EnumSet}

import dotty.tools.dotc.ast.tpd
import dotty.tools.dotc.classpath.FileUtils.{isTasty, isClass}
import dotty.tools.dotc.classpath.FileUtils.{isTasty, hasClassExtension, hasTastyExtension}
import dotty.tools.dotc.core.Contexts._
import dotty.tools.dotc.core.Decorators._
import dotty.tools.dotc.core.Flags._
Expand All @@ -21,7 +21,7 @@ import dotty.tools.dotc.core.Types._
import dotty.tools.dotc.transform.SymUtils._
import dotty.tools.dotc.util.{SrcPos, NoSourcePosition}
import dotty.tools.io
import dotty.tools.io.{AbstractFile, PlainFile, ZipArchive}
import dotty.tools.io.{AbstractFile, PlainFile, ZipArchive, NoAbstractFile}
import xsbti.UseScope
import xsbti.api.DependencyContext
import xsbti.api.DependencyContext._
Expand Down Expand Up @@ -421,58 +421,81 @@ class DependencyRecorder {
usedNames.names.foreach:
case (usedName, scopes) =>
cb.usedName(className, usedName.toString, scopes)
classDependencies.foreach(recordClassDependency(cb, _))
val siblingClassfiles = new mutable.HashMap[PlainFile, Path]
classDependencies.foreach(recordClassDependency(cb, _, siblingClassfiles))
clear()

/** Clear all state. */
def clear(): Unit =
_usedNames.clear()
_classDependencies.clear()
lastOwner = NoSymbol
lastDepSource = NoSymbol
_responsibleForImports = NoSymbol
def clear(): Unit =
_usedNames.clear()
_classDependencies.clear()
lastOwner = NoSymbol
lastDepSource = NoSymbol
_responsibleForImports = NoSymbol

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

def binaryDependency(file: Path, binaryClassName: String) =
cb.binaryDependency(file, binaryClassName, fromClassName, sourceFile, dep.context)

def processExternalDependency(depFile: AbstractFile, binaryClassName: String) = {
depFile match {
case ze: ZipArchive#Entry => // The dependency comes from a JAR
ze.underlyingSource match
case Some(zip) if zip.jpath != null =>
binaryDependency(zip.jpath, binaryClassName)
case _ =>
case pf: PlainFile => // The dependency comes from a class file, Zinc handles JRT filesystem
binaryDependency(pf.jpath, binaryClassName)
case _ =>
internalError(s"Ignoring dependency $depFile of unknown class ${depFile.getClass}}", dep.fromClass.srcPos)
}
}

val depFile = dep.toClass.associatedFile
if (depFile != null) {
/**For a `.tasty` file, constructs a sibling class to the `jpath`.
* Does not validate if it exists as a real file.
*
* Because classpath scanning looks for tasty files first, `dep.fromClass` will be
* associated to a `.tasty` file. However Zinc records all dependencies either based on `.jar` or `.class` files,
* where classes are in directories on the filesystem.
*
* So if the dependency comes from an upstream `.tasty` file and it was not packaged in a jar, then
* we need to call this to resolve the classfile that will eventually exist at runtime.
*
* The way this works is that by the end of compilation analysis,
* we should have called `cb.generatedNonLocalClass` with the same class file name.
*
* FIXME: we still need a way to resolve the correct classfile when we split tasty and classes between
* different outputs (e.g. stdlib-bootstrapped).
*/
def cachedSiblingClass(pf: PlainFile): Path =
siblingClassfiles.getOrElseUpdate(pf, {
val jpath = pf.jpath
jpath.getParent.resolve(jpath.getFileName.toString.stripSuffix(".tasty") + ".class")
})

def binaryDependency(path: Path, binaryClassName: String) =
cb.binaryDependency(path, binaryClassName, fromClassName, sourceFile, dep.context)

val depClass = dep.toClass
val depFile = depClass.associatedFile
if depFile != null then {
// Cannot ignore inheritance relationship coming from the same source (see sbt/zinc#417)
def allowLocal = dep.context == DependencyByInheritance || dep.context == LocalDependencyByInheritance
val depClassFile =
if depFile.isClass then depFile
else depFile.resolveSibling(dep.toClass.binaryClassName + ".class")
if (depClassFile != null) {
// Dependency is external -- source is undefined
processExternalDependency(depClassFile, dep.toClass.binaryClassName)
} else if (allowLocal || depFile != sourceFile.file) {
val isTasty = depFile.hasTastyExtension

def processExternalDependency() = {
val binaryClassName = depClass.binaryClassName
depFile match {
case ze: ZipArchive#Entry => // The dependency comes from a JAR
ze.underlyingSource match
case Some(zip) if zip.jpath != null =>
binaryDependency(zip.jpath, binaryClassName)
case _ =>
case pf: PlainFile => // The dependency comes from a class file, Zinc handles JRT filesystem
binaryDependency(if isTasty then cachedSiblingClass(pf) else pf.jpath, binaryClassName)
case _ =>
internalError(s"Ignoring dependency $depFile of unknown class ${depFile.getClass}}", dep.fromClass.srcPos)
}
}

if isTasty || depFile.hasClassExtension then
processExternalDependency()
else if allowLocal || depFile != sourceFile.file then
// We cannot ignore dependencies coming from the same source file because
// the dependency info needs to propagate. See source-dependencies/trait-trait-211.
val toClassName = classNameAsString(dep.toClass)
val toClassName = classNameAsString(depClass)
cb.classDependency(toClassName, fromClassName, dep.context)
}
}
}

Expand Down
6 changes: 4 additions & 2 deletions compiler/src/dotty/tools/io/AbstractFile.scala
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,10 @@ abstract class AbstractFile extends Iterable[AbstractFile] {
/** Returns the path of this abstract file in a canonical form. */
def canonicalPath: String = if (jpath == null) path else jpath.normalize.toString

/** Checks extension case insensitively. */
/** Checks extension case insensitively. TODO: change to enum */
def hasExtension(other: String): Boolean = extension == other.toLowerCase

/** Returns the extension of this abstract file. TODO: store as an enum to avoid costly comparisons */
val extension: String = Path.extension(name)

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

private def fileOrSubdirectoryNamed(name: String, isDir: Boolean): AbstractFile =
Expand Down
11 changes: 9 additions & 2 deletions compiler/src/dotty/tools/io/PlainFile.scala
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,15 @@ class PlainFile(val givenPath: Path) extends AbstractFile {
*/
def lookupName(name: String, directory: Boolean): AbstractFile = {
val child = givenPath / name
if ((child.isDirectory && directory) || (child.isFile && !directory)) new PlainFile(child)
else null
if directory then
if child.isDirectory /* IO! */ then
new PlainFile(child)
else
null
else if child.isFile /* IO! */ then
new PlainFile(child)
else
null
}

/** Does this abstract file denote an existing file? */
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dotty/tools/io/ZipArchive.scala
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ abstract class ZipArchive(override val jpath: JPath, release: Option[String]) ex
// have to keep this name for compat with sbt's compiler-interface
def getArchive: ZipFile = null
override def underlyingSource: Option[ZipArchive] = Some(self)
override def resolveSibling(name: String): AbstractFile =
parent.lookupName(name, directory = false)
override def container: Entry = parent
override def toString: String = self.path + "(" + path + ")"
}

Expand Down