Skip to content

Commit 680b33f

Browse files
cloud-fangatorsmile
authored andcommitted
[SPARK-18016][SQL][FOLLOWUP] merge declareAddedFunctions, initNestedClasses and declareNestedClasses
## What changes were proposed in this pull request? These 3 methods have to be used together, so it makes more sense to merge them into one method and then the caller side only need to call one method. ## How was this patch tested? existing tests. Author: Wenchen Fan <[email protected]> Closes #18579 from cloud-fan/minor.
1 parent 08e0d03 commit 680b33f

File tree

8 files changed

+18
-46
lines changed

8 files changed

+18
-46
lines changed

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

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -302,37 +302,30 @@ class CodegenContext {
302302
}
303303

304304
/**
305-
* Instantiates all nested, private sub-classes as objects to the `OuterClass`
305+
* Declares all function code. If the added functions are too many, split them into nested
306+
* sub-classes to avoid hitting Java compiler constant pool limitation.
306307
*/
307-
private[sql] def initNestedClasses(): String = {
308+
def declareAddedFunctions(): String = {
309+
val inlinedFunctions = classFunctions(outerClassName).values
310+
308311
// Nested, private sub-classes have no mutable state (though they do reference the outer class'
309312
// mutable state), so we declare and initialize them inline to the OuterClass.
310-
classes.filter(_._1 != outerClassName).map {
313+
val initNestedClasses = classes.filter(_._1 != outerClassName).map {
311314
case (className, classInstance) =>
312315
s"private $className $classInstance = new $className();"
313-
}.mkString("\n")
314-
}
315-
316-
/**
317-
* Declares all function code that should be inlined to the `OuterClass`.
318-
*/
319-
private[sql] def declareAddedFunctions(): String = {
320-
classFunctions(outerClassName).values.mkString("\n")
321-
}
316+
}
322317

323-
/**
324-
* Declares all nested, private sub-classes and the function code that should be inlined to them.
325-
*/
326-
private[sql] def declareNestedClasses(): String = {
327-
classFunctions.filterKeys(_ != outerClassName).map {
318+
val declareNestedClasses = classFunctions.filterKeys(_ != outerClassName).map {
328319
case (className, functions) =>
329320
s"""
330321
|private class $className {
331322
| ${functions.values.mkString("\n")}
332323
|}
333324
""".stripMargin
334325
}
335-
}.mkString("\n")
326+
327+
(inlinedFunctions ++ initNestedClasses ++ declareNestedClasses).mkString("\n")
328+
}
336329

337330
final val JAVA_BOOLEAN = "boolean"
338331
final val JAVA_BYTE = "byte"

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,6 @@ object GenerateMutableProjection extends CodeGenerator[Seq[Expression], MutableP
115115
${ctx.initPartition()}
116116
}
117117

118-
${ctx.declareAddedFunctions()}
119-
120118
public ${classOf[BaseMutableProjection].getName} target(InternalRow row) {
121119
mutableRow = row;
122120
return this;
@@ -136,8 +134,7 @@ object GenerateMutableProjection extends CodeGenerator[Seq[Expression], MutableP
136134
return mutableRow;
137135
}
138136

139-
${ctx.initNestedClasses()}
140-
${ctx.declareNestedClasses()}
137+
${ctx.declareAddedFunctions()}
141138
}
142139
"""
143140

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,15 +173,12 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
173173
${ctx.initMutableStates()}
174174
}
175175

176-
${ctx.declareAddedFunctions()}
177-
178176
public int compare(InternalRow a, InternalRow b) {
179177
$comparisons
180178
return 0;
181179
}
182180

183-
${ctx.initNestedClasses()}
184-
${ctx.declareNestedClasses()}
181+
${ctx.declareAddedFunctions()}
185182
}"""
186183

187184
val code = CodeFormatter.stripOverlappingComments(

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,12 @@ object GeneratePredicate extends CodeGenerator[Expression, Predicate] {
6666
${ctx.initPartition()}
6767
}
6868

69-
${ctx.declareAddedFunctions()}
70-
7169
public boolean eval(InternalRow ${ctx.INPUT_ROW}) {
7270
${eval.code}
7371
return !${eval.isNull} && ${eval.value};
7472
}
7573

76-
${ctx.initNestedClasses()}
77-
${ctx.declareNestedClasses()}
74+
${ctx.declareAddedFunctions()}
7875
}"""
7976

8077
val code = CodeFormatter.stripOverlappingComments(

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,16 +175,13 @@ object GenerateSafeProjection extends CodeGenerator[Seq[Expression], Projection]
175175
${ctx.initPartition()}
176176
}
177177

178-
${ctx.declareAddedFunctions()}
179-
180178
public java.lang.Object apply(java.lang.Object _i) {
181179
InternalRow ${ctx.INPUT_ROW} = (InternalRow) _i;
182180
$allExpressions
183181
return mutableRow;
184182
}
185183

186-
${ctx.initNestedClasses()}
187-
${ctx.declareNestedClasses()}
184+
${ctx.declareAddedFunctions()}
188185
}
189186
"""
190187

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -391,8 +391,6 @@ object GenerateUnsafeProjection extends CodeGenerator[Seq[Expression], UnsafePro
391391
${ctx.initPartition()}
392392
}
393393

394-
${ctx.declareAddedFunctions()}
395-
396394
// Scala.Function1 need this
397395
public java.lang.Object apply(java.lang.Object row) {
398396
return apply((InternalRow) row);
@@ -403,8 +401,7 @@ object GenerateUnsafeProjection extends CodeGenerator[Seq[Expression], UnsafePro
403401
return ${eval.value};
404402
}
405403

406-
${ctx.initNestedClasses()}
407-
${ctx.declareNestedClasses()}
404+
${ctx.declareAddedFunctions()}
408405
}
409406
"""
410407

sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegenExec.scala

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -352,14 +352,11 @@ case class WholeStageCodegenExec(child: SparkPlan) extends UnaryExecNode with Co
352352
${ctx.initPartition()}
353353
}
354354

355-
${ctx.declareAddedFunctions()}
356-
357355
protected void processNext() throws java.io.IOException {
358356
${code.trim}
359357
}
360358

361-
${ctx.initNestedClasses()}
362-
${ctx.declareNestedClasses()}
359+
${ctx.declareAddedFunctions()}
363360
}
364361
""".trim
365362

sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,6 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
192192
this.columnIndexes = columnIndexes;
193193
}
194194

195-
${ctx.declareAddedFunctions()}
196-
197195
public boolean hasNext() {
198196
if (currentRow < numRowsInBatch) {
199197
return true;
@@ -222,8 +220,7 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
222220
return unsafeRow;
223221
}
224222

225-
${ctx.initNestedClasses()}
226-
${ctx.declareNestedClasses()}
223+
${ctx.declareAddedFunctions()}
227224
}"""
228225

229226
val code = CodeFormatter.stripOverlappingComments(

0 commit comments

Comments
 (0)