Memory efficiency for used names collection and other Relations building

[retronym]

• 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.
• Factor this into RelationBuilder, and use this in
  ProtobufReaders where I recently made similar opts.

[retronym]

scala/scala#8726 is the 2.12.x backport of efficient immutable.HashMap$Builder (thanks, @mkeskells!). With that, or with 2.13, this step is step is quite efficient.

I considered an alternative of just doing an unsafeToImmutableMap wrapper, but that is a somewhat more fragile if the users of the built Relation start to call map operations on it.

It would be great if we had a richer immutable.HashMap.Builder that supported key getOrElseUpdate on the map being built, as well as a find transformValues step. Then, we could build up the desired structure from the begining and do a second pass that just mutates the data in the leaves.

[dwijnand]

A light-touch/non-paranoid LGTM.

[retronym]

Note to self: the benchmarks are unlikely to show much benefit on 2.12.11. Run on 2.12.x-SNAPSHOT instead.

In that build:

• ambient allocations are reduced by sharing the buffer for classfile reading, so we can better focus on the changes in this PR.
• immutable.HashMap.newBuilder is way more efficient.

[dotta]

@retronym Hey! :)

I'm concerned that with this change we may experience concurrency hazards when compiling with Hydra (such as visibility issues), as the underlying AnalysisCallback instance is shared between the different compilation threads. If you agree, could we find a way to make this change compatible with Hydra?

\cc @dragos @eed3si9n

[retronym]

Yep, that's totally fine with me. Looking at this again, I see I did synchronizes writes to the usedName RelationBuilder, but I don't have a synchronized in the final read in def addUsedNames so updates from the threads might not be visible.

[retronym]

You might want a different performance tradeoff (preferring less potential for contention and worrying less about allocation pressure). In that case you could create a second implementation of RelationBuilder that is more like the original code (TrieMap-based) and enable that in Hydra.

[dotta]

Yep, that's it. Problem is that swapping the AnalysisCallback from the outside (i.e., from a sbt plugin) isn't really possible at the moment (and we should do it in a way that works within the IntelliJ scala-plugin, as it leverages zinc and we have a Hydra integration in IntelliJ too). This doesn't look simple to achieve.

As the changes in this PR are motivated by (allocation) performance, what's the order of magnitude of the savings obtained using the current implementation? (I suspect you have been seeing a meaningful difference in large code bases, haven't you? If so, could you share some figures) By the way, would you have a benchmark that we can use to see if we can tweak the implementation in a way that would make it work with Hydra too, and at no meaningful penalty for vanilla Scala?

[dotta]

@retronym ping :)

[retronym]

@dotta We've now got a way to avoid building relations (bi-directional maps) for used names. So we can go back to the lock-free data structures: https://github.com/sbt/zinc/pull/995/files#r677157060