Skip to content

Conversation

@davies
Copy link
Contributor

@davies davies commented Jun 9, 2015

In order to have better performance out of box, this PR turn on codegen by default, then codegen can be tested by sql/test and hive/test.

This PR also fix some corner cases for codegen.

Before 1.5 release, we should re-visit this, turn it off if it's not stable or causing regressions.

cc @rxin @JoshRosen

@SparkQA
Copy link

SparkQA commented Jun 9, 2015

Test build #34518 has finished for PR 6726 at commit a8618c9.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • logInfo(s"Using user defined output committer class $
    • logInfo(s"Using output committer class $

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we call a function here? define some BinaryUtils to put everything for binary there. That makes it much easier to change.

Code generated version can just generate the code to call that function.

@SparkQA
Copy link

SparkQA commented Jun 10, 2015

Test build #34544 has finished for PR 6726 at commit c8e7cd2.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 10, 2015

Test build #34545 has finished for PR 6726 at commit f3886fa.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should remove the case here, i.e.

case IntegerType | BooleanType | LongType | DoubleType | FloatType | ShortType | ByteType
      | DateType =>
  (eval1, eval2) => s"$eval1 == $eval2"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also remove the case for other cases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so code gen is completely off if I use any thread safe == false function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can improve the coverage later, make all the hive tests passed first.

@SparkQA
Copy link

SparkQA commented Jun 10, 2015

Test build #34574 has finished for PR 6726 at commit 207e339.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 10, 2015

Test build #34566 has finished for PR 6726 at commit 44573a3.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class IsNull(child: Expression) extends UnaryExpression with Predicate
    • case class IsNotNull(child: Expression) extends UnaryExpression with Predicate

Davies Liu added 2 commits June 10, 2015 10:48
…codegen

Conflicts:
	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
@SparkQA
Copy link

SparkQA commented Jun 10, 2015

Test build #34623 has finished for PR 6726 at commit 99fc139.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@davies davies changed the title [SPARK-7184] [SQL] enable codegen by default [SPARK-7184] [SPARK-8190] [SQL] enable codegen by default Jun 10, 2015
@davies davies changed the title [SPARK-7184] [SPARK-8190] [SQL] enable codegen by default [SPARK-7184] [SQL] enable codegen by default Jun 11, 2015
Conflicts:
	sql/catalyst/src/main/scala/org/apache/spark/sql/BaseRow.java
	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala
	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateProjection.scala
	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/literals.scala
	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
	sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateUtilsSuite.scala
@SparkQA
Copy link

SparkQA commented Jun 12, 2015

Test build #34727 has finished for PR 6726 at commit ff5b75a.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Conflicts:
	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/nullFunctions.scala
	sql/core/src/main/scala/org/apache/spark/sql/execution/SparkPlan.scala
	sql/core/src/main/scala/org/apache/spark/sql/execution/expressions/MonotonicallyIncreasingID.scala
	sql/core/src/main/scala/org/apache/spark/sql/sources/commands.scala
	sql/core/src/main/scala/org/apache/spark/sql/sources/interfaces.scala
	sql/hive/src/main/scala/org/apache/spark/sql/hive/hiveUdfs.scala
@SparkQA
Copy link

SparkQA commented Jun 13, 2015

Test build #34844 has finished for PR 6726 at commit a7d75da.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class IsNull(child: Expression) extends UnaryExpression with Predicate
    • case class IsNotNull(child: Expression) extends UnaryExpression with Predicate

Conflicts:
	sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/nullFunctions.scala
@SparkQA
Copy link

SparkQA commented Jun 14, 2015

Test build #34859 has finished for PR 6726 at commit 3017a47.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class IsNull(child: Expression) extends UnaryExpression with Predicate
    • case class IsNotNull(child: Expression) extends UnaryExpression with Predicate

@SparkQA
Copy link

SparkQA commented Jun 14, 2015

Test build #34865 has finished for PR 6726 at commit 73750ea.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class JoinedRow extends InternalRow
    • class JoinedRow2 extends InternalRow
    • class JoinedRow3 extends InternalRow
    • class JoinedRow4 extends InternalRow
    • class JoinedRow5 extends InternalRow
    • class JoinedRow6 extends InternalRow
    • class BaseOrdering extends Ordering[InternalRow]
    • case class IsNull(child: Expression) extends UnaryExpression with Predicate
    • case class IsNotNull(child: Expression) extends UnaryExpression with Predicate

@davies
Copy link
Contributor Author

davies commented Jun 14, 2015

@marmbrus @rxin I think this is ready to review, thanks!

@SparkQA
Copy link

SparkQA commented Jun 14, 2015

Test build #34877 has finished for PR 6726 at commit f3b25a5.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class IsNull(child: Expression) extends UnaryExpression with Predicate
    • case class IsNotNull(child: Expression) extends UnaryExpression with Predicate

@rxin
Copy link
Contributor

rxin commented Jun 16, 2015

Going to merge this one.

@asfgit asfgit closed this in bc76a0f Jun 16, 2015
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
In order to have better performance out of box, this PR turn on codegen by default, then codegen can be tested by sql/test and hive/test.

This PR also fix some corner cases for codegen.

Before 1.5 release, we should re-visit this, turn it off if it's not stable or causing regressions.

cc rxin JoshRosen

Author: Davies Liu <[email protected]>

Closes apache#6726 from davies/enable_codegen and squashes the following commits:

f3b25a5 [Davies Liu] fix warning
73750ea [Davies Liu] fix long overflow when compare
3017a47 [Davies Liu] Merge branch 'master' of github.com:apache/spark into enable_codegen
a7d75da [Davies Liu] Merge branch 'master' of github.com:apache/spark into enable_codegen
ff5b75a [Davies Liu] Merge branch 'master' of github.com:apache/spark into enable_codegen
f4cf2c2 [Davies Liu] fix style
99fc139 [Davies Liu] Merge branch 'enable_codegen' of github.com:davies/spark into enable_codegen
91fc7a2 [Davies Liu] disable codegen for ScalaUDF
207e339 [Davies Liu] Update CodeGenerator.scala
44573a3 [Davies Liu] check thread safety of expression
f3886fa [Davies Liu] don't inline primitiveTerm for null literal
c8e7cd2 [Davies Liu] address comment
a8618c9 [Davies Liu] enable codegen by default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants