Avoid generic array instantation in hot paths #5911

retronym · 2017-05-17T09:47:35Z

Before:

[info] 
[info] Benchmark                                   (extraArgs)  (source)    Mode  Cnt     Score   Error  Units
[info] HotScalacBenchmark.compile                                         sample  500  1049.371 ± 3.062  ms/op
[info] HotScalacBenchmark.compile:compile·p0.00                           sample       1000.342          ms/op
[info] HotScalacBenchmark.compile:compile·p0.50                           sample       1047.527          ms/op
[info] HotScalacBenchmark.compile:compile·p0.90                           sample       1075.839          ms/op
[info] HotScalacBenchmark.compile:compile·p0.95                           sample       1082.130          ms/op
[info] HotScalacBenchmark.compile:compile·p0.99                           sample       1113.462          ms/op
[info] HotScalacBenchmark.compile:compile·p0.999                          sample       1176.502          ms/op
[info] HotScalacBenchmark.compile:compile·p0.9999                         sample       1176.502          ms/op
[info] HotScalacBenchmark.compile:compile·p1.00                           sample       1176.502          ms/op

After:

[info] Benchmark                                   (extraArgs)  (source)    Mode  Cnt     Score   Error  Units
[info] HotScalacBenchmark.compile                                         sample  500  1045.162 ± 2.888  ms/op
[info] HotScalacBenchmark.compile:compile·p0.00                           sample        984.613          ms/op
[info] HotScalacBenchmark.compile:compile·p0.50                           sample       1046.479          ms/op
[info] HotScalacBenchmark.compile:compile·p0.90                           sample       1068.499          ms/op
[info] HotScalacBenchmark.compile:compile·p0.95                           sample       1075.839          ms/op
[info] HotScalacBenchmark.compile:compile·p0.99                           sample       1090.498          ms/op
[info] HotScalacBenchmark.compile:compile·p0.999                          sample       1117.782          ms/op
[info] HotScalacBenchmark.compile:compile·p0.9999                         sample       1117.782          ms/op
[info] HotScalacBenchmark.compile:compile·p1.00                           sample       1117.782          ms/op

rorygraves · 2017-05-17T10:19:39Z

src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala

+      } else {
+        val result = new Array[asm.Type](numCapturedParams)
+        result(0) = typeToBType(lambdaTarget.owner.info).toASMType
+        capturedParams.iterator.map(sym => typeToBType(sym.info).toASMType).copyToArray(result, 1)


There is a bug here. This loses the last parameter. The array should be numCapturedParams+1 - copyToArray will simply throw away the arguments that do not fix.

Is numCapturedParams ever 0? If so this could be optimised further.

Good catch. Turns out the second branch (!isStatic) is never taken, as we chose to make all lambda implementation methods static, which meant we could leave them public without having to mangle the names. But I've pushed a fix in case this becomes live code in the future.

mkeskells · 2017-05-17T10:24:20Z

src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala

-      val (capturedParams, lambdaParams) = lambdaTarget.paramss.head.splitAt(lambdaTarget.paramss.head.length - arity)
-      val invokedType = asm.Type.getMethodDescriptor(asmType(functionalInterface), (receiver ::: capturedParams).map(sym => typeToBType(sym.info).toASMType): _*)
+      val numCapturedParams = lambdaTarget.paramss.head.length - arity
+      val (capturedParams, lambdaParams) = lambdaTarget.paramss.head.splitAt(numCapturedParams)


there is a special case where arity == 0 which avoid the splitAt

List.splitAt could be optimized where split == 0 and in for Nil - seperate PR probably

mkeskells · 2017-05-17T10:26:04Z

src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala

@@ -501,7 +501,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
      currentRun.symData get sym match {
        case Some(pickle) if !nme.isModuleName(newTermName(jclassName)) =>
          val scalaAnnot = {
-            val sigBytes = ScalaSigBytes(pickle.bytes.take(pickle.writeIndex))
+            val sigBytes = ScalaSigBytes(java.util.Arrays.copyOf(pickle.bytes, pickle.writeIndex))


why this change - isnt this what .take does as we fixed the Array methods

Good point. We had to revert that change in #5821 because it wasn't binary compatible. We should re-apply the original patch to the 2.13.x branch.

retronym · 2017-05-18T03:13:32Z

I"m going to close this PR for now as the benchmark isn't conclusive. I re-ran it and got 1054ms vs 1056ms.

scala-jenkins added this to the 2.12.3 milestone May 17, 2017

retronym force-pushed the faster/newarray branch from 4155af0 to 3da470b Compare May 17, 2017 09:48

rorygraves reviewed May 17, 2017

View reviewed changes

mkeskells reviewed May 17, 2017

View reviewed changes

Avoid generic array instantation in hot paths

b17cfe5

retronym force-pushed the faster/newarray branch from 3da470b to b17cfe5 Compare May 18, 2017 01:35

retronym closed this May 18, 2017

SethTisue removed this from the 2.12.3 milestone Jun 27, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Avoid generic array instantation in hot paths #5911

Avoid generic array instantation in hot paths #5911

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Avoid generic array instantation in hot paths #5911

Avoid generic array instantation in hot paths #5911

Uh oh!

Conversation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!