Skip to content

[SPARK-35384][SQL][FOLLOWUP] Move HashMap.get out of InvokeLike.invoke #32532

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ trait InvokeLike extends Expression with NonSQLExpression {

protected lazy val needNullCheck: Boolean = propagateNull && arguments.exists(_.nullable)
protected lazy val evaluatedArgs: Array[Object] = new Array[Object](arguments.length)
private lazy val boxingFn: Any => Any =
ScalaReflection.typeBoxedJavaMapping
.get(dataType)
.map(cls => v => cls.cast(v))
.getOrElse(identity)


/**
* Prepares codes for arguments.
Expand Down Expand Up @@ -122,12 +128,7 @@ trait InvokeLike extends Expression with NonSQLExpression {
* @param dataType the data type of the return object
* @return the return object of a method call
*/
def invoke(
obj: Any,
method: Method,
arguments: Seq[Expression],
input: InternalRow,
dataType: DataType): Any = {
def invoke(obj: Any, method: Method, input: InternalRow): Any = {
var i = 0
val len = arguments.length
while (i < len) {
Expand All @@ -145,12 +146,7 @@ trait InvokeLike extends Expression with NonSQLExpression {
case e: java.lang.reflect.InvocationTargetException if e.getCause != null =>
throw e.getCause
}
val boxedClass = ScalaReflection.typeBoxedJavaMapping.get(dataType)
if (boxedClass.isDefined) {
boxedClass.get.cast(ret)
} else {
ret
}
boxingFn(ret)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could speed up the performance a bit.

Are you going to attach the new benchmark result?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Yes I'll attach benchmark results for this later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm probably missing something dumb here but dataType is an argument to this method - how can we have one function for it? I don't see where dataType is available to the trait

Copy link
Member Author

@sunchao sunchao May 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit misleading by looking at it, but in the call sites of invoke we currently always pass Expression.dataType (which is overridden by Invoke and StaticInvoke) as its argument, so the parameter is actually unnecessary. The same goes to arguments. To avoid confusion I think we can just remove these two parameters.

}
}

Expand Down Expand Up @@ -256,7 +252,7 @@ case class StaticInvoke(
@transient lazy val method = findMethod(cls, functionName, argClasses)

override def eval(input: InternalRow): Any = {
invoke(null, method, arguments, input, dataType)
invoke(null, method, input)
}

override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
Expand Down Expand Up @@ -358,7 +354,7 @@ case class Invoke(
} else {
obj.getClass.getMethod(functionName, argClasses: _*)
}
invoke(obj, invokeMethod, arguments, input, dataType)
invoke(obj, invokeMethod, input)
}
}

Expand Down
64 changes: 32 additions & 32 deletions sql/core/benchmarks/V2FunctionBenchmark-jdk11-results.txt
Original file line number Diff line number Diff line change
@@ -1,44 +1,44 @@
OpenJDK 64-Bit Server VM 11.0.11+9-LTS on Linux 5.4.0-1046-azure
Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz
OpenJDK 64-Bit Server VM 11.0.11+9-LTS on Linux 5.4.0-1047-azure
Intel(R) Xeon(R) Platinum 8171M CPU @ 2.60GHz
scalar function (long + long) -> long, result_nullable = true codegen = true: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------------------------------------------
native_long_add 17138 17431 486 29.2 34.3 1.0X
java_long_add_default 47386 48316 1583 10.6 94.8 0.4X
java_long_add_magic 19409 19532 152 25.8 38.8 0.9X
java_long_add_static_magic 18257 18294 33 27.4 36.5 0.9X
scala_long_add_default 49259 49512 235 10.2 98.5 0.3X
scala_long_add_magic 18964 19025 53 26.4 37.9 0.9X
native_long_add 14079 14697 555 35.5 28.2 1.0X
java_long_add_default 36350 38220 1620 13.8 72.7 0.4X
java_long_add_magic 14910 15251 336 33.5 29.8 0.9X
java_long_add_static_magic 14863 14962 161 33.6 29.7 0.9X
scala_long_add_default 41715 42786 1040 12.0 83.4 0.3X
scala_long_add_magic 15712 15775 58 31.8 31.4 0.9X

OpenJDK 64-Bit Server VM 11.0.11+9-LTS on Linux 5.4.0-1046-azure
Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz
OpenJDK 64-Bit Server VM 11.0.11+9-LTS on Linux 5.4.0-1047-azure
Intel(R) Xeon(R) Platinum 8171M CPU @ 2.60GHz
scalar function (long + long) -> long, result_nullable = false codegen = true: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
-------------------------------------------------------------------------------------------------------------------------------------------------------------
native_long_add 16814 16916 99 29.7 33.6 1.0X
java_long_add_default 43725 43909 216 11.4 87.4 0.4X
java_long_add_magic 19015 19060 39 26.3 38.0 0.9X
java_long_add_static_magic 18940 18993 52 26.4 37.9 0.9X
scala_long_add_default 43804 43874 88 11.4 87.6 0.4X
scala_long_add_magic 18753 18791 34 26.7 37.5 0.9X
native_long_add 13959 14048 80 35.8 27.9 1.0X
java_long_add_default 40773 41318 580 12.3 81.5 0.3X
java_long_add_magic 15929 16145 205 31.4 31.9 0.9X
java_long_add_static_magic 13384 13948 496 37.4 26.8 1.0X
scala_long_add_default 37782 39099 1141 13.2 75.6 0.4X
scala_long_add_magic 14553 14982 372 34.4 29.1 1.0X

OpenJDK 64-Bit Server VM 11.0.11+9-LTS on Linux 5.4.0-1046-azure
Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz
OpenJDK 64-Bit Server VM 11.0.11+9-LTS on Linux 5.4.0-1047-azure
Intel(R) Xeon(R) Platinum 8171M CPU @ 2.60GHz
scalar function (long + long) -> long, result_nullable = true codegen = false: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
-------------------------------------------------------------------------------------------------------------------------------------------------------------
native_long_add 42493 42830 506 11.8 85.0 1.0X
java_long_add_default 54557 54710 141 9.2 109.1 0.8X
java_long_add_magic 74409 74564 227 6.7 148.8 0.6X
java_long_add_static_magic 75081 75235 190 6.7 150.2 0.6X
scala_long_add_default 54789 54862 77 9.1 109.6 0.8X
scala_long_add_magic 73777 73886 96 6.8 147.6 0.6X
native_long_add 31564 32912 1167 15.8 63.1 1.0X
java_long_add_default 45392 46662 1821 11.0 90.8 0.7X
java_long_add_magic 44650 45705 1230 11.2 89.3 0.7X
java_long_add_static_magic 46391 47033 573 10.8 92.8 0.7X
scala_long_add_default 42915 44688 1654 11.7 85.8 0.7X
scala_long_add_magic 45617 46073 644 11.0 91.2 0.7X

OpenJDK 64-Bit Server VM 11.0.11+9-LTS on Linux 5.4.0-1046-azure
Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz
OpenJDK 64-Bit Server VM 11.0.11+9-LTS on Linux 5.4.0-1047-azure
Intel(R) Xeon(R) Platinum 8171M CPU @ 2.60GHz
scalar function (long + long) -> long, result_nullable = false codegen = false: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
--------------------------------------------------------------------------------------------------------------------------------------------------------------
native_long_add 37357 37490 116 13.4 74.7 1.0X
java_long_add_default 53166 53192 23 9.4 106.3 0.7X
java_long_add_magic 70501 71258 1121 7.1 141.0 0.5X
java_long_add_static_magic 68934 69636 1115 7.3 137.9 0.5X
scala_long_add_default 53075 53146 62 9.4 106.2 0.7X
scala_long_add_magic 69838 70746 1442 7.2 139.7 0.5X
native_long_add 30192 30399 186 16.6 60.4 1.0X
java_long_add_default 41940 42698 679 11.9 83.9 0.7X
java_long_add_magic 45087 45760 628 11.1 90.2 0.7X
java_long_add_static_magic 44109 45979 1726 11.3 88.2 0.7X
scala_long_add_default 41676 42064 375 12.0 83.4 0.7X
scala_long_add_magic 44886 45825 858 11.1 89.8 0.7X

Loading