From cda4713f89d1968aaacac5687cc3235fe1462b01 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Thu, 30 May 2019 16:24:56 -0700 Subject: [PATCH 1/3] Make AllLibraryJars a case object This improves the toString and also allows it to be used in a pattern match. --- main/src/main/scala/sbt/ClassLoaderLayeringStrategy.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main/src/main/scala/sbt/ClassLoaderLayeringStrategy.scala b/main/src/main/scala/sbt/ClassLoaderLayeringStrategy.scala index 483900728..cfb360fc4 100644 --- a/main/src/main/scala/sbt/ClassLoaderLayeringStrategy.scala +++ b/main/src/main/scala/sbt/ClassLoaderLayeringStrategy.scala @@ -96,5 +96,5 @@ object ClassLoaderLayeringStrategy { /** * Add a layer on top of the ScalaLibrary layer for all of the jar dependencies. */ - object AllLibraryJars extends AllLibraryJars + case object AllLibraryJars extends AllLibraryJars } From cf73bbbafc06a83d3f84124052deb962f71d4e74 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Thu, 30 May 2019 16:39:00 -0700 Subject: [PATCH 2/3] Fix ScalaLibrary again I was benchmarking sbt with turbo mode on and found that tests weren't running. This was because we were inadvertently excluding all of the dependency jars from the dynamic classpath. I have no idea why the scripted tests didn't catch this. The scalatest scripted test didn't catch this because 'test' just automaticaly succeeds if no test frameworks are found. To guard against regression, I had to ensure that 'test' failed for every strategy if a bad test file was present. --- main/src/main/scala/sbt/internal/ClassLoaders.scala | 6 ++++-- .../sbt-test/classloader-cache/package-private/test | 2 ++ sbt/src/sbt-test/classloader-cache/scalatest/test | 12 ++++++++++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/main/src/main/scala/sbt/internal/ClassLoaders.scala b/main/src/main/scala/sbt/internal/ClassLoaders.scala index 37ace0f72..3b4c607fc 100644 --- a/main/src/main/scala/sbt/internal/ClassLoaders.scala +++ b/main/src/main/scala/sbt/internal/ClassLoaders.scala @@ -132,7 +132,6 @@ private[sbt] object ClassLoaders { case _: AllLibraryJars => true case _ => false } - val allDependenciesSet = allDependencies.toSet val scalaLibraryLayer = layer(si.libraryJars, interfaceLoader, cache, resources, tmp) val cpFiles = fullCP.map(_._1) @@ -148,7 +147,10 @@ private[sbt] object ClassLoaders { else resourceLayer // layer 4 - val dynamicClasspath = cpFiles.filterNot(allDependenciesSet ++ si.libraryJars) + val filteredSet = + if (layerDependencies) allDependencies.toSet ++ si.libraryJars + else Set(si.libraryJars: _*) + val dynamicClasspath = cpFiles.filterNot(filteredSet) new LayeredClassLoader(dynamicClasspath, dependencyLayer, resources, tmp) } ClasspathUtilities.filterByClasspath(cpFiles, raw) diff --git a/sbt/src/sbt-test/classloader-cache/package-private/test b/sbt/src/sbt-test/classloader-cache/package-private/test index 64ef9d785..dd298596e 100644 --- a/sbt/src/sbt-test/classloader-cache/package-private/test +++ b/sbt/src/sbt-test/classloader-cache/package-private/test @@ -1,3 +1,5 @@ +> set Compile / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.AllLibraryJars + -> run > set Compile / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.Flat diff --git a/sbt/src/sbt-test/classloader-cache/scalatest/test b/sbt/src/sbt-test/classloader-cache/scalatest/test index 9b34bf4a6..055516751 100644 --- a/sbt/src/sbt-test/classloader-cache/scalatest/test +++ b/sbt/src/sbt-test/classloader-cache/scalatest/test @@ -1,3 +1,5 @@ +> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.AllLibraryJars + > test > set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.Flat @@ -8,8 +10,14 @@ > test -> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.AllLibraryJars - $ copy-file changes/bad.scala src/test/scala/sbt/ScalatestTest.scala -> test + +> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.Flat + +-> test + +> set Test / classLoaderLayeringStrategy := ClassLoaderLayeringStrategy.AllLibraryJars + +-> test From 5faf78af96887420575c372daa795b8ba27e9820 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Thu, 30 May 2019 16:42:02 -0700 Subject: [PATCH 3/3] Add scala reflect layer Not caching scala reflect is extremely painful if the build uses scalatest. It adds O(1second) to my watch performance benchmarks. It actually made sbt 1.3.0 much slower than 0.13.17 --- .../main/scala/sbt/internal/ClassLoaders.scala | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/main/src/main/scala/sbt/internal/ClassLoaders.scala b/main/src/main/scala/sbt/internal/ClassLoaders.scala index 3b4c607fc..773ec4813 100644 --- a/main/src/main/scala/sbt/internal/ClassLoaders.scala +++ b/main/src/main/scala/sbt/internal/ClassLoaders.scala @@ -105,7 +105,7 @@ private[sbt] object ClassLoaders { } /* * Create a layered classloader. There are up to four layers: - * 1) the scala instance class loader + * 1) the scala instance class loader (may actually be two layers if scala-reflect is used) * 2) the resource layer * 3) the dependency jars * 4) the rest of the classpath @@ -135,11 +135,18 @@ private[sbt] object ClassLoaders { val scalaLibraryLayer = layer(si.libraryJars, interfaceLoader, cache, resources, tmp) val cpFiles = fullCP.map(_._1) + val scalaReflectJar = allDependencies.find(_.getName == "scala-reflect.jar") + val scalaReflectLayer = scalaReflectJar + .map { file => + layer(file :: Nil, scalaLibraryLayer, cache, resources, tmp) + } + .getOrElse(scalaLibraryLayer) + // layer 2 (resources) val resourceLayer = if (layerDependencies) - getResourceLayer(cpFiles, resourceCP, scalaLibraryLayer, cache, resources) - else scalaLibraryLayer + getResourceLayer(cpFiles, resourceCP, scalaReflectLayer, cache, resources) + else scalaReflectLayer // layer 3 (optional if in the test config and the runtime layer is not shared) val dependencyLayer = @@ -148,8 +155,8 @@ private[sbt] object ClassLoaders { // layer 4 val filteredSet = - if (layerDependencies) allDependencies.toSet ++ si.libraryJars - else Set(si.libraryJars: _*) + if (layerDependencies) allDependencies.toSet ++ si.libraryJars ++ scalaReflectJar + else Set(si.libraryJars ++ scalaReflectJar: _*) val dynamicClasspath = cpFiles.filterNot(filteredSet) new LayeredClassLoader(dynamicClasspath, dependencyLayer, resources, tmp) }