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/12/07 13:24:45 UTC
spark git commit: [SPARK-22696][SQL] objects functions should not use
unneeded global variables
Repository: spark
Updated Branches:
refs/heads/master fc2944630 -> b79071910
[SPARK-22696][SQL] objects functions should not use unneeded global variables
## What changes were proposed in this pull request?
Some objects functions are using global variables which are not needed. This can generate some unneeded entries in the constant pool.
The PR replaces the unneeded global variables with local variables.
## How was this patch tested?
added UTs
Author: Marco Gaido <mg...@hortonworks.com>
Author: Marco Gaido <ma...@gmail.com>
Closes #19908 from mgaido91/SPARK-22696.
Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/b7907191
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/b7907191
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/b7907191
Branch: refs/heads/master
Commit: b79071910e61a851931b8b96d4acb66e0576caa7
Parents: fc29446
Author: Marco Gaido <mg...@hortonworks.com>
Authored: Thu Dec 7 21:24:36 2017 +0800
Committer: Wenchen Fan <we...@databricks.com>
Committed: Thu Dec 7 21:24:36 2017 +0800
----------------------------------------------------------------------
.../catalyst/expressions/objects/objects.scala | 61 +++++++++++---------
.../expressions/CodeGenerationSuite.scala | 16 ++++-
2 files changed, 49 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/spark/blob/b7907191/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
----------------------------------------------------------------------
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
index 730b2ff..349afec 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
@@ -1106,27 +1106,31 @@ case class CreateExternalRow(children: Seq[Expression], schema: StructType)
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
val rowClass = classOf[GenericRowWithSchema].getName
val values = ctx.freshName("values")
- ctx.addMutableState("Object[]", values)
val childrenCodes = children.zipWithIndex.map { case (e, i) =>
val eval = e.genCode(ctx)
- eval.code + s"""
- if (${eval.isNull}) {
- $values[$i] = null;
- } else {
- $values[$i] = ${eval.value};
- }
- """
+ s"""
+ |${eval.code}
+ |if (${eval.isNull}) {
+ | $values[$i] = null;
+ |} else {
+ | $values[$i] = ${eval.value};
+ |}
+ """.stripMargin
}
- val childrenCode = ctx.splitExpressionsWithCurrentInputs(childrenCodes)
- val schemaField = ctx.addReferenceObj("schema", schema)
+ val childrenCode = ctx.splitExpressionsWithCurrentInputs(
+ expressions = childrenCodes,
+ funcName = "createExternalRow",
+ extraArguments = "Object[]" -> values :: Nil)
+ val schemaField = ctx.addReferenceMinorObj(schema)
- val code = s"""
- $values = new Object[${children.size}];
- $childrenCode
- final ${classOf[Row].getName} ${ev.value} = new $rowClass($values, $schemaField);
- """
+ val code =
+ s"""
+ |Object[] $values = new Object[${children.size}];
+ |$childrenCode
+ |final ${classOf[Row].getName} ${ev.value} = new $rowClass($values, $schemaField);
+ """.stripMargin
ev.copy(code = code, isNull = "false")
}
}
@@ -1244,25 +1248,28 @@ case class InitializeJavaBean(beanInstance: Expression, setters: Map[String, Exp
val javaBeanInstance = ctx.freshName("javaBean")
val beanInstanceJavaType = ctx.javaType(beanInstance.dataType)
- ctx.addMutableState(beanInstanceJavaType, javaBeanInstance)
val initialize = setters.map {
case (setterMethod, fieldValue) =>
val fieldGen = fieldValue.genCode(ctx)
s"""
- ${fieldGen.code}
- ${javaBeanInstance}.$setterMethod(${fieldGen.value});
- """
+ |${fieldGen.code}
+ |$javaBeanInstance.$setterMethod(${fieldGen.value});
+ """.stripMargin
}
- val initializeCode = ctx.splitExpressionsWithCurrentInputs(initialize.toSeq)
+ val initializeCode = ctx.splitExpressionsWithCurrentInputs(
+ expressions = initialize.toSeq,
+ funcName = "initializeJavaBean",
+ extraArguments = beanInstanceJavaType -> javaBeanInstance :: Nil)
- val code = s"""
- ${instanceGen.code}
- ${javaBeanInstance} = ${instanceGen.value};
- if (!${instanceGen.isNull}) {
- $initializeCode
- }
- """
+ val code =
+ s"""
+ |${instanceGen.code}
+ |$beanInstanceJavaType $javaBeanInstance = ${instanceGen.value};
+ |if (!${instanceGen.isNull}) {
+ | $initializeCode
+ |}
+ """.stripMargin
ev.copy(code = code, isNull = instanceGen.isNull, value = instanceGen.value)
}
}
http://git-wip-us.apache.org/repos/asf/spark/blob/b7907191/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 a4198f8..40bf29b 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
@@ -25,7 +25,7 @@ import org.apache.spark.sql.Row
import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.dsl.expressions._
import org.apache.spark.sql.catalyst.expressions.codegen._
-import org.apache.spark.sql.catalyst.expressions.objects.{AssertNotNull, CreateExternalRow, GetExternalRowField, ValidateExternalType}
+import org.apache.spark.sql.catalyst.expressions.objects._
import org.apache.spark.sql.catalyst.util.{ArrayBasedMapData, DateTimeUtils}
import org.apache.spark.sql.types._
import org.apache.spark.unsafe.types.UTF8String
@@ -380,4 +380,18 @@ class CodeGenerationSuite extends SparkFunSuite with ExpressionEvalHelper {
s"Incorrect Evaluation: expressions: $exprAnd, actual: $actualAnd, expected: $expectedAnd")
}
}
+
+ test("SPARK-22696: CreateExternalRow should not use global variables") {
+ val ctx = new CodegenContext
+ val schema = new StructType().add("a", IntegerType).add("b", StringType)
+ CreateExternalRow(Seq(Literal(1), Literal("x")), schema).genCode(ctx)
+ assert(ctx.mutableStates.isEmpty)
+ }
+
+ test("SPARK-22696: InitializeJavaBean should not use global variables") {
+ val ctx = new CodegenContext
+ InitializeJavaBean(Literal.fromObject(new java.util.LinkedList[Int]),
+ Map("add" -> Literal(1))).genCode(ctx)
+ assert(ctx.mutableStates.isEmpty)
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org