[SPARK-21603][SQL]The wholestage codegen will be much slower then that is closed when the function is too long #18810

eatoncys · 2017-08-02T04:56:53Z

What changes were proposed in this pull request?

Close the whole stage codegen when the function lines is longer than the maxlines which will be setted by
spark.sql.codegen.MaxFunctionLength parameter, because when the function is too long , it will not get the JIT optimizing.
A benchmark test result is 10x slower when the generated function is too long :

ignore("max function length of wholestagecodegen") {
val N = 20 << 15

val benchmark = new Benchmark("max function length of wholestagecodegen", N)
def f(): Unit = sparkSession.range(N)
  .selectExpr(
    "id",
    "(id & 1023) as k1",
    "cast(id & 1023 as double) as k2",
    "cast(id & 1023 as int) as k3",
    "case when id > 100 and id <= 200 then 1 else 0 end as v1",
    "case when id > 200 and id <= 300 then 1 else 0 end as v2",
    "case when id > 300 and id <= 400 then 1 else 0 end as v3",
    "case when id > 400 and id <= 500 then 1 else 0 end as v4",
    "case when id > 500 and id <= 600 then 1 else 0 end as v5",
    "case when id > 600 and id <= 700 then 1 else 0 end as v6",
    "case when id > 700 and id <= 800 then 1 else 0 end as v7",
    "case when id > 800 and id <= 900 then 1 else 0 end as v8",
    "case when id > 900 and id <= 1000 then 1 else 0 end as v9",
    "case when id > 1000 and id <= 1100 then 1 else 0 end as v10",
    "case when id > 1100 and id <= 1200 then 1 else 0 end as v11",
    "case when id > 1200 and id <= 1300 then 1 else 0 end as v12",
    "case when id > 1300 and id <= 1400 then 1 else 0 end as v13",
    "case when id > 1400 and id <= 1500 then 1 else 0 end as v14",
    "case when id > 1500 and id <= 1600 then 1 else 0 end as v15",
    "case when id > 1600 and id <= 1700 then 1 else 0 end as v16",
    "case when id > 1700 and id <= 1800 then 1 else 0 end as v17",
    "case when id > 1800 and id <= 1900 then 1 else 0 end as v18")
  .groupBy("k1", "k2", "k3")
  .sum()
  .collect()

benchmark.addCase(s"codegen = F") { iter =>
  sparkSession.conf.set("spark.sql.codegen.wholeStage", "false")
  f()
}

benchmark.addCase(s"codegen = T") { iter =>
  sparkSession.conf.set("spark.sql.codegen.wholeStage", "true")
  sparkSession.conf.set("spark.sql.codegen.MaxFunctionLength", "10000")
  f()
}

benchmark.run()

/*
Java HotSpot(TM) 64-Bit Server VM 1.8.0_111-b14 on Windows 7 6.1
Intel64 Family 6 Model 58 Stepping 9, GenuineIntel
max function length of wholestagecodegen: Best/Avg Time(ms)    Rate(M/s)   Per Row(ns)   Relative
------------------------------------------------------------------------------------------------
codegen = F                                    443 /  507          1.5         676.0       1.0X
codegen = T                                   3279 / 3283          0.2        5002.6       0.1X
 */

}

How was this patch tested?

Run the unit test

kiszk · 2017-08-03T05:25:12Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala

+  def existTooLongFunction(): Boolean = {
+    classFunctions.exists { case (className, functions) =>
+      functions.exists{ case (name, code) =>
+        CodeFormatter.stripExtraNewLines(code).count(_ == '\n') > SQLConf.get.maxFunctionLength


Could you elaborate why only the number of characters for a function is a decision factor to enable or disable the whole-stage codegen?

@kiszk Because when the JVM parameter -XX:+DontCompileHugeMethods is true, it can not get the JIT optimization when the byte code of a function is longer than 8000, I just estimate a function lines by 8000 byte code, maybe there are some other good ways.

Got it. Thank you for your explanation. It would be good to add comment for this reasoning.

We have seen the similar control at here. Can we unify these control mechanisms into one?

Does this line counts include code comments in the functions? Do we need to strip comments?

kiszk · 2017-08-03T05:26:12Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

+    .doc("The maximum number of function length that will be supported before" +
+    " deactivating whole-stage codegen.")
+    .intConf
+    .createWithDefault(1500)


Would it be possible to explain why 1500 is the good value as default?

kiszk · 2017-08-03T05:26:55Z

sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/AggregateBenchmark.scala

+
+    benchmark.addCase(s"codegen = T") { iter =>
+      sparkSession.conf.set("spark.sql.codegen.wholeStage", "true")
+      sparkSession.conf.set("spark.sql.codegen.MaxFunctionLength", "10000")


Why doesn't this benchmark use the default number 1500?

Ok, I have added a test use the default number 1500, thanks.

gatorsmile · 2017-08-07T05:06:42Z

ok to test

SparkQA · 2017-08-07T05:09:00Z

Test build #80318 has finished for PR 18810 at commit 1b0ac5e.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

gatorsmile · 2017-08-07T05:41:20Z

sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegenExec.scala

+    val existLongFunction = ctx.existTooLongFunction
+    if (existLongFunction) {
+      logWarning(s"Function is too long, Whole-stage codegen disabled for this plan:\n "
+        + s"$treeString")


This could be very big. Please follow what did in #18658

@gatorsmile , thank you for review, the treeString not contains the code, it only contains the tree string of the Physical plan like below:
*HashAggregate(keys=[k1#2395L, k2#2396, k3#2397], functions=[partial_sum(id#2392L)...
+- *Project [id#2392L, (id#2392L & 1023) AS k1#2395L, cast((id#2392L & 1023) as double) AS k2#2396...
+- *Range (0, 655360, step=1, splits=1)
So, I think it will not be very big.

gatorsmile · 2017-08-07T05:44:21Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala

@@ -356,6 +356,16 @@ class CodegenContext {
  private val placeHolderToComments = new mutable.HashMap[String, String]

  /**
+   * Returns the length of codegen function  is too long or not
+   */
+  def existTooLongFunction(): Boolean = {


Adding a checking logics here, instead of returning Boolean?

Ok, I have modified it, thanks.

SparkQA · 2017-08-07T07:04:51Z

Test build #80325 has finished for PR 18810 at commit 7e84753.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2017-08-07T07:05:57Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala

@@ -356,6 +356,18 @@ class CodegenContext {
  private val placeHolderToComments = new mutable.HashMap[String, String]

  /**
+   * Returns the length of codegen function  is too long or not


nit: Returns if the length ....

nit: Please remove extra space before is too long.

Ok, I have modified it, thanks.

viirya · 2017-08-07T07:16:29Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

+  val WHOLESTAGE_MAX_FUNCTION_LEN = buildConf("spark.sql.codegen.MaxFunctionLength")
+    .internal()
+    .doc("The maximum number of function length that will be supported before" +
+    " deactivating whole-stage codegen.")


nit: add indent here.

viirya · 2017-08-07T07:21:37Z

sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegenExec.scala

@@ -370,6 +370,12 @@ case class WholeStageCodegenExec(child: SparkPlan) extends UnaryExecNode with Co

  override def doExecute(): RDD[InternalRow] = {
    val (ctx, cleanedSource) = doCodeGen()
+    val existLongFunction = ctx.existTooLongFunction
+    if (existLongFunction) {
+      logWarning(s"Function is too long, Whole-stage codegen disabled for this plan:\n "


Simply explain why Whole-stage codegen is disabled. Found too long generated codes and JIT optimization might not work. Whole-stage codegen disabled for this plan...

Btw, we can also add log message like You can change the config spark.sql.codegen.MaxFunctionLength to adjust the function length limit.

In case users wants to run whole-stage codegen intentionally.

SparkQA · 2017-08-07T09:57:00Z

Test build #80326 has finished for PR 18810 at commit c4235dc.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-08-07T10:36:31Z

Test build #80329 has finished for PR 18810 at commit 52da6b2.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

eatoncys · 2017-08-08T08:23:58Z

cc @gatorsmile

gatorsmile · 2017-08-09T07:12:43Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

@@ -572,6 +572,13 @@ object SQLConf {
      "disable logging or -1 to apply no limit.")
    .createWithDefault(1000)

+  val WHOLESTAGE_MAX_FUNCTION_LEN = buildConf("spark.sql.codegen.MaxFunctionLength")


MaxFunctionLength -> maxLinesPerFunction

Ok, I have modified it, thanks

gatorsmile · 2017-08-09T07:15:24Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala

@@ -356,6 +356,18 @@ class CodegenContext {
  private val placeHolderToComments = new mutable.HashMap[String, String]

  /**
+   * Returns if the length of codegen function is too long or not


length is misleading. Here, it is just number of lines.

Ok, I have modified it, thanks

gatorsmile · 2017-08-09T07:16:32Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala

+  def existTooLongFunction(): Boolean = {
+    classFunctions.exists { case (className, functions) =>
+      functions.exists{ case (name, code) =>
+        CodeFormatter.stripExtraNewLines(code).count(_ == '\n') > SQLConf.get.maxFunctionLength


Like what @viirya said, could you need to check whether it excludes the comments?

Ok, I have modified it to count lines without comments and extra new lines

SparkQA · 2017-08-10T03:49:10Z

Test build #80470 has finished for PR 18810 at commit d0c753a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

gatorsmile · 2017-08-10T04:04:58Z

sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegenExec.scala

+      logWarning(s"Found too long generated codes and JIT optimization might not work, " +
+        s"Whole-stage codegen disabled for this plan, " +
+        s"You can change the config spark.sql.codegen.MaxFunctionLength " +
+        s"to adjust the function length limit:\n "


Please remove the useless s

gatorsmile · 2017-08-16T16:12:04Z

Thanks! Merging to master.

maropu · 2017-08-20T23:53:32Z

(copied from jira for just-in-case) Just for your information, I checked the performance changes of TPCDS before/after the pr #18810; the pr affected Q17/Q66 only (that is, they have too long codegen'd functions). The changes are as follows (just run TPCDSQueryBenchmark);
Q17: https://github.com/apache/spark/blob/master/sql/core/src/test/resources/tpcds/q17.sql
Q66: https://github.com/apache/spark/blob/master/sql/core/src/test/resources/tpcds/q66.sql

Q17 w/o this pr, 3224.0  --> q17 w/this pr, 2627.0 (perf. improvement)
Q66 w/o this pr, 1712.0 -->  q66 w/this pr, 3032.0 (perf. regression)

It seems their queries have gen'd funcs with 2800~2900 lines, so if we set 2900 at spark.sql.codegen.maxLinesPerFunction, we could keep the previous performance w/o pr18810.

viirya · 2017-08-21T02:30:00Z

@maropu Interesting. Would you like to benchmark with #18931 too? It is my attempt to solve long code-gen functions without disabling it.

maropu · 2017-08-21T02:39:02Z

yea, I'll do

maropu · 2017-08-21T02:48:32Z

Btw, as for merged prs, I'm just monitoring TPCDS perf. in here. Also, I wrote a script before to run TPCDS on pending prs: https://github.com/maropu/spark-tpcds-datagen#helper-scripts-for-benchmarks.

gatorsmile · 2017-08-21T17:32:52Z

Thank you for tracking it! Could you adjust the conf to a higher number (e.g. 4097) and rerun the perf?

maropu · 2017-08-22T00:50:49Z

ok, I'll make a pr as follow-up.

…Function into 4000 ## What changes were proposed in this pull request? This pr changed the default value of `maxLinesPerFunction` into `4000`. In #18810, we had this new option to disable code generation for too long functions and I found this option only affected `Q17` and `Q66` in TPC-DS. But, `Q66` had some performance regression: ``` Q17 w/o #18810, 3224ms --> q17 w/#18810, 2627ms (improvement) Q66 w/o #18810, 1712ms --> q66 w/#18810, 3032ms (regression) ``` To keep the previous performance in TPC-DS, we better set higher value at `maxLinesPerFunction` by default. ## How was this patch tested? Existing tests. Author: Takeshi Yamamuro <[email protected]> Closes #19021 from maropu/SPARK-21603-FOLLOWUP-1.

…d code ## What changes were proposed in this pull request? This pr added code to check actual bytecode size when compiling generated code. In #18810, we added code to give up code compilation and use interpreter execution in `SparkPlan` if the line number of generated functions goes over `maxLinesPerFunction`. But, we already have code to collect metrics for compiled bytecode size in `CodeGenerator` object. So,we could easily reuse the code for this purpose. ## How was this patch tested? Added tests in `WholeStageCodegenSuite`. Author: Takeshi Yamamuro <[email protected]> Closes #19083 from maropu/SPARK-21871.

…d code This pr added code to check actual bytecode size when compiling generated code. In apache#18810, we added code to give up code compilation and use interpreter execution in `SparkPlan` if the line number of generated functions goes over `maxLinesPerFunction`. But, we already have code to collect metrics for compiled bytecode size in `CodeGenerator` object. So,we could easily reuse the code for this purpose. Added tests in `WholeStageCodegenSuite`. Author: Takeshi Yamamuro <[email protected]> Closes apache#19083 from maropu/SPARK-21871.

## What changes were proposed in this pull request? This pr proposed to split aggregation code into small functions in `HashAggregateExec`. In #18810, we got performance regression if JVMs didn't compile too long functions. I checked and I found the codegen of `HashAggregateExec` frequently goes over the limit when a query has too many aggregate functions (e.g., q66 in TPCDS). The current master places all the generated aggregation code in a single function. In this pr, I modified the code to assign an individual function for each aggregate function (e.g., `SUM` and `AVG`). For example, in a query `SELECT SUM(a), AVG(a) FROM VALUES(1) t(a)`, the proposed code defines two functions for `SUM(a)` and `AVG(a)` as follows; - generated code with this pr (https://gist.github.com/maropu/812990012bc967a78364be0fa793f559): ``` /* 173 */ private void agg_doConsume_0(InternalRow inputadapter_row_0, long agg_expr_0_0, boolean agg_exprIsNull_0_0, double agg_expr_1_0, boolean agg_exprIsNull_1_0, long agg_expr_2_0, boolean agg_exprIsNull_2_0) throws java.io.IOException { /* 174 */ // do aggregate /* 175 */ // common sub-expressions /* 176 */ /* 177 */ // evaluate aggregate functions and update aggregation buffers /* 178 */ agg_doAggregate_sum_0(agg_exprIsNull_0_0, agg_expr_0_0); /* 179 */ agg_doAggregate_avg_0(agg_expr_1_0, agg_exprIsNull_1_0, agg_exprIsNull_2_0, agg_expr_2_0); /* 180 */ /* 181 */ } ... /* 071 */ private void agg_doAggregate_avg_0(double agg_expr_1_0, boolean agg_exprIsNull_1_0, boolean agg_exprIsNull_2_0, long agg_expr_2_0) throws java.io.IOException { /* 072 */ // do aggregate for avg /* 073 */ // evaluate aggregate function /* 074 */ boolean agg_isNull_19 = true; /* 075 */ double agg_value_19 = -1.0; ... /* 114 */ private void agg_doAggregate_sum_0(boolean agg_exprIsNull_0_0, long agg_expr_0_0) throws java.io.IOException { /* 115 */ // do aggregate for sum /* 116 */ // evaluate aggregate function /* 117 */ agg_agg_isNull_11_0 = true; /* 118 */ long agg_value_11 = -1L; ``` - generated code in the current master (https://gist.github.com/maropu/e9d772af2c98d8991a6a5f0af7841760) ``` /* 059 */ private void agg_doConsume_0(InternalRow localtablescan_row_0, int agg_expr_0_0) throws java.io.IOException { /* 060 */ // do aggregate /* 061 */ // common sub-expressions /* 062 */ boolean agg_isNull_4 = false; /* 063 */ long agg_value_4 = -1L; /* 064 */ if (!false) { /* 065 */ agg_value_4 = (long) agg_expr_0_0; /* 066 */ } /* 067 */ // evaluate aggregate function /* 068 */ agg_agg_isNull_7_0 = true; /* 069 */ long agg_value_7 = -1L; /* 070 */ do { /* 071 */ if (!agg_bufIsNull_0) { /* 072 */ agg_agg_isNull_7_0 = false; /* 073 */ agg_value_7 = agg_bufValue_0; /* 074 */ continue; /* 075 */ } /* 076 */ /* 077 */ boolean agg_isNull_9 = false; /* 078 */ long agg_value_9 = -1L; /* 079 */ if (!false) { /* 080 */ agg_value_9 = (long) 0; /* 081 */ } /* 082 */ if (!agg_isNull_9) { /* 083 */ agg_agg_isNull_7_0 = false; /* 084 */ agg_value_7 = agg_value_9; /* 085 */ continue; /* 086 */ } /* 087 */ /* 088 */ } while (false); /* 089 */ /* 090 */ long agg_value_6 = -1L; /* 091 */ /* 092 */ agg_value_6 = agg_value_7 + agg_value_4; /* 093 */ boolean agg_isNull_11 = true; /* 094 */ double agg_value_11 = -1.0; /* 095 */ /* 096 */ if (!agg_bufIsNull_1) { /* 097 */ agg_agg_isNull_13_0 = true; /* 098 */ double agg_value_13 = -1.0; /* 099 */ do { /* 100 */ boolean agg_isNull_14 = agg_isNull_4; /* 101 */ double agg_value_14 = -1.0; /* 102 */ if (!agg_isNull_4) { /* 103 */ agg_value_14 = (double) agg_value_4; /* 104 */ } /* 105 */ if (!agg_isNull_14) { /* 106 */ agg_agg_isNull_13_0 = false; /* 107 */ agg_value_13 = agg_value_14; /* 108 */ continue; /* 109 */ } /* 110 */ /* 111 */ boolean agg_isNull_15 = false; /* 112 */ double agg_value_15 = -1.0; /* 113 */ if (!false) { /* 114 */ agg_value_15 = (double) 0; /* 115 */ } /* 116 */ if (!agg_isNull_15) { /* 117 */ agg_agg_isNull_13_0 = false; /* 118 */ agg_value_13 = agg_value_15; /* 119 */ continue; /* 120 */ } /* 121 */ /* 122 */ } while (false); /* 123 */ /* 124 */ agg_isNull_11 = false; // resultCode could change nullability. /* 125 */ /* 126 */ agg_value_11 = agg_bufValue_1 + agg_value_13; /* 127 */ /* 128 */ } /* 129 */ boolean agg_isNull_17 = false; /* 130 */ long agg_value_17 = -1L; /* 131 */ if (!false && agg_isNull_4) { /* 132 */ agg_isNull_17 = agg_bufIsNull_2; /* 133 */ agg_value_17 = agg_bufValue_2; /* 134 */ } else { /* 135 */ boolean agg_isNull_20 = true; /* 136 */ long agg_value_20 = -1L; /* 137 */ /* 138 */ if (!agg_bufIsNull_2) { /* 139 */ agg_isNull_20 = false; // resultCode could change nullability. /* 140 */ /* 141 */ agg_value_20 = agg_bufValue_2 + 1L; /* 142 */ /* 143 */ } /* 144 */ agg_isNull_17 = agg_isNull_20; /* 145 */ agg_value_17 = agg_value_20; /* 146 */ } /* 147 */ // update aggregation buffer /* 148 */ agg_bufIsNull_0 = false; /* 149 */ agg_bufValue_0 = agg_value_6; /* 150 */ /* 151 */ agg_bufIsNull_1 = agg_isNull_11; /* 152 */ agg_bufValue_1 = agg_value_11; /* 153 */ /* 154 */ agg_bufIsNull_2 = agg_isNull_17; /* 155 */ agg_bufValue_2 = agg_value_17; /* 156 */ /* 157 */ } ``` You can check the previous discussion in #19082 ## How was this patch tested? Existing tests Closes #20965 from maropu/SPARK-21870-2. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

## What changes were proposed in this pull request? This pr proposed to split aggregation code into small functions in `HashAggregateExec`. In apache#18810, we got performance regression if JVMs didn't compile too long functions. I checked and I found the codegen of `HashAggregateExec` frequently goes over the limit when a query has too many aggregate functions (e.g., q66 in TPCDS). The current master places all the generated aggregation code in a single function. In this pr, I modified the code to assign an individual function for each aggregate function (e.g., `SUM` and `AVG`). For example, in a query `SELECT SUM(a), AVG(a) FROM VALUES(1) t(a)`, the proposed code defines two functions for `SUM(a)` and `AVG(a)` as follows; - generated code with this pr (https://gist.github.com/maropu/812990012bc967a78364be0fa793f559): ``` /* 173 */ private void agg_doConsume_0(InternalRow inputadapter_row_0, long agg_expr_0_0, boolean agg_exprIsNull_0_0, double agg_expr_1_0, boolean agg_exprIsNull_1_0, long agg_expr_2_0, boolean agg_exprIsNull_2_0) throws java.io.IOException { /* 174 */ // do aggregate /* 175 */ // common sub-expressions /* 176 */ /* 177 */ // evaluate aggregate functions and update aggregation buffers /* 178 */ agg_doAggregate_sum_0(agg_exprIsNull_0_0, agg_expr_0_0); /* 179 */ agg_doAggregate_avg_0(agg_expr_1_0, agg_exprIsNull_1_0, agg_exprIsNull_2_0, agg_expr_2_0); /* 180 */ /* 181 */ } ... /* 071 */ private void agg_doAggregate_avg_0(double agg_expr_1_0, boolean agg_exprIsNull_1_0, boolean agg_exprIsNull_2_0, long agg_expr_2_0) throws java.io.IOException { /* 072 */ // do aggregate for avg /* 073 */ // evaluate aggregate function /* 074 */ boolean agg_isNull_19 = true; /* 075 */ double agg_value_19 = -1.0; ... /* 114 */ private void agg_doAggregate_sum_0(boolean agg_exprIsNull_0_0, long agg_expr_0_0) throws java.io.IOException { /* 115 */ // do aggregate for sum /* 116 */ // evaluate aggregate function /* 117 */ agg_agg_isNull_11_0 = true; /* 118 */ long agg_value_11 = -1L; ``` - generated code in the current master (https://gist.github.com/maropu/e9d772af2c98d8991a6a5f0af7841760) ``` /* 059 */ private void agg_doConsume_0(InternalRow localtablescan_row_0, int agg_expr_0_0) throws java.io.IOException { /* 060 */ // do aggregate /* 061 */ // common sub-expressions /* 062 */ boolean agg_isNull_4 = false; /* 063 */ long agg_value_4 = -1L; /* 064 */ if (!false) { /* 065 */ agg_value_4 = (long) agg_expr_0_0; /* 066 */ } /* 067 */ // evaluate aggregate function /* 068 */ agg_agg_isNull_7_0 = true; /* 069 */ long agg_value_7 = -1L; /* 070 */ do { /* 071 */ if (!agg_bufIsNull_0) { /* 072 */ agg_agg_isNull_7_0 = false; /* 073 */ agg_value_7 = agg_bufValue_0; /* 074 */ continue; /* 075 */ } /* 076 */ /* 077 */ boolean agg_isNull_9 = false; /* 078 */ long agg_value_9 = -1L; /* 079 */ if (!false) { /* 080 */ agg_value_9 = (long) 0; /* 081 */ } /* 082 */ if (!agg_isNull_9) { /* 083 */ agg_agg_isNull_7_0 = false; /* 084 */ agg_value_7 = agg_value_9; /* 085 */ continue; /* 086 */ } /* 087 */ /* 088 */ } while (false); /* 089 */ /* 090 */ long agg_value_6 = -1L; /* 091 */ /* 092 */ agg_value_6 = agg_value_7 + agg_value_4; /* 093 */ boolean agg_isNull_11 = true; /* 094 */ double agg_value_11 = -1.0; /* 095 */ /* 096 */ if (!agg_bufIsNull_1) { /* 097 */ agg_agg_isNull_13_0 = true; /* 098 */ double agg_value_13 = -1.0; /* 099 */ do { /* 100 */ boolean agg_isNull_14 = agg_isNull_4; /* 101 */ double agg_value_14 = -1.0; /* 102 */ if (!agg_isNull_4) { /* 103 */ agg_value_14 = (double) agg_value_4; /* 104 */ } /* 105 */ if (!agg_isNull_14) { /* 106 */ agg_agg_isNull_13_0 = false; /* 107 */ agg_value_13 = agg_value_14; /* 108 */ continue; /* 109 */ } /* 110 */ /* 111 */ boolean agg_isNull_15 = false; /* 112 */ double agg_value_15 = -1.0; /* 113 */ if (!false) { /* 114 */ agg_value_15 = (double) 0; /* 115 */ } /* 116 */ if (!agg_isNull_15) { /* 117 */ agg_agg_isNull_13_0 = false; /* 118 */ agg_value_13 = agg_value_15; /* 119 */ continue; /* 120 */ } /* 121 */ /* 122 */ } while (false); /* 123 */ /* 124 */ agg_isNull_11 = false; // resultCode could change nullability. /* 125 */ /* 126 */ agg_value_11 = agg_bufValue_1 + agg_value_13; /* 127 */ /* 128 */ } /* 129 */ boolean agg_isNull_17 = false; /* 130 */ long agg_value_17 = -1L; /* 131 */ if (!false && agg_isNull_4) { /* 132 */ agg_isNull_17 = agg_bufIsNull_2; /* 133 */ agg_value_17 = agg_bufValue_2; /* 134 */ } else { /* 135 */ boolean agg_isNull_20 = true; /* 136 */ long agg_value_20 = -1L; /* 137 */ /* 138 */ if (!agg_bufIsNull_2) { /* 139 */ agg_isNull_20 = false; // resultCode could change nullability. /* 140 */ /* 141 */ agg_value_20 = agg_bufValue_2 + 1L; /* 142 */ /* 143 */ } /* 144 */ agg_isNull_17 = agg_isNull_20; /* 145 */ agg_value_17 = agg_value_20; /* 146 */ } /* 147 */ // update aggregation buffer /* 148 */ agg_bufIsNull_0 = false; /* 149 */ agg_bufValue_0 = agg_value_6; /* 150 */ /* 151 */ agg_bufIsNull_1 = agg_isNull_11; /* 152 */ agg_bufValue_1 = agg_value_11; /* 153 */ /* 154 */ agg_bufIsNull_2 = agg_isNull_17; /* 155 */ agg_bufValue_2 = agg_value_17; /* 156 */ /* 157 */ } ``` You can check the previous discussion in apache#19082 ## How was this patch tested? Existing tests Closes apache#20965 from maropu/SPARK-21870-2. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

eatoncys added 2 commits August 2, 2017 11:48

The wholestage codegen will be slower when the function is too long

ca9eff6

The wholestage codegen will be slower when the function is too long

1b0ac5e

eatoncys changed the title ~~[SPARK-21603][sql]The wholestage codegen will be much slower then wholestage codegen is closed when the function is too long~~ [SPARK-21603][sql]The wholestage codegen will be much slower then that is closed when the function is too long Aug 2, 2017

kiszk reviewed Aug 3, 2017

View reviewed changes

gatorsmile reviewed Aug 7, 2017

View reviewed changes

eatoncys added 3 commits August 7, 2017 14:49

Modify annotation of existTooLongFunction

5582f00

Modify annotation of existTooLongFunction

7c185c6

Modify annotation of existTooLongFunction

7e84753

viirya reviewed Aug 7, 2017

View reviewed changes

Modify annotation of existTooLongFunction

c4235dc

viirya reviewed Aug 7, 2017

View reviewed changes

Modify annotation of existTooLongFunction

52da6b2

gatorsmile reviewed Aug 9, 2017

View reviewed changes

eatoncys added 3 commits August 10, 2017 09:00

count lines of function without comments

d0c753a

Add benchmark with default value

d3238e9

Add benchmark with default value

d44a2f8

gatorsmile reviewed Aug 10, 2017

View reviewed changes

asfgit closed this in 1cce1a3 Aug 16, 2017

maropu mentioned this pull request Aug 22, 2017

[SPARK-21603][SQL][FOLLOW-UP] Change the default value of maxLinesPerFunction into 4000 #19021

Closed

This was referenced Aug 24, 2017

[SPARK-21603][SQL][FOLLOW-UP] Use -1 to disable maxLinesPerFunction #19031

Closed

[SPARK-21870][SQL] Split aggregation code into small functions #19082

Closed

[SPARK-21871][SQL] Check actual bytecode size when compiling generated code #19083

Closed

juliuszsompolski mentioned this pull request Sep 22, 2017

[SPARK-22103] Move HashAggregateExec parent consume to a separate function in codegen #19324

Closed

maropu mentioned this pull request Oct 7, 2017

[SPARK-22215][SQL] Add configuration to set the threshold for generated class #19447

Closed

dongjoon-hyun mentioned this pull request Jan 30, 2018

[SPARK-23267] [SQL] Increase spark.sql.codegen.hugeMethodLimit to 65535 #20434

Closed

maropu mentioned this pull request Apr 3, 2018

[SPARK-21870][SQL] Split aggregation code into small functions #20965

Closed

[SPARK-21603][SQL]The wholestage codegen will be much slower then that is closed when the function is too long #18810

[SPARK-21603][SQL]The wholestage codegen will be much slower then that is closed when the function is too long #18810

Conversation

eatoncys commented Aug 2, 2017

What changes were proposed in this pull request?

How was this patch tested?

Choose a reason for hiding this comment

eatoncys Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

kiszk Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

viirya Aug 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gatorsmile commented Aug 7, 2017

SparkQA commented Aug 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Aug 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Aug 7, 2017

SparkQA commented Aug 7, 2017

eatoncys commented Aug 8, 2017

gatorsmile Aug 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Aug 10, 2017

Choose a reason for hiding this comment

gatorsmile commented Aug 16, 2017

maropu commented Aug 20, 2017

viirya commented Aug 21, 2017

maropu commented Aug 21, 2017

maropu commented Aug 21, 2017

gatorsmile commented Aug 21, 2017 • edited Loading

maropu commented Aug 22, 2017

eatoncys Aug 4, 2017 •

edited

Loading

kiszk Aug 4, 2017 •

edited

Loading

viirya Aug 7, 2017 •

edited

Loading

gatorsmile Aug 9, 2017 •

edited

Loading

gatorsmile commented Aug 21, 2017 •

edited

Loading