You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by we...@apache.org on 2017/11/12 21:44:52 UTC
spark git commit: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit
problem with AND or OR
Repository: spark
Updated Branches:
refs/heads/master b3f9dbf48 -> 9bf696dbe
[SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem with AND or OR
## What changes were proposed in this pull request?
This PR changes `AND` or `OR` code generation to place condition and then expressions' generated code into separated methods if these size could be large. When the method is newly generated, variables for `isNull` and `value` are declared as an instance variable to pass these values (e.g. `isNull1409` and `value1409`) to the callers of the generated method.
This PR resolved two cases:
* large code size of left expression
* large code size of right expression
## How was this patch tested?
Added a new test case into `CodeGenerationSuite`
Author: Kazuaki Ishizaki <is...@jp.ibm.com>
Closes #18972 from kiszk/SPARK-21720.
Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/9bf696db
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/9bf696db
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/9bf696db
Branch: refs/heads/master
Commit: 9bf696dbece6b1993880efba24a6d32c54c4d11c
Parents: b3f9dbf
Author: Kazuaki Ishizaki <is...@jp.ibm.com>
Authored: Sun Nov 12 22:44:47 2017 +0100
Committer: Wenchen Fan <we...@databricks.com>
Committed: Sun Nov 12 22:44:47 2017 +0100
----------------------------------------------------------------------
.../expressions/codegen/CodeGenerator.scala | 30 +++++++
.../expressions/conditionalExpressions.scala | 29 +------
.../sql/catalyst/expressions/predicates.scala | 82 +++++++++++++++++++-
.../expressions/CodeGenerationSuite.scala | 39 ++++++++++
.../org/apache/spark/sql/DataFrameSuite.scala | 6 +-
5 files changed, 157 insertions(+), 29 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/spark/blob/9bf696db/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
----------------------------------------------------------------------
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 98eda2a..3dc3f8e 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
@@ -909,6 +909,36 @@ class CodegenContext {
}
/**
+ * Wrap the generated code of expression, which was created from a row object in INPUT_ROW,
+ * by a function. ev.isNull and ev.value are passed by global variables
+ *
+ * @param ev the code to evaluate expressions.
+ * @param dataType the data type of ev.value.
+ * @param baseFuncName the split function name base.
+ */
+ def createAndAddFunction(
+ ev: ExprCode,
+ dataType: DataType,
+ baseFuncName: String): (String, String, String) = {
+ val globalIsNull = freshName("isNull")
+ addMutableState("boolean", globalIsNull, s"$globalIsNull = false;")
+ val globalValue = freshName("value")
+ addMutableState(javaType(dataType), globalValue,
+ s"$globalValue = ${defaultValue(dataType)};")
+ val funcName = freshName(baseFuncName)
+ val funcBody =
+ s"""
+ |private void $funcName(InternalRow ${INPUT_ROW}) {
+ | ${ev.code.trim}
+ | $globalIsNull = ${ev.isNull};
+ | $globalValue = ${ev.value};
+ |}
+ """.stripMargin
+ val fullFuncName = addNewFunction(funcName, funcBody)
+ (fullFuncName, globalIsNull, globalValue)
+ }
+
+ /**
* Perform a function which generates a sequence of ExprCodes with a given mapping between
* expressions and common expressions, instead of using the mapping in current context.
*/
http://git-wip-us.apache.org/repos/asf/spark/blob/9bf696db/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
index d95b59d..c41a10c 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala
@@ -72,11 +72,11 @@ case class If(predicate: Expression, trueValue: Expression, falseValue: Expressi
(ctx.INPUT_ROW != null && ctx.currentVars == null)) {
val (condFuncName, condGlobalIsNull, condGlobalValue) =
- createAndAddFunction(ctx, condEval, predicate.dataType, "evalIfCondExpr")
+ ctx.createAndAddFunction(condEval, predicate.dataType, "evalIfCondExpr")
val (trueFuncName, trueGlobalIsNull, trueGlobalValue) =
- createAndAddFunction(ctx, trueEval, trueValue.dataType, "evalIfTrueExpr")
+ ctx.createAndAddFunction(trueEval, trueValue.dataType, "evalIfTrueExpr")
val (falseFuncName, falseGlobalIsNull, falseGlobalValue) =
- createAndAddFunction(ctx, falseEval, falseValue.dataType, "evalIfFalseExpr")
+ ctx.createAndAddFunction(falseEval, falseValue.dataType, "evalIfFalseExpr")
s"""
$condFuncName(${ctx.INPUT_ROW});
boolean ${ev.isNull} = false;
@@ -112,29 +112,6 @@ case class If(predicate: Expression, trueValue: Expression, falseValue: Expressi
ev.copy(code = generatedCode)
}
- private def createAndAddFunction(
- ctx: CodegenContext,
- ev: ExprCode,
- dataType: DataType,
- baseFuncName: String): (String, String, String) = {
- val globalIsNull = ctx.freshName("isNull")
- ctx.addMutableState("boolean", globalIsNull, s"$globalIsNull = false;")
- val globalValue = ctx.freshName("value")
- ctx.addMutableState(ctx.javaType(dataType), globalValue,
- s"$globalValue = ${ctx.defaultValue(dataType)};")
- val funcName = ctx.freshName(baseFuncName)
- val funcBody =
- s"""
- |private void $funcName(InternalRow ${ctx.INPUT_ROW}) {
- | ${ev.code.trim}
- | $globalIsNull = ${ev.isNull};
- | $globalValue = ${ev.value};
- |}
- """.stripMargin
- val fullFuncName = ctx.addNewFunction(funcName, funcBody)
- (fullFuncName, globalIsNull, globalValue)
- }
-
override def toString: String = s"if ($predicate) $trueValue else $falseValue"
override def sql: String = s"(IF(${predicate.sql}, ${trueValue.sql}, ${falseValue.sql}))"
http://git-wip-us.apache.org/repos/asf/spark/blob/9bf696db/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
index efcd45f..61df5e0 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
@@ -368,7 +368,46 @@ case class And(left: Expression, right: Expression) extends BinaryOperator with
val eval2 = right.genCode(ctx)
// The result should be `false`, if any of them is `false` whenever the other is null or not.
- if (!left.nullable && !right.nullable) {
+
+ // place generated code of eval1 and eval2 in separate methods if their code combined is large
+ val combinedLength = eval1.code.length + eval2.code.length
+ if (combinedLength > 1024 &&
+ // Split these expressions only if they are created from a row object
+ (ctx.INPUT_ROW != null && ctx.currentVars == null)) {
+
+ val (eval1FuncName, eval1GlobalIsNull, eval1GlobalValue) =
+ ctx.createAndAddFunction(eval1, BooleanType, "eval1Expr")
+ val (eval2FuncName, eval2GlobalIsNull, eval2GlobalValue) =
+ ctx.createAndAddFunction(eval2, BooleanType, "eval2Expr")
+ if (!left.nullable && !right.nullable) {
+ val generatedCode = s"""
+ $eval1FuncName(${ctx.INPUT_ROW});
+ boolean ${ev.value} = false;
+ if (${eval1GlobalValue}) {
+ $eval2FuncName(${ctx.INPUT_ROW});
+ ${ev.value} = ${eval2GlobalValue};
+ }
+ """
+ ev.copy(code = generatedCode, isNull = "false")
+ } else {
+ val generatedCode = s"""
+ $eval1FuncName(${ctx.INPUT_ROW});
+ boolean ${ev.isNull} = false;
+ boolean ${ev.value} = false;
+ if (!${eval1GlobalIsNull} && !${eval1GlobalValue}) {
+ } else {
+ $eval2FuncName(${ctx.INPUT_ROW});
+ if (!${eval2GlobalIsNull} && !${eval2GlobalValue}) {
+ } else if (!${eval1GlobalIsNull} && !${eval2GlobalIsNull}) {
+ ${ev.value} = true;
+ } else {
+ ${ev.isNull} = true;
+ }
+ }
+ """
+ ev.copy(code = generatedCode)
+ }
+ } else if (!left.nullable && !right.nullable) {
ev.copy(code = s"""
${eval1.code}
boolean ${ev.value} = false;
@@ -431,7 +470,46 @@ case class Or(left: Expression, right: Expression) extends BinaryOperator with P
val eval2 = right.genCode(ctx)
// The result should be `true`, if any of them is `true` whenever the other is null or not.
- if (!left.nullable && !right.nullable) {
+
+ // place generated code of eval1 and eval2 in separate methods if their code combined is large
+ val combinedLength = eval1.code.length + eval2.code.length
+ if (combinedLength > 1024 &&
+ // Split these expressions only if they are created from a row object
+ (ctx.INPUT_ROW != null && ctx.currentVars == null)) {
+
+ val (eval1FuncName, eval1GlobalIsNull, eval1GlobalValue) =
+ ctx.createAndAddFunction(eval1, BooleanType, "eval1Expr")
+ val (eval2FuncName, eval2GlobalIsNull, eval2GlobalValue) =
+ ctx.createAndAddFunction(eval2, BooleanType, "eval2Expr")
+ if (!left.nullable && !right.nullable) {
+ val generatedCode = s"""
+ $eval1FuncName(${ctx.INPUT_ROW});
+ boolean ${ev.value} = true;
+ if (!${eval1GlobalValue}) {
+ $eval2FuncName(${ctx.INPUT_ROW});
+ ${ev.value} = ${eval2GlobalValue};
+ }
+ """
+ ev.copy(code = generatedCode, isNull = "false")
+ } else {
+ val generatedCode = s"""
+ $eval1FuncName(${ctx.INPUT_ROW});
+ boolean ${ev.isNull} = false;
+ boolean ${ev.value} = true;
+ if (!${eval1GlobalIsNull} && ${eval1GlobalValue}) {
+ } else {
+ $eval2FuncName(${ctx.INPUT_ROW});
+ if (!${eval2GlobalIsNull} && ${eval2GlobalValue}) {
+ } else if (!${eval1GlobalIsNull} && !${eval2GlobalIsNull}) {
+ ${ev.value} = false;
+ } else {
+ ${ev.isNull} = true;
+ }
+ }
+ """
+ ev.copy(code = generatedCode)
+ }
+ } else if (!left.nullable && !right.nullable) {
ev.isNull = "false"
ev.copy(code = s"""
${eval1.code}
http://git-wip-us.apache.org/repos/asf/spark/blob/9bf696db/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala
index 1e6f7b6..8f6289f 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala
@@ -341,4 +341,43 @@ class CodeGenerationSuite extends SparkFunSuite with ExpressionEvalHelper {
// should not throw exception
projection(row)
}
+
+ test("SPARK-21720: split large predications into blocks due to JVM code size limit") {
+ val length = 600
+
+ val input = new GenericInternalRow(length)
+ val utf8Str = UTF8String.fromString(s"abc")
+ for (i <- 0 until length) {
+ input.update(i, utf8Str)
+ }
+
+ var exprOr: Expression = Literal(false)
+ for (i <- 0 until length) {
+ exprOr = Or(EqualTo(BoundReference(i, StringType, true), Literal(s"c$i")), exprOr)
+ }
+
+ val planOr = GenerateMutableProjection.generate(Seq(exprOr))
+ val actualOr = planOr(input).toSeq(Seq(exprOr.dataType))
+ assert(actualOr.length == 1)
+ val expectedOr = false
+
+ if (!checkResult(actualOr.head, expectedOr, exprOr.dataType)) {
+ fail(s"Incorrect Evaluation: expressions: $exprOr, actual: $actualOr, expected: $expectedOr")
+ }
+
+ var exprAnd: Expression = Literal(true)
+ for (i <- 0 until length) {
+ exprAnd = And(EqualTo(BoundReference(i, StringType, true), Literal(s"c$i")), exprAnd)
+ }
+
+ val planAnd = GenerateMutableProjection.generate(Seq(exprAnd))
+ val actualAnd = planAnd(input).toSeq(Seq(exprAnd.dataType))
+ assert(actualAnd.length == 1)
+ val expectedAnd = false
+
+ if (!checkResult(actualAnd.head, expectedAnd, exprAnd.dataType)) {
+ fail(
+ s"Incorrect Evaluation: expressions: $exprAnd, actual: $actualAnd, expected: $expectedAnd")
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/spark/blob/9bf696db/sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala
----------------------------------------------------------------------
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala
index 31bfa77..644e72c 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala
@@ -2097,7 +2097,11 @@ class DataFrameSuite extends QueryTest with SharedSQLContext {
.count
}
- testQuietly("SPARK-19372: Filter can be executed w/o generated code due to JVM code size limit") {
+ // The fix of SPARK-21720 avoid an exception regarding JVM code size limit
+ // TODO: When we make a threshold of splitting statements (1024) configurable,
+ // we will re-enable this with max threshold to cause an exception
+ // See https://github.com/apache/spark/pull/18972/files#r150223463
+ ignore("SPARK-19372: Filter can be executed w/o generated code due to JVM code size limit") {
val N = 400
val rows = Seq(Row.fromSeq(Seq.fill(N)("string")))
val schema = StructType(Seq.tabulate(N)(i => StructField(s"_c$i", StringType)))
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org