Skip to content

Commit 0aedc47

Browse files
committed
Refactor ctx.splitExpressions().
1 parent 33b5fd8 commit 0aedc47

File tree

3 files changed

+37
-43
lines changed

3 files changed

+37
-43
lines changed

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -610,8 +610,13 @@ class CodegenContext {
610610
splitExpressions(expressions, "apply", ("InternalRow", row) :: Nil)
611611
}
612612

613-
private def splitExpressions(
614-
expressions: Seq[String], funcName: String, arguments: Seq[(String, String)]): String = {
613+
def splitExpressions(
614+
expressions: Seq[String],
615+
funcName: String,
616+
arguments: Seq[(String, String)],
617+
returnType: String = "void",
618+
makeSplitFunction: String => String = identity,
619+
foldFunctions: Seq[String] => String = _.mkString("", ";\n", ";")): String = {
615620
val blocks = new ArrayBuffer[String]()
616621
val blockBuilder = new StringBuilder()
617622
for (code <- expressions) {
@@ -632,18 +637,19 @@ class CodegenContext {
632637
blocks.head
633638
} else {
634639
val func = freshName(funcName)
640+
val argString = arguments.map { case (t, name) => s"$t $name" }.mkString(", ")
635641
val functions = blocks.zipWithIndex.map { case (body, i) =>
636642
val name = s"${func}_$i"
637643
val code = s"""
638-
|private void $name(${arguments.map { case (t, name) => s"$t $name" }.mkString(", ")}) {
639-
| $body
644+
|private $returnType $name($argString) {
645+
| ${makeSplitFunction(body)}
640646
|}
641647
""".stripMargin
642648
addNewFunction(name, code)
643649
name
644650
}
645651

646-
functions.map(name => s"$name(${arguments.map(_._2).mkString(", ")});").mkString("\n")
652+
foldFunctions(functions.map(name => s"$name(${arguments.map(_._2).mkString(", ")})"))
647653
}
648654
}
649655

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

Lines changed: 24 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
7272
* Generates the code for ordering based on the given order.
7373
*/
7474
def genComparisons(ctx: CodegenContext, ordering: Seq[SortOrder]): String = {
75-
def comparisons(orderingGroup: Seq[SortOrder]) = orderingGroup.map { order =>
75+
val comparisons = ordering.map { order =>
7676
val eval = order.child.genCode(ctx)
7777
val asc = order.isAscending
7878
val isNullA = ctx.freshName("isNullA")
@@ -117,43 +117,31 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
117117
}
118118
}
119119
"""
120-
}.mkString("\n")
121-
122-
/*
123-
* 40 = 7000 bytes / 170 (around 170 bytes per ordering comparison).
124-
* The maximum byte code size to be compiled for HotSpot is 8000 bytes.
125-
* We should keep less than 8000 bytes.
126-
*/
127-
val numberOfComparisonsThreshold = 40
128-
129-
if (ordering.size <= numberOfComparisonsThreshold) {
130-
comparisons(ordering)
131-
} else {
132-
val groupedOrderingItr = ordering.grouped(numberOfComparisonsThreshold)
133-
val funcNamePrefix = ctx.freshName("compare")
134-
val funcNames = groupedOrderingItr.zipWithIndex.map { case (orderingGroup, i) =>
135-
val funcName = s"${funcNamePrefix}_$i"
136-
val funcCode =
137-
s"""
138-
|private int $funcName(InternalRow a, InternalRow b) {
139-
| InternalRow ${ctx.INPUT_ROW} = null; // Holds current row being evaluated.
140-
| ${comparisons(orderingGroup)}
141-
| return 0;
142-
|}
143-
""".stripMargin
144-
ctx.addNewFunction(funcName, funcCode)
145-
funcName
146-
}
120+
}
147121

148-
funcNames.zipWithIndex.map { case (funcName, i) =>
122+
ctx.splitExpressions(
123+
expressions = comparisons,
124+
funcName = "compare",
125+
arguments = Seq(("InternalRow", "a"), ("InternalRow", "b")),
126+
returnType = "int",
127+
makeSplitFunction = { body =>
149128
s"""
150-
|int comp_$i = ${funcName}(a, b);
151-
|if (comp_$i != 0) {
152-
| return comp_$i;
153-
|}
154-
""".stripMargin
155-
}.mkString
156-
}
129+
InternalRow ${ctx.INPUT_ROW} = null; // Holds current row being evaluated.
130+
$body
131+
return 0;
132+
"""
133+
},
134+
foldFunctions = { funCalls =>
135+
val comp = ctx.freshName("comp")
136+
funCalls.zipWithIndex.map { case (funCall, i) =>
137+
s"""
138+
int ${comp}_$i = $funCall;
139+
if (${comp}_$i != 0) {
140+
return ${comp}_$i;
141+
}
142+
"""
143+
}.mkString
144+
})
157145
}
158146

159147
protected def create(ordering: Seq[SortOrder]): BaseOrdering = {

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/OrderingSuite.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ class OrderingSuite extends SparkFunSuite with ExpressionEvalHelper {
137137
// this is FAILING prior to SPARK-16845, but it should be passing after SPARK-16845
138138
GenerateOrdering.generate(Array.fill(450)(sortOrder))
139139

140-
// verify that we can support up to 10000 ordering comparisons, which should be sufficient
141-
GenerateOrdering.generate(Array.fill(10000)(sortOrder))
140+
// verify that we can support up to 5000 ordering comparisons, which should be sufficient
141+
GenerateOrdering.generate(Array.fill(5000)(sortOrder))
142142
}
143143
}

0 commit comments

Comments
 (0)