Improve performance of loading used names from persisted Analysis file

[dwijnand]

Refs #984

[eed3si9n]

Thanks!

[retronym]

This is a very late pong to this ping from @dotta

[retronym]

Running this benchmark prior:

package sbt.internal.inc

import org.openjdk.jmh.annotations.{
  Benchmark,
  BenchmarkMode,
  Fork,
  Measurement,
  Mode,
  Param,
  Scope,
  Setup,
  State,
  Warmup
}
import xsbti.UseScope

import java.io.File
import java.util.concurrent.TimeUnit

@State(Scope.Benchmark)
@BenchmarkMode(Array(Mode.AverageTime))
@Measurement(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS)
@Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
@Fork(value = 3)
class AnalysisSerializationBenchmark {
  @Param(Array("/Users/jz/code/scala/target/compiler/zinc/inc_compile.zip"))
  var analysisFile: String = _
  var firstClassName: String = _

  @Setup
  def setup(): Unit = {
    val store = FileAnalysisStore.binary(new File(analysisFile))
    val analysis = store.get().get().getAnalysis.asInstanceOf[Analysis]
    firstClassName = analysis.relations.classes._2s.head
  }

  @Benchmark def deserialize() = {
    val store = FileAnalysisStore.binary(new File(analysisFile))
    val analysis = store.get().get().getAnalysis.asInstanceOf[Analysis]
    val usedNames = analysis.relations.names
    val mod = ModifiedNames(
      Set(
        UsedName.make(
          "name_that_does_not_exist_QWERTY",
          java.util.EnumSet.allOf[UseScope](classOf[UseScope])
        )
      )
    )
    usedNames.forward(firstClassName).iterator.exists(mod.isModified)
  }
}

And the slightly modified version post:

 @Benchmark def deserialize() = {
    val store = FileAnalysisStore.binary(new File(analysisFile))
    val analysis = store.get().get().getAnalysis.asInstanceOf[Analysis]
    val usedNames = analysis.relations.names
    usedNames.hasAffectedNames(
      ModifiedNames(
        Set(
          UsedName.make(
            "name_that_does_not_exist_QWERTY",
            java.util.EnumSet.allOf[UseScope](classOf[UseScope])
          )
        )
      ),
      firstClassName
    )
  }

Shows:

Baseline

[info] Benchmark                                                                                                               (analysisFile)  Mode  Cnt          Score         Error   Units
[info] AnalysisSerializationBenchmark.deserialize                                   /Users/jz/code/scala/target/compiler/zinc/inc_compile.zip  avgt   20          0.329 ±       0.020    s/op
[info] AnalysisSerializationBenchmark.deserialize:·gc.alloc.rate.norm               /Users/jz/code/scala/target/compiler/zinc/inc_compile.zip  avgt   20  205411175.467 ± 3736502.161    B/op

Post

[info] Benchmark                                                                                                               (analysisFile)  Mode  Cnt          Score          Error   Units
[info] AnalysisSerializationBenchmark.deserialize                                   /Users/jz/code/scala/target/compiler/zinc/inc_compile.zip  avgt   20          0.227 ±        0.010    s/op
[info] AnalysisSerializationBenchmark.deserialize:·gc.alloc.rate.norm               /Users/jz/code/scala/target/compiler/zinc/inc_compile.zip  avgt   20  170022246.320 ±   526307.356    B/op

Post "only intern used names locally ..."

[info] Benchmark                                                                              (analysisFile)  Mode  Cnt  Score   Error  Units
[info] AnalysisSerializationBenchmark.deserialize  /Users/jz/code/scala/target/compiler/zinc/inc_compile.zip  avgt   30  0.186 ± 0.008   s/op

[retronym]

Test failure, I think unrelated:

 [info] - should not compile Java for no-op *** FAILED ***
[info]   java.lang.RuntimeException: java.lang.IllegalArgumentException: MALFORMED
[info]   at com.sun.tools.javac.main.Main.compile(Main.java:559)
[info]   at com.sun.tools.javac.api.JavacTaskImpl.doCall(JavacTaskImpl.java:129)
[info]   at com.sun.tools.javac.api.JavacTaskImpl.call(JavacTaskImpl.java:138)
[info]   at sbt.internal.inc.javac.LocalJavaCompiler.run(LocalJava.scala:345)
[info]   at sbt.internal.inc.javac.AnalyzingJavaCompiler.$anonfun$compile$12(AnalyzingJavaCompiler.scala:172)
[info]   at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
[info]   at sbt.internal.inc.javac.AnalyzingJavaCompiler.timed(AnalyzingJavaCompiler.scala:262)
[info]   at sbt.internal.inc.javac.AnalyzingJavaCompiler.compile(AnalyzingJavaCompiler.scala:161)
[info]   at sbt.internal.inc.MixedAnalyzingCompiler.$anonfun$compileJava$2(MixedAnalyzingCompiler.scala:103)
[info]   at sbt.internal.inc.MixedAnalyzingCompiler.$anonfun$compileJava$2$adapted(MixedAnalyzingCompiler.scala:91)
[info]   ...
[info]   Cause: java.lang.IllegalArgumentException: MALFORMED
[info]   at java.util.zip.ZipCoder.toString(ZipCoder.java:58)
[info]   at java.util.zip.ZipFile.getZipEntry(ZipFile.java:599)
[info]   at java.util.zip.ZipFile.access$900(ZipFile.java:60)
[info]   at java.util.zip.ZipFile$ZipEntryIterator.next(ZipFile.java:539)
[info]   at java.util.zip.ZipFile$ZipEntryIterator.nextElement(ZipFile.java:514)
[info]   at java.util.zip.ZipFile$ZipEntryIterator.nextElement(ZipFile.java:495)
[info]   at com.sun.tools.javac.file.ZipArchive.initMap(ZipArchive.java:77)
[info]   at com.sun.tools.javac.file.ZipArchive.<init>(ZipArchive.java:70)
[info]   at com.sun.tools.javac.file.ZipArchive.<init>(ZipArchive.java:62)
[info]   at com.sun.tools.javac.file.JavacFileManager.openArchive(JavacFileManager.java:526)
[info]   ...

[retronym]

I did a little more analysis of variations of used-names interning to convince myself that the new, faster version of is "good enough" for footprint reduction.

package sbt.inc.binary

import sbt.internal.inc.FileAnalysisStore

import java.io.File

object Scratch {
  def main(args: Array[String]): Unit = {
    val files = List(
      "/Users/jz/code/scala/target/partest/zinc/inc_compile.zip",
      "/Users/jz/code/scala/target/scaladoc/zinc/inc_compile.zip",
      "/Users/jz/code/scala/target/replFrontend/zinc/inc_compile.zip",
      "/Users/jz/code/scala/target/reflect/zinc/inc_compile.zip",
      "/Users/jz/code/scala/target/repl/zinc/inc_compile.zip",
      "/Users/jz/code/scala/target/library/zinc/inc_compile.zip",
      "/Users/jz/code/scala/target/testkit/zinc/inc_compile.zip",
      "/Users/jz/code/scala/target/scalap/zinc/inc_compile.zip",
      "/Users/jz/code/scala/target/interactive/zinc/inc_compile.zip",
      "/Users/jz/code/scala/target/compiler/zinc/inc_compile.zip"
    )
    val analysisList = // comment out this line for the "ZERO" variation
      files.map(f => FileAnalysisStore.binary(new File(f)).get().get().getAnalysis)
    println("Loaded all analysis files")
    while (true) {
      Thread.sleep(1000) // run: jcmd $PID GC.heap_dump /tmp/dump.hprof and open with IntelliJ or another tool
    }
  }
}

  private class StringTable {
    private val strings = new JHashMap[String, String]()
    def lookupOrEnter(string: String): String = {
      string.intern() // STRING INTERN, or
      string          // NO INTERN, or 
      strings.putIfAbsent(string, string) match { // LOCAL INTERN
        case null => string
        case v    => v
      }
    }
  }

Variation	Description	Footprint (bytes)
ZERO	Measure the baseline footprint by loading then discaring the Analysis files	5,012,612
NO INTERN	No interning	277,412,693
STRING INTERN	Use j.l.String.intern (status quo)	231,665,921
LOCAL INTERN	This PR, intern with the scope of an each Analysis	235,342,817

In summary, I think this PR makes the right tradeoff.

The compressed protobuf files amount to 9.5M on disk, so we have a 25x higher footprint as Java objects. This is work some more investigation to look see if we can be more compact in memory.

[retronym]

We can reduce that footprint by 20% by avoiding the ArrayList[Schema.UseScope] which comes from the protobuf bindings over:

message UsedName {
    string name = 1;
    repeated UseScope scopes = 2;
}

enum UseScope {
    DEFAULT = 0;
    IMPLICIT = 1;
    PATMAT = 2;
}

It would be preferable if the bindings automatically represented this with an EnumSet or just a raw bitmask.

We can manually do the bitmasking ourselves though:

retronym#5

The overhead would also be eliminated if we went ahead with another idea we considered:

message UsedNames {
    repeated string regular = 1;
    repeated string implicit = 2;
    repeated string patmat = 3;
}

That would also get rid of a further 8% overhead of the Schema.UsedName instances.