You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by kiszk <gi...@git.apache.org> on 2016/06/14 16:50:53 UTC
[GitHub] spark pull request #13663: Spark SparkSPARK-15950 Eliminate unreachable code...
GitHub user kiszk opened a pull request:
https://github.com/apache/spark/pull/13663
Spark SparkSPARK-15950 Eliminate unreachable code at projection for complex types
## What changes were proposed in this pull request?
This PR eliminates unreachable code at projection for complex types (e.g. array, map, and struct) to reduce the total size of Java byte code in a method generated by whole stage codegen.
This PR eliminates the following code blocks in the following cases:
1. A type of ```project_value``` is ```UnsafeArrayData```
2. ```project_value is``` ```null```
3. each element of ```project_values``` is ```null``` if it never occurs
Examples for complex types
````java
val df = sparkContext.parallelize(Seq(1, 2), 1).toDF("v")
df.selectExpr("Array(v + 2, v + 3)").collect
df.selectExpr("struct(v + 3, v + 4)").collect
df.selectExpr("map(v + 3, v + 4)").collect
````
Before applying this PR into ```Array```
```java
/* 028 */ protected void processNext() throws java.io.IOException {
/* 029 */ while (inputadapter_input.hasNext()) {
/* 030 */ InternalRow inputadapter_row = (InternalRow) inputadapter_input.next();
/* 031 */ final boolean project_isNull = false;
/* 032 */ this.project_values = new Object[2];
/* 033 */ double inputadapter_value = inputadapter_row.getDouble(0);
/* 034 */
/* 035 */ double project_value1 = -1.0;
/* 036 */ project_value1 = inputadapter_value + 2.2D;
/* 037 */ if (false) {
/* 038 */ project_values[0] = null;
/* 039 */ } else {
/* 040 */ project_values[0] = project_value1;
/* 041 */ }
/* 042 */
/* 043 */ double inputadapter_value1 = inputadapter_row.getDouble(1);
/* 044 */
/* 045 */ double project_value4 = -1.0;
/* 046 */ project_value4 = inputadapter_value1 + 3.3D;
/* 047 */ if (false) {
/* 048 */ project_values[1] = null;
/* 049 */ } else {
/* 050 */ project_values[1] = project_value4;
/* 051 */ }
/* 052 */
/* 053 */ final ArrayData project_value = new org.apache.spark.sql.catalyst.util.GenericArrayData(project_values);
/* 054 */ this.project_values = null;
/* 055 */ project_holder.reset();
/* 056 */
/* 057 */ project_rowWriter.zeroOutNullBytes();
/* 058 */
/* 059 */ if (project_isNull) {
/* 060 */ project_rowWriter.setNullAt(0);
/* 061 */ } else {
/* 062 */ // Remember the current cursor so that we can calculate how many bytes are
/* 063 */ // written later.
/* 064 */ final int project_tmpCursor = project_holder.cursor;
/* 065 */
/* 066 */ if (project_value instanceof UnsafeArrayData) {
/* 067 */ final int project_sizeInBytes = ((UnsafeArrayData) project_value).getSizeInBytes();
/* 068 */ // grow the global buffer before writing data.
/* 069 */ project_holder.grow(project_sizeInBytes);
/* 070 */ ((UnsafeArrayData) project_value).writeToMemory(project_holder.buffer, project_holder.cursor);
/* 071 */ project_holder.cursor += project_sizeInBytes;
/* 072 */
/* 073 */ } else {
/* 074 */ final int project_numElements = project_value.numElements();
/* 075 */ project_arrayWriter.initialize(project_holder, project_numElements, 8);
/* 076 */
/* 077 */ for (int project_index = 0; project_index < project_numElements; project_index++) {
/* 078 */ if (project_value.isNullAt(project_index)) {
/* 079 */ project_arrayWriter.setNullAt(project_index);
/* 080 */ } else {
/* 081 */ final double project_element = project_value.getDouble(project_index);
/* 082 */ project_arrayWriter.write(project_index, project_element);
/* 083 */ }
/* 084 */ }
/* 085 */ }
/* 086 */
/* 087 */ project_rowWriter.setOffsetAndSize(0, project_tmpCursor, project_holder.cursor - project_tmpCursor);
/* 088 */ project_rowWriter.alignToWords(project_holder.cursor - project_tmpCursor);
/* 089 */ }
/* 090 */ project_result.setTotalSize(project_holder.totalSize());
/* 091 */ append(project_result);
/* 092 */ if (shouldStop()) return;
/* 093 */ }
/* 094 */ }
````
After applying this PR into ```Array```
````java
/* 028 */ protected void processNext() throws java.io.IOException {
/* 029 */ while (inputadapter_input.hasNext()) {
/* 030 */ InternalRow inputadapter_row = (InternalRow) inputadapter_input.next();
/* 031 */ final boolean project_isNull = false;
/* 032 */ this.project_values = new Object[2];
/* 033 */ double inputadapter_value = inputadapter_row.getDouble(0);
/* 034 */
/* 035 */ double project_value1 = -1.0;
/* 036 */ project_value1 = inputadapter_value + 2.2D;
/* 037 */ project_values[0] = project_value1;
/* 038 */ double inputadapter_value1 = inputadapter_row.getDouble(1);
/* 039 */
/* 040 */ double project_value4 = -1.0;
/* 041 */ project_value4 = inputadapter_value1 + 3.3D;
/* 042 */ project_values[1] = project_value4;
/* 043 */ final ArrayData project_value = new org.apache.spark.sql.catalyst.util.GenericArrayData(project_values);
/* 044 */ this.project_values = null;
/* 045 */ project_holder.reset();
/* 046 */
/* 047 */ // Remember the current cursor so that we can calculate how many bytes are
/* 048 */ // written later.
/* 049 */ final int project_tmpCursor = project_holder.cursor;
/* 050 */
/* 051 */ final int project_numElements = project_value.numElements();
/* 052 */ project_arrayWriter.initialize(project_holder, project_numElements, 8);
/* 053 */
/* 054 */ for (int project_index = 0; project_index < project_numElements; project_index++) {
/* 055 */ if (project_value.isNullAt(project_index)) {
/* 056 */ project_arrayWriter.setNullAt(project_index);
/* 057 */ } else {
/* 058 */ final double project_element = project_value.getDouble(project_index);
/* 059 */ project_arrayWriter.write(project_index, project_element);
/* 060 */ }
/* 061 */ }
/* 062 */
/* 063 */ project_rowWriter.setOffsetAndSize(0, project_tmpCursor, project_holder.cursor - project_tmpCursor);
/* 064 */ project_rowWriter.alignToWords(project_holder.cursor - project_tmpCursor);
/* 065 */ project_result.setTotalSize(project_holder.totalSize());
/* 066 */ append(project_result);
/* 067 */ if (shouldStop()) return;
/* 068 */ }
/* 069 */ }
````
## How was this patch tested?
Added some unit tests into ```DataFrameComplexTypeSuite```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kiszk/spark SPARK-15950
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/13663.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 #13663
----
----
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67288169
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameComplexTypeSuite.scala ---
@@ -26,6 +26,38 @@ import org.apache.spark.sql.test.SharedSQLContext
class DataFrameComplexTypeSuite extends QueryTest with SharedSQLContext {
import testImplicits._
+ test("primitive type on array") {
+ val df = sparkContext.parallelize(Seq(1, 2), 1).toDF("v")
--- End diff --
how do these tests prove the `zeroOutNullBytes` is eliminated?
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13663
**[Test build #60639 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60639/consoleFull)** for PR 13663 at commit [`8d7d311`](https://github.com/apache/spark/commit/8d7d311c5808f983e5de19f50c5034e89e6b7629).
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67819822
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameComplexTypeSuite.scala ---
@@ -26,6 +27,55 @@ import org.apache.spark.sql.test.SharedSQLContext
class DataFrameComplexTypeSuite extends QueryTest with SharedSQLContext {
import testImplicits._
+ test("create an array") {
+ val df = sparkContext.parallelize(Seq(1, 2), 1).toDF("v")
+ df.selectExpr("Array(v + 3, v + 4)").collect
--- End diff --
what does it test?
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13663
**[Test build #60939 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60939/consoleFull)** for PR 13663 at commit [`7ad542d`](https://github.com/apache/spark/commit/7ad542d81005bf074f6df8404b3cb228d277621c).
* This patch passes all 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
[GitHub] spark issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on the issue:
https://github.com/apache/spark/pull/13663
+1 for @cloud-fan said.
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67315144
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala ---
@@ -71,7 +71,8 @@ case class CreateArray(children: Seq[Expression]) extends Expression {
s"""
final ArrayData ${ev.value} = new $arrayClass($values);
this.$values = null;
- """)
+ """,
+ isNull = "false")
--- End diff --
Got it. Removed this assignment.
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67863392
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameComplexTypeSuite.scala ---
@@ -26,6 +27,55 @@ import org.apache.spark.sql.test.SharedSQLContext
class DataFrameComplexTypeSuite extends QueryTest with SharedSQLContext {
import testImplicits._
+ test("create an array") {
+ val df = sparkContext.parallelize(Seq(1, 2), 1).toDF("v")
+ df.selectExpr("Array(v + 3, v + 4)").collect
--- End diff --
I see. I will remove these three tests.
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67288116
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala ---
@@ -71,7 +71,8 @@ case class CreateArray(children: Seq[Expression]) extends Expression {
s"""
final ArrayData ${ev.value} = new $arrayClass($values);
this.$values = null;
- """)
+ """,
+ isNull = "false")
--- End diff --
with this, I think we can remove the `final boolean ${ev.isNull} = false;` in the generated code.
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13663
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60582/
Test PASSed.
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13663
Merged build finished. Test PASSed.
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67209280
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala ---
@@ -60,18 +60,24 @@ case class CreateArray(children: Seq[Expression]) extends Expression {
ctx.INPUT_ROW,
children.zipWithIndex.map { case (e, i) =>
val eval = e.genCode(ctx)
- eval.code + s"""
- if (${eval.isNull}) {
--- End diff --
Javac or just-in-time compiler can optimize this. But, code generator can easily do it without a global variable, too. I think that this is a good tradeoff.
What would you think?
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13663
**[Test build #60495 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60495/consoleFull)** for PR 13663 at commit [`9e7b91d`](https://github.com/apache/spark/commit/9e7b91db49a2949bb17d85beada70ee1f2117a14).
* This patch passes all 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
[GitHub] spark issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/13663
https://github.com/apache/spark/pull/16008 implemented the same solution.
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13663
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60495/
Test PASSed.
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13663
**[Test build #60639 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60639/consoleFull)** for PR 13663 at commit [`8d7d311`](https://github.com/apache/spark/commit/8d7d311c5808f983e5de19f50c5034e89e6b7629).
* This patch passes all 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
[GitHub] spark issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13663
Merged build finished. Test PASSed.
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13663
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60639/
Test PASSed.
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67849394
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameComplexTypeSuite.scala ---
@@ -26,6 +27,55 @@ import org.apache.spark.sql.test.SharedSQLContext
class DataFrameComplexTypeSuite extends QueryTest with SharedSQLContext {
import testImplicits._
+ test("create an array") {
+ val df = sparkContext.parallelize(Seq(1, 2), 1).toDF("v")
+ df.selectExpr("Array(v + 3, v + 4)").collect
--- End diff --
To make sure, when the code is eliminated, the simply array creation works well without any exception.
Do you think this is unnecessary?
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13663
Merged build finished. Test PASSed.
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13663
Merged build finished. Test PASSed.
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/13663
@cloud-fan , @davies , I would appreciate it if you would look at this again. I think that I had addressed all of your comments.
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13663
**[Test build #60939 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60939/consoleFull)** for PR 13663 at commit [`7ad542d`](https://github.com/apache/spark/commit/7ad542d81005bf074f6df8404b3cb228d277621c).
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67386476
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala ---
@@ -60,18 +60,24 @@ case class CreateArray(children: Seq[Expression]) extends Expression {
ctx.INPUT_ROW,
children.zipWithIndex.map { case (e, i) =>
val eval = e.genCode(ctx)
- eval.code + s"""
- if (${eval.isNull}) {
--- End diff --
Just a memo.
I found that ```<exprcode>.map(_.isNull).mkString(" || ")``` is used in some places. This generates code sequence ```isNULL = false || false;```
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67315501
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameComplexTypeSuite.scala ---
@@ -26,6 +26,38 @@ import org.apache.spark.sql.test.SharedSQLContext
class DataFrameComplexTypeSuite extends QueryTest with SharedSQLContext {
import testImplicits._
+ test("primitive type on array") {
+ val df = sparkContext.parallelize(Seq(1, 2), 1).toDF("v")
--- End diff --
I added unit tests to check whether the ```zeroOutNullBytes``` is eliminated.
I eliminated three cases such as ```array on array```. I think that we need to unit tests to generate an array, struct, or map, in order to check it works well even when we eliminate ```zeroOutNullBytes```.
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67272226
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala ---
@@ -60,18 +60,24 @@ case class CreateArray(children: Seq[Expression]) extends Expression {
ctx.INPUT_ROW,
children.zipWithIndex.map { case (e, i) =>
val eval = e.genCode(ctx)
- eval.code + s"""
- if (${eval.isNull}) {
--- End diff --
@cloud-fan , the latest commit only avoids ```zeroOutNullBytes``` by a simple and clear way.
Would it be possible to review 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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13663
**[Test build #60569 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60569/consoleFull)** for PR 13663 at commit [`4458f4f`](https://github.com/apache/spark/commit/4458f4f9c0b9d08937b1e2f4fe028cdd2969fc9a).
* This patch passes all 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
[GitHub] spark issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13663
**[Test build #60582 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60582/consoleFull)** for PR 13663 at commit [`a193007`](https://github.com/apache/spark/commit/a19300736210ed30663ec00c531b6c40d4127a92).
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/13663
@cloud-fan and @davies , thank you for your comments.
A global variable is used for 1. I will address 1. by using another approach without using a global variable in another PR. This PR will focus on 2. and 3. that are addressed without using a global variable.
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13663
**[Test build #60582 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60582/consoleFull)** for PR 13663 at commit [`a193007`](https://github.com/apache/spark/commit/a19300736210ed30663ec00c531b6c40d4127a92).
* This patch passes all 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
[GitHub] spark issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/13663
IIUC, this PR adds a global variable to eliminate some if branches that are very easy to be optimized by javac or JIT.
I think this is not a good tradeoff, the readability of the scala code is much more important than the readability(or code size) of the generated java code.
cc @davies
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13663
**[Test build #60569 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60569/consoleFull)** for PR 13663 at commit [`4458f4f`](https://github.com/apache/spark/commit/4458f4f9c0b9d08937b1e2f4fe028cdd2969fc9a).
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13663
Merged build finished. Test PASSed.
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk closed the pull request at:
https://github.com/apache/spark/pull/13663
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67200614
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala ---
@@ -60,18 +60,24 @@ case class CreateArray(children: Seq[Expression]) extends Expression {
ctx.INPUT_ROW,
children.zipWithIndex.map { case (e, i) =>
val eval = e.genCode(ctx)
- eval.code + s"""
- if (${eval.isNull}) {
--- End diff --
I think javac can optimize `if (false)` for us and we don't need to worry about it
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67213607
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala ---
@@ -60,18 +60,24 @@ case class CreateArray(children: Seq[Expression]) extends Expression {
ctx.INPUT_ROW,
children.zipWithIndex.map { case (e, i) =>
val eval = e.genCode(ctx)
- eval.code + s"""
- if (${eval.isNull}) {
--- End diff --
Let me change my opinion. Another PR will handle this elimination and related features.
This PR will focus on "each element of project_values is null if it never occurs".
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13663
**[Test build #60495 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60495/consoleFull)** for PR 13663 at commit [`9e7b91d`](https://github.com/apache/spark/commit/9e7b91db49a2949bb17d85beada70ee1f2117a14).
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67225615
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeCreator.scala ---
@@ -60,18 +60,24 @@ case class CreateArray(children: Seq[Expression]) extends Expression {
ctx.INPUT_ROW,
children.zipWithIndex.map { case (e, i) =>
val eval = e.genCode(ctx)
- eval.code + s"""
- if (${eval.isNull}) {
--- End diff --
it's not easy, for example:
```
boolean isNull = false || false;
if (isNull) ...
```
this should be optimized, but is very hard for our string based codegen framework.
I think it makes more sense to leave them for javac or JIT, again, the readability of generated java code doesn't matter.
BTW, it's a good idea to avoid `zeroOutNullBytes`, if we can figure out a simple and clear way to do it.
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13663
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60569/
Test PASSed.
---
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 #13663: [SPARK-15950][SQL] Eliminate unreachable code at ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/13663#discussion_r67852633
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameComplexTypeSuite.scala ---
@@ -26,6 +27,55 @@ import org.apache.spark.sql.test.SharedSQLContext
class DataFrameComplexTypeSuite extends QueryTest with SharedSQLContext {
import testImplicits._
+ test("create an array") {
+ val df = sparkContext.parallelize(Seq(1, 2), 1).toDF("v")
+ df.selectExpr("Array(v + 3, v + 4)").collect
--- End diff --
I think we already have tests for array creation (if not, then the test coverage of spark is too bad...)
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13663
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60939/
Test PASSed.
---
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 issue #13663: [SPARK-15950][SQL] Eliminate unreachable code at project...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/13663
@cloud-fan , @davies , I would appreciate it if you would look at this again.
---
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