-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-35384][SQL] Improve performance for InvokeLike.invoke #32527
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,7 @@ trait InvokeLike extends Expression with NonSQLExpression { | |
| def propagateNull: Boolean | ||
|
|
||
| protected lazy val needNullCheck: Boolean = propagateNull && arguments.exists(_.nullable) | ||
| protected lazy val evaluatedArgs: Array[Object] = new Array[Object](arguments.length) | ||
|
|
||
| /** | ||
| * Prepares codes for arguments. | ||
|
|
@@ -127,13 +128,18 @@ trait InvokeLike extends Expression with NonSQLExpression { | |
| arguments: Seq[Expression], | ||
| input: InternalRow, | ||
| dataType: DataType): Any = { | ||
| val args = arguments.map(e => e.eval(input).asInstanceOf[Object]) | ||
| if (needNullCheck && args.exists(_ == null)) { | ||
| var i = 0 | ||
| val len = arguments.length | ||
| while (i < len) { | ||
| evaluatedArgs(i) = arguments(i).eval(input).asInstanceOf[Object] | ||
| i += 1 | ||
| } | ||
| if (needNullCheck && evaluatedArgs.contains(null)) { | ||
| // return null if one of arguments is null | ||
| null | ||
| } else { | ||
| val ret = try { | ||
| method.invoke(obj, args: _*) | ||
| method.invoke(obj, evaluatedArgs: _*) | ||
| } catch { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we also improve the last piece? We can create a function for it
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can do the similar thing in
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea let me try it. In the profiling after this PR,
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if we can do the similar thing in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right. Another idea:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm I'm not sure. Looking at usages of Looking at the profiling result for Although this is somewhat unrelated to the above as @transient lazy val method = targetObject.dataType match {
case ObjectType(cls) =>
Some(findMethod(cls, encodedFunctionName, argClasses))
case _ => None
}we may need new benchmarks if we decide to do this.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. makes sense, for UDF, it's just an extra |
||
| // Re-throw the original exception. | ||
| case e: java.lang.reflect.InvocationTargetException if e.getCause != null => | ||
|
|
||
| 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) Platinum 8171M CPU @ 2.60GHz | ||
| Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz | ||
| 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 16015 16309 407 31.2 32.0 1.0X | ||
| java_long_add_default 48899 49122 352 10.2 97.8 0.3X | ||
| java_long_add_magic 19169 19302 117 26.1 38.3 0.8X | ||
| java_long_add_static_magic 18308 18373 57 27.3 36.6 0.9X | ||
| scala_long_add_default 48773 48922 136 10.3 97.5 0.3X | ||
| scala_long_add_magic 18372 18422 44 27.2 36.7 0.9X | ||
| 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 | ||
|
|
||
| OpenJDK 64-Bit Server VM 11.0.11+9-LTS on Linux 5.4.0-1046-azure | ||
| Intel(R) Xeon(R) Platinum 8171M CPU @ 2.60GHz | ||
| Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz | ||
| 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 16414 16452 41 30.5 32.8 1.0X | ||
| java_long_add_default 47640 47767 134 10.5 95.3 0.3X | ||
| java_long_add_magic 18413 18554 139 27.2 36.8 0.9X | ||
| java_long_add_static_magic 16659 16707 43 30.0 33.3 1.0X | ||
| scala_long_add_default 47821 47857 48 10.5 95.6 0.3X | ||
| scala_long_add_magic 18406 18502 99 27.2 36.8 0.9X | ||
| 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 | ||
|
|
||
| OpenJDK 64-Bit Server VM 11.0.11+9-LTS on Linux 5.4.0-1046-azure | ||
| Intel(R) Xeon(R) Platinum 8171M CPU @ 2.60GHz | ||
| Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz | ||
| 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 36335 36366 27 13.8 72.7 1.0X | ||
| java_long_add_default 53930 54056 155 9.3 107.9 0.7X | ||
| java_long_add_magic 126621 127109 471 3.9 253.2 0.3X | ||
| java_long_add_static_magic 126914 127193 251 3.9 253.8 0.3X | ||
| scala_long_add_default 55812 55949 141 9.0 111.6 0.7X | ||
| scala_long_add_magic 127629 127900 420 3.9 255.3 0.3X | ||
| 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 | ||
|
|
||
| OpenJDK 64-Bit Server VM 11.0.11+9-LTS on Linux 5.4.0-1046-azure | ||
| Intel(R) Xeon(R) Platinum 8171M CPU @ 2.60GHz | ||
| Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz | ||
| 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 37433 37794 312 13.4 74.9 1.0X | ||
| java_long_add_default 53629 53946 416 9.3 107.3 0.7X | ||
| java_long_add_magic 160091 160605 549 3.1 320.2 0.2X | ||
| java_long_add_static_magic 157228 158430 1372 3.2 314.5 0.2X | ||
| scala_long_add_default 54026 54197 187 9.3 108.1 0.7X | ||
| scala_long_add_magic 160926 161351 526 3.1 321.9 0.2X | ||
| 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 | ||
|
|

Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need to keep
evaluatedArgsas a (lazy) val?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean just use val?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't we evaluate
argumentsfor each timeinvokeis called? Why not just havingval evaluatedArgs: Array[Object] = new Array[Object](arguments.length)ininvoke?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it aims to reuse
Array[Object]itself and only changes the values of array.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea even though we evaluate
argumentsfor eachinvokecall we can reuse the same array to store the results of evaluation. I guess it's better than allocating a newArray[Object]for each input row.