From e56f8afa03035140280bc8d3d878ad225def381e Mon Sep 17 00:00:00 2001 From: Christopher Vogt Date: Sun, 27 Nov 2016 16:00:58 -0500 Subject: replace flawed concurrent hashmap cache with consistent replacement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The concurrent hashmap approach to classloader caching was flawed. Assume you have two concurrently running builds A and B and projects P2 and P3 depending on project P1. And assume a time sequence where A compiles P1, then compiles P2, then P1’s sources change, then B compiles P1, then A compiles P3. At the end P2 and P3 will have different versions of P1 as their parent classloaders. This is inconsistent. The easiest way to work around this is making sure only one thread is changing the classloader cache during it’s entire run. This would mean either no concurrency or what we have done here, which is letting threads work on a copy of the cache and replace the original cache in the end using an atomic operation. This means the thread that finishes last wins, but for caching that’s fine. Worst case some things aren’t cached in a concurrent execution. This change also means that we don’t need concurrent hashmaps for the classloader cache anymore since no two theads will access the same hashmap. We still need a concurrent hashmap for the class caches inside of the classloaders as multiple threads can access the same classloaders. --- stage1/ClassLoaderCache.scala | 4 ++-- stage1/ContextImplementation.scala | 5 ++--- stage1/KeyLockedLazyCache.scala | 4 +--- stage1/PoorMansProfiler.scala | 4 ++-- stage1/Stage1.scala | 4 ++-- stage1/Stage1Lib.scala | 1 - stage1/cbt.scala | 1 - stage1/resolver.scala | 2 -- 8 files changed, 9 insertions(+), 16 deletions(-) (limited to 'stage1') diff --git a/stage1/ClassLoaderCache.scala b/stage1/ClassLoaderCache.scala index 2011562..af0970e 100644 --- a/stage1/ClassLoaderCache.scala +++ b/stage1/ClassLoaderCache.scala @@ -1,12 +1,12 @@ package cbt import java.net._ -import java.util.concurrent.ConcurrentHashMap +import java.util._ import collection.JavaConverters._ case class ClassLoaderCache( logger: Logger, - private[cbt] hashMap: ConcurrentHashMap[AnyRef,AnyRef] + private[cbt] hashMap: java.util.Map[AnyRef,AnyRef] ){ val cache = new KeyLockedLazyCache[ClassLoader]( hashMap, Some(logger) ) override def toString = ( diff --git a/stage1/ContextImplementation.scala b/stage1/ContextImplementation.scala index 6eb2e53..3b610c0 100644 --- a/stage1/ContextImplementation.scala +++ b/stage1/ContextImplementation.scala @@ -1,6 +1,5 @@ package cbt import java.io._ -import java.util.concurrent.ConcurrentHashMap import java.lang._ case class ContextImplementation( @@ -11,8 +10,8 @@ case class ContextImplementation( startCompat: Long, cbtHasChangedCompat: Boolean, scalaVersionOrNull: String, - persistentCache: ConcurrentHashMap[AnyRef,AnyRef], - transientCache: ConcurrentHashMap[AnyRef,AnyRef], + persistentCache: java.util.Map[AnyRef,AnyRef], + transientCache: java.util.Map[AnyRef,AnyRef], cache: File, cbtHome: File, cbtRootHome: File, diff --git a/stage1/KeyLockedLazyCache.scala b/stage1/KeyLockedLazyCache.scala index 2602523..2047b81 100644 --- a/stage1/KeyLockedLazyCache.scala +++ b/stage1/KeyLockedLazyCache.scala @@ -1,7 +1,5 @@ package cbt -import java.util.concurrent.ConcurrentHashMap - private[cbt] class LockableKey /** A hashMap that lazily computes values if needed during lookup. @@ -9,7 +7,7 @@ Locking occurs on the key, so separate keys can be looked up simultaneously without a deadlock. */ final private[cbt] class KeyLockedLazyCache[T <: AnyRef]( - val hashMap: ConcurrentHashMap[AnyRef,AnyRef], + val hashMap: java.util.Map[AnyRef,AnyRef], logger: Option[Logger] ){ def get( key: AnyRef, value: => T ): T = { diff --git a/stage1/PoorMansProfiler.scala b/stage1/PoorMansProfiler.scala index b7aa47d..4bc44ba 100644 --- a/stage1/PoorMansProfiler.scala +++ b/stage1/PoorMansProfiler.scala @@ -1,10 +1,10 @@ /* // temporary debugging tool package cbt -import java.util.concurrent.ConcurrentHashMap +import java.util._ import collection.JavaConversions._ object PoorMansProfiler{ - val entries = new ConcurrentHashMap[String, Long] + val entries = new HashMap[String, Long] def profile[T](name: String)(code: => T): T = { val before = System.currentTimeMillis if(!(entries containsKey name)){ diff --git a/stage1/Stage1.scala b/stage1/Stage1.scala index cd46d6b..2f8f960 100644 --- a/stage1/Stage1.scala +++ b/stage1/Stage1.scala @@ -1,7 +1,7 @@ package cbt import java.io._ -import java.util.concurrent.ConcurrentHashMap +import java.util._ import scala.collection.JavaConverters._ @@ -159,7 +159,7 @@ object Stage1{ cbtHome: File, buildStage1: BuildStage1Result, start: java.lang.Long, - persistentCache: ConcurrentHashMap[AnyRef,AnyRef] + persistentCache: java.util.Map[AnyRef,AnyRef] ): Int = { val args = Stage1ArgsParser(_args.toVector) val logger = new Logger(args.enabledLoggers, start) diff --git a/stage1/Stage1Lib.scala b/stage1/Stage1Lib.scala index 8fdde54..9b48409 100644 --- a/stage1/Stage1Lib.scala +++ b/stage1/Stage1Lib.scala @@ -9,7 +9,6 @@ import java.nio.file.attribute.FileTime import javax.tools._ import java.security._ import java.util.{Set=>_,Map=>_,List=>_,_} -import java.util.concurrent.ConcurrentHashMap import javax.xml.bind.annotation.adapters.HexBinaryAdapter // CLI interop diff --git a/stage1/cbt.scala b/stage1/cbt.scala index 985f619..22242d7 100644 --- a/stage1/cbt.scala +++ b/stage1/cbt.scala @@ -2,7 +2,6 @@ package cbt import java.io._ import java.nio.file._ import java.net._ -import java.util.concurrent.ConcurrentHashMap object `package`{ implicit class TypeInferenceSafeEquals[T](value: T){ diff --git a/stage1/resolver.scala b/stage1/resolver.scala index ff5ad68..1f94c7f 100644 --- a/stage1/resolver.scala +++ b/stage1/resolver.scala @@ -4,8 +4,6 @@ import java.nio.charset.StandardCharsets import java.net._ import java.io._ import scala.xml._ -import scala.concurrent._ -import scala.concurrent.duration._ trait DependencyImplementation extends Dependency{ implicit protected def logger: Logger -- cgit v1.2.3