Skip to content

Commit

Permalink
Move notification of non-local classes to API
Browse files Browse the repository at this point in the history
Registers only non-local generated classes in the callback by extracting
information about its names and using the names to generate class file paths.

Mimics the previous logic that was present in `Analyzer`, despite the fact
that now we construct the names that the compiler will give to every non-local
class independently of genbcode.

Why do we do this? The motivation is that we want to run the incremental algorithm
independently of the compiler pipeline. This independence enables us to:

1. Offload the incremental compiler logic out of the primary pipeline and
   run the incremental phases concurrently.
2. Know before the compilation is completed whether another compilation will or
   will not be required. This is important to make incremental compilation work
   with pipelining and enables further optimizations; for example, we can start
   subsequent incremental compilations before (!) the initial compilation is done.
   This can buy us ~30-40% faster incremental compiler iterations.

This method only takes care of non-local classes because local clsases have no
relevance in the correctness of the algorithm and can be registered after genbcode.
Local classes are only used to contruct the relations of products and to produce
the list of generated files + stamps, but names referring to local classes **never**
show up in the name hashes of classes' APIs, hence never considered for name hashing.

As local class files are owned by other classes that change whenever they change,
we could most likely live without adding their class files to the products relation
and registering their stamps. However, to be on the safe side, we will continue to
register the local products in `Analyzer`.
  • Loading branch information
jvican committed Aug 22, 2018
1 parent 49031d4 commit 856d416
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 21 deletions.
139 changes: 136 additions & 3 deletions internal/compiler-bridge/src/main/scala/xsbt/API.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,31 @@ object API {
val name = "xsbt-api"
}

final class API(val global: CallbackGlobal) extends Compat with GlobalHelpers {
final class API(val global: CallbackGlobal) extends Compat with GlobalHelpers with ClassName {
import global._

import scala.collection.mutable
private val nonLocalClassSymbolsInCurrentUnits = new mutable.HashSet[Symbol]()

def newPhase(prev: Phase) = new ApiPhase(prev)
class ApiPhase(prev: Phase) extends GlobalPhase(prev) {
override def description = "Extracts the public API from source files."
def name = API.name
override def run(): Unit = {
val start = System.currentTimeMillis
super.run()

// After processing all units, register generated classes
registerGeneratedClasses(nonLocalClassSymbolsInCurrentUnits.iterator)
nonLocalClassSymbolsInCurrentUnits.clear()

callback.apiPhaseCompleted()
val stop = System.currentTimeMillis
debuglog("API phase took : " + ((stop - start) / 1000.0) + " s")
}

def apply(unit: global.CompilationUnit): Unit = processUnit(unit)

private def processUnit(unit: CompilationUnit) = if (!unit.isJava) processScalaUnit(unit)

private def processScalaUnit(unit: CompilationUnit): Unit = {
val sourceFile = unit.source.file.file
debuglog("Traversing " + sourceFile)
Expand All @@ -59,6 +65,133 @@ final class API(val global: CallbackGlobal) extends Compat with GlobalHelpers {
while (mainClassesIt.hasNext) {
callback.mainClass(sourceFile, mainClassesIt.next())
}

extractApi.allExtractedNonLocalSymbols.foreach { cs =>
// Only add the class symbols defined in this compilation unit
if (cs.sourceFile != null) nonLocalClassSymbolsInCurrentUnits.+=(cs)
}
}
}

private case class FlattenedNames(binaryName: String, className: String)

/**
* Replicate the behaviour of `fullName` with a few changes to the code to produce
* correct file-system compatible full names for non-local classes. It mimics the
* paths of the class files produced by genbcode.
*
* Changes compared to the normal version in the compiler:
*
* 1. It will use the encoded name instead of the normal name.
* 2. It will not skip the name of the package object class (required for the class file path).
*
* Note that using `javaBinaryName` is not useful for these symbols because we
* need the encoded names. Zinc keeps track of encoded names in both the binary
* names and the Zinc names.
*
* @param symbol The symbol for which we extract the full name.
* @param separator The separator that we will apply between every name.
* @param suffix The suffix to add at the end (in case it's a module).
* @param includePackageObjectClassNames Include package object class names or not.
* @return The full name.
*/
def fullName(
symbol: Symbol,
separator: Char,
suffix: CharSequence,
includePackageObjectClassNames: Boolean
): String = {
var b: java.lang.StringBuffer = null
def loop(size: Int, sym: Symbol): Unit = {
val symName = sym.name
// Use of encoded to produce correct paths for names that have symbols
val encodedName = symName.encoded
val nSize = encodedName.length - (if (symName.endsWith(nme.LOCAL_SUFFIX_STRING)) 1 else 0)
if (sym.isRoot || sym.isRootPackage || sym == NoSymbol || sym.owner.isEffectiveRoot) {
val capacity = size + nSize
b = new java.lang.StringBuffer(capacity)
b.append(chrs, symName.start, nSize)
} else {
val next = if (sym.owner.isPackageObjectClass) sym.owner else sym.effectiveOwner.enclClass
loop(size + nSize + 1, next)
// Addition to normal `fullName` to produce correct names for nested non-local classes
if (sym.isNestedClass) b.append(nme.MODULE_SUFFIX_STRING) else b.append(separator)
b.append(chrs, symName.start, nSize)
}
}
loop(suffix.length(), symbol)
b.append(suffix)
b.toString
}

/**
* Registers only non-local generated classes in the callback by extracting
* information about its names and using the names to generate class file paths.
*
* Mimics the previous logic that was present in `Analyzer`, despite the fact
* that now we construct the names that the compiler will give to every non-local
* class independently of genbcode.
*
* Why do we do this? The motivation is that we want to run the incremental algorithm
* independently of the compiler pipeline. This independence enables us to:
*
* 1. Offload the incremental compiler logic out of the primary pipeline and
* run the incremental phases concurrently.
* 2. Know before the compilation is completed whether another compilation will or
* will not be required. This is important to make incremental compilation work
* with pipelining and enables further optimizations; for example, we can start
* subsequent incremental compilations before (!) the initial compilation is done.
* This can buy us ~30-40% faster incremental compiler iterations.
*
* This method only takes care of non-local classes because local clsases have no
* relevance in the correctness of the algorithm and can be registered after genbcode.
* Local classes are only used to contruct the relations of products and to produce
* the list of generated files + stamps, but names referring to local classes **never**
* show up in the name hashes of classes' APIs, hence never considered for name hashing.
*
* As local class files are owned by other classes that change whenever they change,
* we could most likely live without adding their class files to the products relation
* and registering their stamps. However, to be on the safe side, we will continue to
* register the local products in `Analyzer`.
*
* @param allClassSymbols The class symbols found in all the compilation units.
*/
def registerGeneratedClasses(classSymbols: Iterator[Symbol]): Unit = {
classSymbols.foreach { symbol =>
val sourceFile = symbol.sourceFile
val sourceJavaFile =
if (sourceFile == null) symbol.enclosingTopLevelClass.sourceFile.file else sourceFile.file

def registerProductNames(names: FlattenedNames): Unit = {
// Guard against a local class in case it surreptitiously leaks here
if (!symbol.isLocalClass) {
val classFileName = s"${names.binaryName}.class"
val outputDir = global.settings.outputDirs.outputDirFor(sourceFile).file
val classFile = new java.io.File(outputDir, classFileName)
val zincClassName = names.className
val srcClassName = classNameAsString(symbol)
callback.generatedNonLocalClass(sourceJavaFile, classFile, zincClassName, srcClassName)
} else ()
}

val names = FlattenedNames(
//flattenedNames(symbol)
fullName(symbol, java.io.File.separatorChar, symbol.moduleSuffix, true),
fullName(symbol, '.', symbol.moduleSuffix, false)
)

registerProductNames(names)

// Register the names of top-level module symbols that emit two class files
val isTopLevelUniqueModule =
symbol.owner.isPackageClass && symbol.isModuleClass && symbol.companionClass == NoSymbol
if (isTopLevelUniqueModule || symbol.isPackageObject) {
val names = FlattenedNames(
fullName(symbol, java.io.File.separatorChar, "", true),
fullName(symbol, '.', "", false)
)
registerProductNames(names)
}
}
}

Expand Down
26 changes: 9 additions & 17 deletions internal/compiler-bridge/src/main/scala/xsbt/Analyzer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import scala.tools.nsc.Phase
object Analyzer {
def name = "xsbt-analyzer"
}

final class Analyzer(val global: CallbackGlobal) extends LocateClassFile {
import global._

Expand All @@ -20,34 +21,25 @@ final class Analyzer(val global: CallbackGlobal) extends LocateClassFile {
override def description =
"Finds concrete instances of provided superclasses, and application entry points."
def name = Analyzer.name

def apply(unit: CompilationUnit): Unit = {
if (!unit.isJava) {
val sourceFile = unit.source.file.file
// build list of generated classes
for (iclass <- unit.icode) {
val sym = iclass.symbol
val outputDir = settings.outputDirs.outputDirFor(sym.sourceFile).file
def addGenerated(separatorRequired: Boolean): Unit = {
for (classFile <- outputDirs map (fileForClass(_, sym, separatorRequired)) find (_.exists)) {
val classFile = fileForClass(outputDir, sym, separatorRequired)
if (classFile.exists()) {
assert(sym.isClass, s"${sym.fullName} is not a class")
// we would like to use Symbol.isLocalClass but that relies on Symbol.owner which
// is lost at this point due to lambdalift
// the LocalNonLocalClass.isLocal can return None, which means, we're asking about
// the class it has not seen before. How's that possible given we're performing a lookup
// for every declared class in Dependency phase? We can have new classes introduced after
// Dependency phase has ran. For example, the implementation classes for traits.
val isLocalClass = localToNonLocalClass.isLocal(sym).getOrElse(true)
if (!isLocalClass) {
val srcClassName = classNameAsString(sym)
val binaryClassName = flatclassName(sym, '.', separatorRequired)
callback.generatedNonLocalClass(sourceFile,
classFile,
binaryClassName,
srcClassName)
} else {
// Use own map of local classes computed before lambdalift to ascertain class locality
if (localToNonLocalClass.isLocal(sym).getOrElse(true)) {
// Inform callback about local classes, non-local classes have been reported in API
callback.generatedLocalClass(sourceFile, classFile)
}
}
}

if (sym.isModuleClass && !sym.isImplClass) {
if (isTopLevelModule(sym) && sym.companionClass == NoSymbol)
addGenerated(false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ class ExtractAPI[GlobalType <: Global](

private[this] val emptyStringArray = Array.empty[String]

private[this] val allNonLocalClassSymbols = perRunCaches.newSet[Symbol]()
private[this] val allNonLocalClassesInSrc = perRunCaches.newSet[xsbti.api.ClassLike]()
private[this] val _mainClasses = perRunCaches.newSet[String]()

Expand Down Expand Up @@ -430,7 +431,8 @@ class ExtractAPI[GlobalType <: Global](
def mkVar = Some(fieldDef(in, sym, keepConst = false, xsbti.api.Var.of(_, _, _, _, _)))
def mkVal = Some(fieldDef(in, sym, keepConst = true, xsbti.api.Val.of(_, _, _, _, _)))
if (isClass(sym))
if (ignoreClass(sym)) None else Some(classLike(in, sym))
if (ignoreClass(sym)) {allNonLocalClassSymbols.+=(sym); None}
else Some(classLike(in, sym))
else if (sym.isNonClassType)
Some(typeDef(in, sym))
else if (sym.isVariable)
Expand Down Expand Up @@ -646,6 +648,8 @@ class ExtractAPI[GlobalType <: Global](
allNonLocalClassesInSrc.toSet
}

def allExtractedNonLocalSymbols: Set[Symbol] = allNonLocalClassSymbols.toSet

def mainClasses: Set[String] = {
forceStructures()
_mainClasses.toSet
Expand Down Expand Up @@ -691,6 +695,7 @@ class ExtractAPI[GlobalType <: Global](
val classWithMembers = constructClass(structure)

allNonLocalClassesInSrc += classWithMembers
allNonLocalClassSymbols += sym

if (sym.isStatic && defType == DefinitionType.Module && definitions.hasJavaMainMethod(sym)) {
_mainClasses += name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class LocalToNonLocalClass[G <: CallbackGlobal](val global: G) {
assert(s.isClass, s"The ${s.fullName} is not a class.")
cache.getOrElseUpdate(s, lookupNonLocal(s))
}

private def lookupNonLocal(s: Symbol): Symbol = {
if (s.owner.isPackageClass) s
else if (s.owner.isClass) {
Expand Down

0 comments on commit 856d416

Please sign in to comment.