From 21f554cfcb5104a54432b7cec8405b51690f861c Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 24 Apr 2020 13:53:11 +1000 Subject: [PATCH] Memory efficiency for used names collection - Build the maps in the used name relation in a mutable map, then use a HashMap builder in a second pass to build the resulting maps. In Scala 2.12.12+ or 2.13.0+, use of the builder is fastest as the builder mutates the structure internally. - Intern the keys and values, so we don't have a separate instance of `UsedName("toString", EnumSet.of(UseScope.Default)` for each call site. --- .../sbt/internal/inc/IncrementalCompile.scala | 18 +++---- .../sbt/internal/inc/RelationBuilder.scala | 51 +++++++++++++++++++ .../scala/sbt/internal/inc/Relations.scala | 7 +-- 3 files changed, 64 insertions(+), 12 deletions(-) create mode 100644 internal/zinc-core/src/main/scala/sbt/internal/inc/RelationBuilder.scala diff --git a/internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCompile.scala b/internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCompile.scala index 96dd5e1a5a..aee7e4dde0 100644 --- a/internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCompile.scala +++ b/internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCompile.scala @@ -187,7 +187,7 @@ private final class AnalysisCallback( private[this] val objectApis = new TrieMap[String, ApiInfo] private[this] val classPublicNameHashes = new TrieMap[String, Array[NameHash]] private[this] val objectPublicNameHashes = new TrieMap[String, Array[NameHash]] - private[this] val usedNames = new TrieMap[String, ConcurrentSet[UsedName]] + private[this] val usedNames = new RelationBuilder[String, UsedName] private[this] val unreporteds = new TrieMap[VirtualFileRef, ConcurrentLinkedQueue[Problem]] private[this] val reporteds = new TrieMap[VirtualFileRef, ConcurrentLinkedQueue[Problem]] private[this] val mainClasses = new TrieMap[VirtualFileRef, ConcurrentLinkedQueue[String]] @@ -367,8 +367,11 @@ private final class AnalysisCallback( () } - def usedName(className: String, name: String, useScopes: util.EnumSet[UseScope]) = - add(usedNames, className, UsedName(name, useScopes)) + def usedName(className: String, name: String, useScopes: util.EnumSet[UseScope]) = { + usedNames.synchronized { + usedNames(className) = UsedName(name, useScopes) + } + } override def enabled(): Boolean = options.enabled @@ -381,12 +384,9 @@ private final class AnalysisCallback( def getOrNil[A, B](m: collection.Map[A, Seq[B]], a: A): Seq[B] = m.get(a).toList.flatten def addCompilation(base: Analysis): Analysis = base.copy(compilations = base.compilations.add(compilation)) - def addUsedNames(base: Analysis): Analysis = usedNames.foldLeft(base) { - case (a, (className, names)) => - import scala.collection.JavaConverters._ - names.asScala.foldLeft(a) { - case (a, name) => a.copy(relations = a.relations.addUsedName(className, name)) - } + def addUsedNames(base: Analysis): Analysis = { + assert(base.relations.names.size == 0) + base.copy(relations = base.relations.addUsedNames(usedNames.result())) } private def companionsWithHash(className: String): (Companions, HashAPI.Hash, HashAPI.Hash) = { diff --git a/internal/zinc-core/src/main/scala/sbt/internal/inc/RelationBuilder.scala b/internal/zinc-core/src/main/scala/sbt/internal/inc/RelationBuilder.scala new file mode 100644 index 0000000000..f54129540b --- /dev/null +++ b/internal/zinc-core/src/main/scala/sbt/internal/inc/RelationBuilder.scala @@ -0,0 +1,51 @@ +/* + * Zinc - The incremental compiler for Scala. + * Copyright Lightbend, Inc. and Mark Harrah + * + * Licensed under Apache License 2.0 + * (http://www.apache.org/licenses/LICENSE-2.0). + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package sbt.internal.inc + +import sbt.internal.util.Relation + +import scala.collection.{ immutable, mutable } + +final class RelationBuilder[A <: AnyRef, B <: AnyRef]() { + private[this] val forward = + new java.util.HashMap[A, (A, mutable.Builder[B, immutable.HashSet[B]])]() + private[this] val reverse = + new java.util.HashMap[B, (B, mutable.Builder[A, immutable.HashSet[A]])]() + + def update(a: A, b: B): Unit = { + val (internedB, asBuilder) = + reverse.computeIfAbsent(b, (b => (b, immutable.HashSet.newBuilder[A]))) + val (internedA, bsBuilder) = + forward.computeIfAbsent(a, (a => (a, immutable.HashSet.newBuilder[B]))) + asBuilder += internedA + bsBuilder += internedB + } + + def +=(other: Relation[A, B]): Unit = { + for ((a, bs) <- other.forwardMap.iterator) { + for (b <- bs) { + update(a, b) + } + } + } + + def result(): Relation[A, B] = { + def toImmutable[K, V]( + map: java.util.HashMap[K, (K, mutable.Builder[V, immutable.HashSet[V]])] + ): immutable.HashMap[K, immutable.HashSet[V]] = { + val builder = immutable.HashMap.newBuilder[K, immutable.HashSet[V]] + map.entrySet().forEach(e => builder.+=((e.getKey, e.getValue._2.result()))) + builder.result() + } + Relation.make[A, B](toImmutable(forward), toImmutable(reverse)) + } +} diff --git a/internal/zinc-core/src/main/scala/sbt/internal/inc/Relations.scala b/internal/zinc-core/src/main/scala/sbt/internal/inc/Relations.scala index ff0b29b20b..35db361d4c 100644 --- a/internal/zinc-core/src/main/scala/sbt/internal/inc/Relations.scala +++ b/internal/zinc-core/src/main/scala/sbt/internal/inc/Relations.scala @@ -140,7 +140,7 @@ trait Relations { deps: Iterable[(VirtualFileRef, String, XStamp)] ): Relations - private[inc] def addUsedName(className: String, name: UsedName): Relations + private[inc] def addUsedNames(data: Relation[String, UsedName]): Relations /** Concatenates the two relations. Acts naively, i.e., doesn't internalize external deps on added files. */ def ++(o: Relations): Relations @@ -636,7 +636,7 @@ private class MRelationsNameHashing( productClassName = productClassName ) - override private[inc] def addUsedName(className: String, name: UsedName): Relations = + private[inc] def addUsedNames(data: Relation[String, UsedName]): Relations = { new MRelationsNameHashing( srcProd, libraryDep, @@ -644,9 +644,10 @@ private class MRelationsNameHashing( internalDependencies = internalDependencies, externalDependencies = externalDependencies, classes, - names = names + (className, name), + names = if (names.forwardMap.isEmpty) data else names ++ data, productClassName = productClassName ) + } override def inheritance: ClassDependencies = new ClassDependencies(