From 0b6a190169ed0b16558c7d5fd3ba365a1b6795b9 Mon Sep 17 00:00:00 2001 From: Josh Rosen Date: Tue, 31 May 2016 13:20:08 -0700 Subject: [PATCH 1/2] Use flag to disable comments in generated code. --- .../expressions/codegen/CodeGenerator.scala | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala index 2706c38d9e0a1..d0c69e62bbe57 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala @@ -24,6 +24,7 @@ import scala.language.existentials import com.google.common.cache.{CacheBuilder, CacheLoader} import org.codehaus.janino.ClassBodyEvaluator +import org.apache.spark.SparkEnv import org.apache.spark.internal.Logging import org.apache.spark.sql.catalyst.InternalRow import org.apache.spark.sql.catalyst.expressions._ @@ -720,15 +721,23 @@ class CodegenContext { /** * Register a comment and return the corresponding place holder */ - def registerComment(text: String): String = { - val name = freshName("c") - val comment = if (text.contains("\n") || text.contains("\r")) { - text.split("(\r\n)|\r|\n").mkString("/**\n * ", "\n * ", "\n */") + def registerComment(text: => String): String = { + // By default, disable comments in generated code because computing the comments themselves can + // be extremely expensive in certain cases, such as deeply-nested expressions which operate over + // inputs with wide schemas. For more details on the performance issues that motivated this + // flat, see SPARK-15680. + if (SparkEnv.get.conf.getBoolean("spark.sql.codegen.comments", false)) { + val name = freshName("c") + val comment = if (text.contains("\n") || text.contains("\r")) { + text.split("(\r\n)|\r|\n").mkString("/**\n * ", "\n * ", "\n */") + } else { + s"// $text" + } + placeHolderToComments += (name -> comment) + s"/*$name*/" } else { - s"// $text" + "" } - placeHolderToComments += (name -> comment) - s"/*$name*/" } } From db4624166bbb5e104923e77f222ce69c89c260fa Mon Sep 17 00:00:00 2001 From: Josh Rosen Date: Tue, 31 May 2016 14:42:34 -0700 Subject: [PATCH 2/2] Fix NPE in tests. --- .../spark/sql/catalyst/expressions/codegen/CodeGenerator.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala index d0c69e62bbe57..307af13ac72c4 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala @@ -726,7 +726,7 @@ class CodegenContext { // be extremely expensive in certain cases, such as deeply-nested expressions which operate over // inputs with wide schemas. For more details on the performance issues that motivated this // flat, see SPARK-15680. - if (SparkEnv.get.conf.getBoolean("spark.sql.codegen.comments", false)) { + if (SparkEnv.get != null && SparkEnv.get.conf.getBoolean("spark.sql.codegen.comments", false)) { val name = freshName("c") val comment = if (text.contains("\n") || text.contains("\r")) { text.split("(\r\n)|\r|\n").mkString("/**\n * ", "\n * ", "\n */")