Synchronize ClassLoaderCache and the Scala provider cache.

Construction of Scala providers was already properly synchronized jvm and machine-wide.
The cache on top of construction was not and neither was the newer ClassLoaderCache.
This could cause the same Scala version to be loaded in multiple class loaders, taking
up more permgen space and possibly decreasing performance due to less effective jit.

The issue is very rare in practice for 0.13 because of the low probability of contention
on ClassLoaderCache.  This is because the work for a cache miss is mainly the construction
of a URLClassLoader.  In 0.12, however, the work potentially involved network access and
class loading (not just class loader construction), thus greatly increasing the probability
of contention and thus duplicate work (i.e. class loader construction).

When there is contention, multiple class loaders are constructed and then preserved by the
scalaInstance task in each project throughout the first task execution.  Only when multiple
scalaInstance tasks execute simultaneously and only during the first execution does this occur.
(Technically, it could still happen later, but it doesn't in practice.)

This means that the number of duplicate class loaders should quickly saturate instead of growing
linearly with the number of projects.  It also means that the impact depends on the exact
tree structure of projects.  A linear chain of dependencies will be unaffected, but a build with
independent leaves may be limited by the number of cores.  The number of cores affects
the number of threads typically used by the task engine, which limits the number of concurrently
executing scalaInstance tasks.

In summary, this might affect the first, cold compilation of a multi-module project with
independent leaves on a multi-core machine with Scala version different from the version used
for sbt.  It might increase the maximum permgen requirements as well as slow the jit compilation
by up to one task execution.  Subsequent compilations should be unaffected and the permgen
utilization return to be as expected.
This commit is contained in:
Mark Harrah 2013-06-19 19:20:37 -04:00
parent 6091e60611
commit 5dc671c7f8
6 changed files with 65 additions and 3 deletions

View File

@ -9,7 +9,7 @@ import java.util.HashMap
final class Cache[K,X,V](create: (K,X) => V)
{
private[this] val delegate = new HashMap[K,Reference[V]]
def apply(k: K, x: X): V = getFromReference(k, x, delegate.get(k))
def apply(k: K, x: X): V = synchronized { getFromReference(k, x, delegate.get(k)) }
private[this] def getFromReference(k: K, x: X, existingRef: Reference[V]) = if(existingRef eq null) newEntry(k, x) else get(k, x, existingRef.get)
private[this] def get(k: K, x: X, existing: V) = if(existing == null) newEntry(k, x) else existing
private[this] def newEntry(k: K, x: X): V =

View File

@ -60,7 +60,7 @@ object Sbt extends Build
// Path, IO (formerly FileUtilities), NameFilter and other I/O utility classes
lazy val ioSub = testedBaseProject(utilPath / "io", "IO") dependsOn(controlSub) settings(ioSettings : _ *)
// Utilities related to reflection, managing Scala versions, and custom class loaders
lazy val classpathSub = baseProject(utilPath / "classpath", "Classpath") dependsOn(launchInterfaceSub, interfaceSub, ioSub) settings(scalaCompiler)
lazy val classpathSub = testedBaseProject(utilPath / "classpath", "Classpath") dependsOn(launchInterfaceSub, interfaceSub, ioSub) settings(scalaCompiler)
// Command line-related utilities.
lazy val completeSub = testedBaseProject(utilPath / "complete", "Completion") dependsOn(collectionSub, controlSub, ioSub) settings(jline : _*)
// logging

View File

@ -0,0 +1,20 @@
import sbt._
import Keys._
object Build extends Build {
lazy val checkLoader = TaskKey[Unit]("check-loaders")
def checkTask = subs.map(sub => scalaInstance in LocalProject(sub.id)).join.map { sis =>
assert(sis.sliding(2).forall{ case Seq(x,y) => x.loader == y.loader }, "Not all ScalaInstances had the same class loader.")
}
override def projects = root +: subs
lazy val root = Project("root", file(".")).settings( checkLoader <<= checkTask, concurrentRestrictions := Nil )
lazy val subs = ( for(i <- 1 to 20) yield newProject(i) ).toSeq
def newProject(i: Int): Project = Project("x" + i.toString, file(i.toString)).settings(
scalaVersion := "2.9.2" // this should be a Scala version different from the one sbt uses
)
}

View File

@ -0,0 +1,7 @@
# verify that the class loader for Scala used for each project is the same
# this test failed in 0.12.4-RC2 and earlier. Even though 0.13.0-Beta2
# technically had a problem as well, it is harder to make 0.13 exhibit
# problems in practice due to the different approach that substantially
# reduces the probability of simultaneous access. The better test for 0.13+
# is the unit test on ClassLoaderCache
> check-loaders

View File

@ -8,7 +8,11 @@ import java.util.HashMap
private[sbt] final class ClassLoaderCache(val commonParent: ClassLoader)
{
private[this] val delegate = new HashMap[List[File],Reference[CachedClassLoader]]
def apply(files: List[File]): ClassLoader =
/** Returns a ClassLoader with `commonParent` as a parent and that will load classes from classpath `files`.
* The returned ClassLoader may be cached from a previous call if the last modified time of all `files` is unchanged.
* This method is thread-safe.*/
def apply(files: List[File]): ClassLoader = synchronized
{
val tstamps = files.map(_.lastModified)
getFromReference(files, tstamps, delegate.get(files))

View File

@ -0,0 +1,31 @@
package sbt
package classpath
import org.scalacheck._
import Prop._
import java.io.File
object ConcurrentCache extends Properties("ClassLoaderCache concurrent access")
{
implicit lazy val concurrentArb: Arbitrary[Int] = Arbitrary( Gen.choose(1, 1000) )
implicit lazy val filenameArb: Arbitrary[String] = Arbitrary( Gen.alphaStr )
property("Same class loader for same classpaths concurrently processed") = forAll { (names: List[String], concurrent: Int) =>
withcp(names.distinct) { files =>
val cache = new ClassLoaderCache(null)
val loaders = (1 to concurrent).par.map(_ => cache(files)).toList
sameClassLoader(loaders)
}
}
private[this] def withcp[T](names: List[String])(f: List[File] => T): T = IO.withTemporaryDirectory { tmp =>
val files = names.map{ name =>
val file = new File(tmp, name)
IO.touch(file)
file
}
f(files)
}
private[this] def sameClassLoader(loaders: Seq[ClassLoader]): Boolean = loaders.size < 2 ||
loaders.sliding(2).forall { case Seq(x,y) => x == y }
}