Skip to content
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.

Sign up for GitHub

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

Make warm compilation speed with sbt 2x faster #2262

Merged
merged 2 commits into from
Apr 19, 2017
Merged

Make warm compilation speed with sbt 2x faster #2262

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d773a32
Make warm compilation speed with sbt 2x faster
smarter Apr 14, 2017
934ab5b
Make CompilerClassLoader#fixBridgeLoader synchronized
smarter Apr 15, 2017
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
15 changes: 14 additions & 1 deletion sbt-bridge/src/xsbt/CompilerClassLoader.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package xsbt

import java.net.{URL, URLClassLoader}

import scala.collection.mutable

/** A classloader to run the compiler
*
* A CompilerClassLoader is constructed from a list of `urls` that need to be on
Expand Down Expand Up @@ -42,6 +44,14 @@ class CompilerClassLoader(urls: Array[URL], sbtLoader: ClassLoader)
}

object CompilerClassLoader {
/** Cache the result of `fixBridgeLoader`.
*
* Reusing ClassLoaders is important for warm performance since otherwise the
* JIT code cache for the compiler will be discarded between every call to
* the sbt `compile` task.
*/
private[this] val fixedLoaderCache = new mutable.WeakHashMap[ClassLoader, ClassLoader]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in weak hashMap only values are weak. You are keeping SBT classloaders(keys) from being GC-d.
I'd prefer to have one more level of indirection to also remove this issue.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? From https://docs.oracle.com/javase/8/docs/api/java/util/WeakHashMap.html: "Hash table based implementation of the Map interface, with weak keys. An entry in a WeakHashMap will automatically be removed when its key is no longer in ordinary use."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right.


/** Fix the compiler bridge ClassLoader
*
* Soundtrack: https://www.youtube.com/watch?v=imamcajBEJs
Expand Down Expand Up @@ -70,7 +80,10 @@ object CompilerClassLoader {
* @param bridgeLoader The classloader that sbt uses to load the compiler bridge
* @return A fixed classloader that works with dotty
*/
def fixBridgeLoader(bridgeLoader: ClassLoader) = bridgeLoader match {
def fixBridgeLoader(bridgeLoader: ClassLoader): ClassLoader =
fixedLoaderCache.getOrElseUpdate(bridgeLoader, computeFixedLoader(bridgeLoader))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this method be synchronized?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW: CompilerClassLoader.loadClass should definitely be synchronized.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, this method should be synchronized. I don't think loadClass needs to be however because I'm only calling super.loadClass and resolveClass which should both be thread-safe. In fact, I think that I should be able to call https://docs.oracle.com/javase/8/docs/api/java/lang/ClassLoader.html#registerAsParallelCapable-- to make my ClassLoader parallel.


private[this] def computeFixedLoader(bridgeLoader: ClassLoader) = bridgeLoader match {
case bridgeLoader: URLClassLoader =>
val dualLoader = bridgeLoader.getParent
val dualLoaderClass = dualLoader.getClass
Expand Down