You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kafka.apache.org by ij...@apache.org on 2021/02/23 06:07:16 UTC

[kafka] 01/02: KAFKA-12357: Do not inline methods from the scala package by default (#10174)

This is an automated email from the ASF dual-hosted git repository.

ijuma pushed a commit to branch 2.8
in repository https://gitbox.apache.org/repos/asf/kafka.git

commit 58c2ed422e27213dedbac2d59da266f6f2cd97ea
Author: Ismael Juma <is...@juma.me.uk>
AuthorDate: Mon Feb 22 21:11:17 2021 -0800

    KAFKA-12357: Do not inline methods from the scala package by default (#10174)
    
    As mentioned in #9548, users currently use the kafka jar (`core` module)
    for integration testing and the current inlining behavior causes
    problems when the user's classpath contains a different Scala version
    than the one that was used for compilation (e.g. 2.13.4 versus 2.13.3).
    
    An example error:
    
    `java.lang.NoClassDefFoundError: scala/math/Ordering$$anon$7`
    
    We now disable inlining of the `scala` package by default, but make it
    easy to enable it for those who so desire (a good option if you can
    ensure the scala library version matches the one used for compilation).
    While at it, we make it possible to disable scala compiler optimizations
    (`none`) or to use only method local optimizations (`method`). This can
    be useful if optimizing for compilation time during development.
    
    Verified behavior by running gradlew with `--debug` and checking the
    output after `[zinc] The Scala compiler is invoked with:`
    
    Reviewers: Chia-Ping Tsai <ch...@gmail.com>
---
 README.md    |  7 +++++++
 build.gradle | 32 ++++++++++++++++++--------------
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/README.md b/README.md
index 34e13b6..3c3f013 100644
--- a/README.md
+++ b/README.md
@@ -221,6 +221,13 @@ The following options should be set with a `-P` switch, for example `./gradlew -
 * `enableTestCoverage`: enables test coverage plugins and tasks, including bytecode enhancement of classes required to track said
 coverage. Note that this introduces some overhead when running tests and hence why it's disabled by default (the overhead
 varies, but 15-20% is a reasonable estimate).
+* `scalaOptimizerMode`: configures the optimizing behavior of the scala compiler, the value should be one of `none`, `method`, `inline-kafka` or
+`inline-scala` (the default is `inline-kafka`). `none` is the scala compiler default, which only eliminates unreachable code. `method` also
+includes method-local optimizations. `inline-kafka` adds inlining of methods within the kafka packages. Finally, `inline-scala` also
+includes inlining of methods within the scala library (which avoids lambda allocations for methods like `Option.exists`). `inline-scala` is
+only safe if the Scala library version is the same at compile time and runtime. Since we cannot guarantee this for all cases (for example, users
+may depend on the kafka jar for integration tests where they may include a scala library with a different version), we don't enable it by
+default. See https://www.lightbend.com/blog/scala-inliner-optimizer for more details.
 
 ### Dependency Analysis ###
 
diff --git a/build.gradle b/build.gradle
index bf92f96..e9b2512 100644
--- a/build.gradle
+++ b/build.gradle
@@ -134,6 +134,12 @@ ext {
 
   userEnableTestCoverage = project.hasProperty("enableTestCoverage") ? enableTestCoverage : false
 
+  // See README.md for details on this option and the reasoning for the default
+  userScalaOptimizerMode = project.hasProperty("scalaOptimizerMode") ? scalaOptimizerMode : "inline-kafka"
+  def scalaOptimizerValues = ["none", "method", "inline-kafka", "inline-scala"]
+  if (!scalaOptimizerValues.contains(userScalaOptimizerMode))
+    throw new GradleException("Unexpected value for scalaOptimizerMode property. Expected one of $scalaOptimizerValues), but received: $userScalaOptimizerMode")
+
   generatedDocsDir = new File("${project.rootDir}/docs/generated")
 
   commitId = project.hasProperty('commitId') ? commitId : null
@@ -515,21 +521,19 @@ subprojects {
       "-Xlint:unused"
     ]
 
-    // Inline more aggressively when compiling the `core` jar since it's not meant to be used as a library.
-    // More specifically, inline classes from the Scala library so that we can inline methods like `Option.exists`
-    // and avoid lambda allocations. This is only safe if the Scala library version is the same at compile time
-    // and runtime. We cannot guarantee this for libraries like kafka streams, so only inline classes from the
-    // Kafka project in that case.
-    List<String> inlineFrom
-    if (project.name.equals('core'))
-      inlineFrom = ["-opt-inline-from:scala.**", "-opt-inline-from:kafka.**", "-opt-inline-from:org.apache.kafka.**"]
-    else
-      inlineFrom = ["-opt-inline-from:org.apache.kafka.**"]
+    // See README.md for details on this option and the meaning of each value
+    if (userScalaOptimizerMode.equals("method"))
+      scalaCompileOptions.additionalParameters += ["-opt:l:method"]
+    else if (userScalaOptimizerMode.startsWith("inline-")) {
+      List<String> inlineFrom = ["-opt-inline-from:org.apache.kafka.**"]
+      if (project.name.equals('core'))
+        inlineFrom.add("-opt-inline-from:kafka.**")
+      if (userScalaOptimizerMode.equals("inline-scala"))
+        inlineFrom.add("-opt-inline-from:scala.**")
 
-    // Somewhat confusingly, `-opt:l:inline` enables all optimizations. `inlineFrom` configures what can be inlined.
-    // See https://www.lightbend.com/blog/scala-inliner-optimizer for more information about the optimizer.
-    scalaCompileOptions.additionalParameters += ["-opt:l:inline"]
-    scalaCompileOptions.additionalParameters += inlineFrom
+      scalaCompileOptions.additionalParameters += ["-opt:l:inline"]
+      scalaCompileOptions.additionalParameters += inlineFrom
+    }
 
     if (versions.baseScala != '2.12') {
       scalaCompileOptions.additionalParameters += ["-opt-warnings", "-Xlint:strict-unsealed-patmat"]