You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by nongli <gi...@git.apache.org> on 2016/02/02 01:32:04 UTC
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
GitHub user nongli opened a pull request:
https://github.com/apache/spark/pull/11008
[SPARK-13123][SQL] Implement whole state codegen for sort.
This does the simplest thing of just assembly a row on consume and driving the
underlying external sorter object.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nongli/spark spark-13123
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/11008.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #11008
----
commit 11e26c9201f939022a9750c4eb7986f63a37bd5f
Author: Nong Li <no...@databricks.com>
Date: 2016-02-02T00:31:05Z
[SPARK-13123][SQL] Implement whole state codegen for sort.
This does the simplest thing of just assembly a row on consume and driving the
underlying external sorter object.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/11008#discussion_r53856345
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/Sort.scala ---
@@ -93,4 +98,63 @@ case class Sort(
sortedIterator
}
}
+
+ override def upstream(): RDD[InternalRow] = {
+ child.asInstanceOf[CodegenSupport].upstream()
+ }
+
+ // Name of sorter variable used in codegen.
+ private var sorterVariable: String = _
+
+ override protected def doProduce(ctx: CodegenContext): String = {
+ val needToSort = ctx.freshName("needToSort")
+ ctx.addMutableState("boolean", needToSort, s"$needToSort = true;")
+
+
+ // Initialize the class member variables. This includes the instance of the Sorter and
+ // the iterator to return sorted rows.
+ val thisPlan = ctx.addReferenceObj("plan", this)
+ sorterVariable = ctx.freshName("sorter")
+ ctx.addMutableState(classOf[UnsafeExternalRowSorter].getName, sorterVariable,
+ s"$sorterVariable = $thisPlan.createSorter();")
+ val sortedIterator = ctx.freshName("sortedIter")
+ ctx.addMutableState("scala.collection.Iterator<UnsafeRow>", sortedIterator, "")
+
+ val addToSorter = ctx.freshName("addToSorter")
+ ctx.addNewFunction(addToSorter,
+ s"""
+ | private void $addToSorter() throws java.io.IOException {
+ | ${child.asInstanceOf[CodegenSupport].produce(ctx, this)}
+ | }
+ """.stripMargin.trim)
+
+ val outputRow = ctx.freshName("outputRow")
+ s"""
+ | if ($needToSort) {
+ | $addToSorter();
+ | $sortedIterator = $sorterVariable.sort();
+ | $needToSort = false;
+ | }
+ |
+ | while ($sortedIterator.hasNext()) {
+ | UnsafeRow $outputRow = (UnsafeRow)$sortedIterator.next();
+ | ${consume(ctx, null, outputRow)}
+ | }
+ """.stripMargin.trim
+ }
+
+ override def doConsume(ctx: CodegenContext, input: Seq[ExprCode]): String = {
+ val colExprs = child.output.zipWithIndex.map { case (attr, i) =>
+ BoundReference(i, attr.dataType, attr.nullable)
+ }
+
+ ctx.currentVars = input
+ val code = GenerateUnsafeProjection.createCode(ctx, colExprs, false)
--- End diff --
If the child can produce UnsafeRow (for example, Exchange), we should have a way to avoid this unpack and pack again, or we will see regression (generated version slower than non-generated).
I think we can pass the variable for input row into `doCosume`, could be null. It's better to do this after #11274 , then we don't need to worry about whether should we create variables for input or not.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11008#issuecomment-178826831
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50584/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11008#issuecomment-178273556
**[Test build #50511 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50511/consoleFull)** for PR 11008 at commit [`11e26c9`](https://github.com/apache/spark/commit/11e26c9201f939022a9750c4eb7986f63a37bd5f).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `public final class UnsafeExternalRowSorter `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by nongli <gi...@git.apache.org>.
Github user nongli commented on the pull request:
https://github.com/apache/spark/pull/11008#issuecomment-178273018
```
/* 001 */ public Object generate(Object[] references) {
/* 002 */ return new GeneratedIterator(references);
/* 003 */ }
/* 004 */
/* 005 */ class GeneratedIterator extends org.apache.spark.sql.execution.BufferedRowIterator {
/* 006 */
/* 007 */ private Object[] references;
/* 008 */ private boolean Sort_needToSort0;
/* 009 */ private org.apache.spark.sql.execution.Sort Sort_plan1;
/* 010 */ private org.apache.spark.sql.execution.UnsafeExternalRowSorter Sort_sorter2;
/* 011 */ private scala.collection.Iterator<UnsafeRow> Sort_sortedIter3;
/* 012 */ private UnsafeRow Sort_result24;
/* 013 */ private org.apache.spark.sql.catalyst.expressions.codegen.BufferHolder Sort_holder25;
/* 014 */ private org.apache.spark.sql.catalyst.expressions.codegen.UnsafeRowWriter Sort_rowWriter26;
/* 015 */
/* 016 */ private void Sort_addToSorter4() throws java.io.IOException {
/* 017 */
/* 018 */ while (input.hasNext()) {
/* 019 */ InternalRow InputAdapter_row5 = (InternalRow) input.next();
/* 020 */ /* input[0, int] */
/* 021 */ boolean InputAdapter_isNull6 = InputAdapter_row5.isNullAt(0);
/* 022 */ int InputAdapter_value7 = InputAdapter_isNull6 ? -1 : (InputAdapter_row5.getInt(0));
/* 023 */ /* input[1, string] */
/* 024 */ boolean InputAdapter_isNull8 = InputAdapter_row5.isNullAt(1);
/* 025 */ UTF8String InputAdapter_value9 = InputAdapter_isNull8 ? null : (InputAdapter_row5.getUTF8String(1));
/* 026 */
/* 027 */ /* (input[0, int] < 20) */
/* 028 */ /* input[0, int] */
/* 029 */
/* 030 */ /* 20 */
/* 031 */
/* 032 */ boolean Filter_value11 = false;
/* 033 */ Filter_value11 = InputAdapter_value7 < 20;
/* 034 */ if (!false && Filter_value11) {
/* 035 */
/* 036 */ /* input[0, int] */
/* 037 */
/* 038 */ /* input[1, string] */
/* 039 */
/* 040 */
/* 041 */ // Convert the input attributes to an UnsafeRow and add it to the sorter
/* 042 */
/* 043 */ Sort_holder25.reset();
/* 044 */
/* 045 */ Sort_rowWriter26.zeroOutNullBytes();
/* 046 */
/* 047 */ /* input[0, int] */
/* 048 */
/* 049 */ if (InputAdapter_isNull6) {
/* 050 */ Sort_rowWriter26.setNullAt(0);
/* 051 */ } else {
/* 052 */ Sort_rowWriter26.write(0, InputAdapter_value7);
/* 053 */ }
/* 054 */
/* 055 */ /* input[1, string] */
/* 056 */
/* 057 */ if (InputAdapter_isNull8) {
/* 058 */ Sort_rowWriter26.setNullAt(1);
/* 059 */ } else {
/* 060 */ Sort_rowWriter26.write(1, InputAdapter_value9);
/* 061 */ }
/* 062 */ Sort_result24.setTotalSize(Sort_holder25.totalSize());
/* 063 */
/* 064 */ Sort_sorter2.insertRow(Sort_result24);
/* 065 */
/* 066 */ }
/* 067 */
/* 068 */ }
/* 069 */
/* 070 */ }
/* 071 */
/* 072 */ public GeneratedIterator(Object[] references) {
/* 073 */ this.references = references;
/* 074 */ Sort_needToSort0 = true;
/* 075 */ this.Sort_plan1 = (org.apache.spark.sql.execution.Sort) references[0];
/* 076 */ Sort_sorter2 = Sort_plan1.createSorter();
/* 077 */
/* 078 */ Sort_result24 = new UnsafeRow(2);
/* 079 */ this.Sort_holder25 = new org.apache.spark.sql.catalyst.expressions.codegen.BufferHolder(Sort_result24, 32);
/* 080 */ this.Sort_rowWriter26 = new org.apache.spark.sql.catalyst.expressions.codegen.UnsafeRowWriter(Sort_holder25, 2);
/* 081 */ }
/* 082 */
/* 083 */ protected void processNext() throws java.io.IOException {
/* 084 */ if (Sort_needToSort0) {
/* 085 */ Sort_addToSorter4();
/* 086 */ Sort_sortedIter3 = Sort_sorter2.sort();
/* 087 */ Sort_needToSort0 = false;
/* 088 */ }
/* 089 */
/* 090 */ while (Sort_sortedIter3.hasNext()) {
/* 091 */ UnsafeRow Sort_outputRow29 = (UnsafeRow)Sort_sortedIter3.next();
/* 092 */ System.out.println(Sort_outputRow29);
/* 093 */ currentRow = Sort_outputRow29;
/* 094 */ return;
/* 095 */ }
/* 096 */ }
/* 097 */ }
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by nongli <gi...@git.apache.org>.
Github user nongli commented on a diff in the pull request:
https://github.com/apache/spark/pull/11008#discussion_r51624075
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/Sort.scala ---
@@ -93,4 +98,63 @@ case class Sort(
sortedIterator
}
}
+
+ override def upstream(): RDD[InternalRow] = {
+ child.asInstanceOf[CodegenSupport].upstream()
+ }
+
+ // Name of sorter variable used in codegen.
+ private var sorterVariable: String = _
--- End diff --
Why? This is the state that needs to be kept between the two member functions in this class.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11008#issuecomment-178273560
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50511/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by nongli <gi...@git.apache.org>.
Github user nongli closed the pull request at:
https://github.com/apache/spark/pull/11008
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/11008#discussion_r51834737
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/Sort.scala ---
@@ -93,4 +98,63 @@ case class Sort(
sortedIterator
}
}
+
+ override def upstream(): RDD[InternalRow] = {
+ child.asInstanceOf[CodegenSupport].upstream()
+ }
+
+ // Name of sorter variable used in codegen.
+ private var sorterVariable: String = _
--- End diff --
it's ok here as discussed offline. i just found mutable state in here as a way to pass variable names through pretty brittle. maybe good to have a more general abstraction for this in codegen, but not that big of a deal right now/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11008#issuecomment-178795947
**[Test build #50584 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50584/consoleFull)** for PR 11008 at commit [`564a5b3`](https://github.com/apache/spark/commit/564a5b362530f7618d17074bdb2ade33dd4ec6f5).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/11008#discussion_r51525091
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/Sort.scala ---
@@ -93,4 +98,63 @@ case class Sort(
sortedIterator
}
}
+
+ override def upstream(): RDD[InternalRow] = {
+ child.asInstanceOf[CodegenSupport].upstream()
+ }
+
+ // Name of sorter variable used in codegen.
+ private var sorterVariable: String = _
--- End diff --
this is pretty ghetto... (although i understand maybe it's the simplest way to implement this)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/11008#issuecomment-191427912
@nongli Can you close this PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11008#issuecomment-178826829
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11008#issuecomment-178273558
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11008#issuecomment-178273316
**[Test build #50511 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50511/consoleFull)** for PR 11008 at commit [`11e26c9`](https://github.com/apache/spark/commit/11e26c9201f939022a9750c4eb7986f63a37bd5f).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13123][SQL] Implement whole state codeg...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11008#issuecomment-178826530
**[Test build #50584 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50584/consoleFull)** for PR 11008 at commit [`564a5b3`](https://github.com/apache/spark/commit/564a5b362530f7618d17074bdb2ade33dd4ec6f5).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org