From 78d0b133c99649db80d1d666ca35b1cb51d3a646 Mon Sep 17 00:00:00 2001 From: Josh Rosen Date: Fri, 9 Jun 2017 12:26:14 -0700 Subject: [PATCH 1/2] Avoid unnecessary StringBuilder construction and int boxing. --- .../shared/src/main/scala/scoverage/Invoker.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scalac-scoverage-runtime/shared/src/main/scala/scoverage/Invoker.scala b/scalac-scoverage-runtime/shared/src/main/scala/scoverage/Invoker.scala index 5ee0c5a7..203ff9e5 100644 --- a/scalac-scoverage-runtime/shared/src/main/scala/scoverage/Invoker.scala +++ b/scalac-scoverage-runtime/shared/src/main/scala/scoverage/Invoker.scala @@ -40,7 +40,7 @@ object Invoker { threadFiles.set(files) } val writer = files.getOrElseUpdate(dataDir, new FileWriter(measurementFile(dataDir), true)) - writer.append(id.toString + '\n').flush() + writer.append(Integer.toString(id)).append("\n").flush() ids.put((dataDir, id), ()) } From 4626fd8d3c0705b2a7fc7e11932508231c8fcead Mon Sep 17 00:00:00 2001 From: Josh Rosen Date: Mon, 12 Jun 2017 15:08:45 -0700 Subject: [PATCH 2/2] Use two levels of maps to avoid hashing tuples on every lookup. --- .../src/main/scala/scoverage/Invoker.scala | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/scalac-scoverage-runtime/shared/src/main/scala/scoverage/Invoker.scala b/scalac-scoverage-runtime/shared/src/main/scala/scoverage/Invoker.scala index 203ff9e5..f7d222bf 100644 --- a/scalac-scoverage-runtime/shared/src/main/scala/scoverage/Invoker.scala +++ b/scalac-scoverage-runtime/shared/src/main/scala/scoverage/Invoker.scala @@ -7,8 +7,12 @@ import scoverage.Platform._ object Invoker { private val MeasurementsPrefix = "scoverage.measurements." - private val threadFiles = new ThreadLocal[ThreadSafeMap[String, FileWriter]] - private val ids = ThreadSafeMap.empty[(String, Int), Any] + private val threadFiles = new ThreadLocal[mutable.HashMap[String, FileWriter]] + + // For each data directory we maintain a thread-safe set tracking the ids that we've already + // seen and recorded. We're using a map as a set, so we only care about its keys and can ignore + // its values. + private val dataDirToIds = ThreadSafeMap.empty[String, ThreadSafeMap[Int, Any]] /** * We record that the given id has been invoked by appending its id to the coverage @@ -31,18 +35,27 @@ object Invoker { // times since for coverage we only care about 1 or more, (it just slows things down to // do it more than once), anything we can do to help is good. This helps especially with code // that is executed many times quickly, eg tight loops. - if (!ids.contains(dataDir, id)) { + if (!dataDirToIds.contains(dataDir)) { + // Guard against SI-7943: "TrieMap method getOrElseUpdate is not thread-safe". + dataDirToIds.synchronized { + if (!dataDirToIds.contains(dataDir)) { + dataDirToIds(dataDir) = ThreadSafeMap.empty[Int, Any] + } + } + } + val ids = dataDirToIds(dataDir) + if (!ids.contains(id)) { // Each thread writes to a separate measurement file, to reduce contention // and because file appends via FileWriter are not atomic on Windows. var files = threadFiles.get() if (files == null) { - files = ThreadSafeMap.empty[String, FileWriter] + files = mutable.HashMap.empty[String, FileWriter] threadFiles.set(files) } val writer = files.getOrElseUpdate(dataDir, new FileWriter(measurementFile(dataDir), true)) writer.append(Integer.toString(id)).append("\n").flush() - ids.put((dataDir, id), ()) + ids.put(id, ()) } }