KAFKA-8991: Enable scalac optimizer

[ijuma]

The scalac optimizer is able to inline methods to avoid lambda allocations, eliminating
the runtime cost of higher order functions in many cases. The compilation parameters
we are using here were introduced in 2.12.x, so we don't enable them for Scala 2.11.
Also, we enable a more aggressive inlining policy for the core project since it's
not meant to be used as a library.

See https://www.lightbend.com/blog/scala-inliner-optimizer for more information about
the optimizer.

I verified that the lambda allocation in the code below (from LogCleaner.scala) went away
after this change with Scala 2.12 and 2.13.

private def consumeAbortedTxnsUpTo(offset: Long): Unit = {
  while (abortedTransactions.headOption.exists(_.firstOffset <= offset)) {
    val abortedTxn = abortedTransactions.dequeue()
    ongoingAbortedTxns.getOrElseUpdate(abortedTxn.producerId, new AbortedTransactionMetadata(abortedTxn))
  }
}

The relevant part of the bytecode when compiled with Scala 2.13 looks like:

private void consumeAbortedTxnsUpTo(long);
    Code:
       0: aload_0
       1: invokespecial #54                 // Method abortedTransactions:()Lscala/collection/mutable/PriorityQueue;
       4: invokevirtual #175                // Method scala/collection/mutable/PriorityQueue.headOption:()Lscala/Option;
       7: dup
       8: ifnonnull     13
      11: aconst_null
      12: athrow
      13: astore        4
      15: aload         4
      17: invokevirtual #145                // Method scala/Option.isEmpty:()Z
      20: ifne          48
      23: aload         4
      25: invokevirtual #148                // Method scala/Option.get:()Ljava/lang/Object;
      28: checkcast     #177                // class kafka/log/AbortedTxn

The increased inlining causes some spurious spotBugs warnings, I added a few suppressions
and fixed one warning by avoiding unnecessary boxing.

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[lbradstreet]

I ran the JMH ReplicaFetcher benchmark from #7443 with this change:

MBP 2015 2.5 GHz Intel Core i7

Before:
Benchmark                                  (partitionCount)  Mode  Cnt        Score       Error  Units
ReplicaFetcherThreadBenchmark.testFetcher               100  avgt   15    10463.937 ±   192.870  ns/op
ReplicaFetcherThreadBenchmark.testFetcher               500  avgt   15    72607.200 ±   522.900  ns/op
ReplicaFetcherThreadBenchmark.testFetcher              1000  avgt   15   165140.230 ±  1573.163  ns/op
ReplicaFetcherThreadBenchmark.testFetcher              5000  avgt   15  1382914.590 ± 17847.354  ns/op

After:
Benchmark                                  (partitionCount)  Mode  Cnt        Score       Error  Units
ReplicaFetcherThreadBenchmark.testFetcher               100  avgt   15    10207.193 ±   59.0646  ns/op
ReplicaFetcherThreadBenchmark.testFetcher               500  avgt   15    70814.440 ±   815.375  ns/op
ReplicaFetcherThreadBenchmark.testFetcher              1000  avgt   15   156053.360 ±  1153.965  ns/op
ReplicaFetcherThreadBenchmark.testFetcher              5000  avgt   15  1345613.534 ± 24347.230  ns/op

r5.xlarge:

Before:
Benchmark                                  (partitionCount)  Mode  Cnt        Score      Error  Units
ReplicaFetcherThreadBenchmark.testFetcher               100  avgt   15    11259.577 ±  259.435  ns/op
ReplicaFetcherThreadBenchmark.testFetcher               500  avgt   15    58342.187 ±  995.836  ns/op
ReplicaFetcherThreadBenchmark.testFetcher              1000  avgt   15   145711.558 ± 2470.027  ns/op
ReplicaFetcherThreadBenchmark.testFetcher              5000  avgt   15  1385902.170 ± 1743.255  ns/op

After:
Benchmark                                  (partitionCount)  Mode  Cnt        Score      Error  Units
ReplicaFetcherThreadBenchmark.testFetcher               100  avgt   15    10996.841 ±  186.663  ns/op
ReplicaFetcherThreadBenchmark.testFetcher               500  avgt   15    57141.687 ± 1592.744  ns/op
ReplicaFetcherThreadBenchmark.testFetcher              1000  avgt   15   145732.925 ± 2603.399  ns/op
ReplicaFetcherThreadBenchmark.testFetcher              5000  avgt   15  1329182.776 ±  397.121  ns/op

On the small partition count runs it looks like it's ~2% improvement, with other costs dominating as the partition count increases.

[guozhangwang]

Thanks for tackling this @ijuma !

[guozhangwang]

Curious why are these piggy-backs added here?

[ijuma]

It was causing a spotBugs failure after the inlining.

[ijuma]

One job passed, 2 failed with seemingly flaky tests.

[ijuma]

retest this please

[ijuma]

2 jobs passed, Scala 2.11 failed with a single failing test but Scala 2.11 is not affected by this PR. @guozhangwang @omkreddy Does the PR look good to you?

[guozhangwang]

LGTM!

[olafurpg]

@ijuma I apologize if this the wrong place to ask a question about this PR. What would be the best place to start a discussion on removing "-opt-inline-from:scala.**" or reducing the scope of the glob to only include relevant methods like Option.exists? I'm not so familiar with the Kafka development process.

At Twitter, we are currently unable to upgrade to 2.5.x because kafka_2.12:2.5.1 inlines anonymous classes from the 2.12.11 library that no longer exist in the 2.12.12 library, resulting in crashes like this

 java.lang.NoClassDefFoundError: scala/math/Ordering$$anon$7

We can work around this issue by publishing a custom build of Kafka but that's not a sustainable solution.

These inlining settings are causing issues for a lot of users based on my search online. Even members of the Scala compiler team who worked on the inliner have suggested that the inliner settings in the Kafka build are based on false assumptions akka/alpakka-kafka#1212 (comment) cc/ @lrytz

[ijuma]

You can submit a PR. :)

[olafurpg]

I opened #9548 :)

[ijuma]

Let's continue the discussion in #9548. Note the comment from @lrytz is regarding the claim that the core module should not be used as a library. This is the recommendation from the Kafka project though, so we can discuss in #9548 why users are depending on this module as a library (i.e. his expertise in the scala compiler/inliner is not relevant to this particular comment).