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 2017/08/17 17:39:00 UTC

[GitHub] spark pull request #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit pr...

GitHub user kiszk opened a pull request:

    https://github.com/apache/spark/pull/18972

    [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem with AND or OR

    ## What changes were proposed in this pull request?
    
    This PR changes `AND` or `OR` code generation to place condition and then expressions' generated code into separated methods if these size could be large. When the method is newly generated, variables for isNull and value are declared as an instance variable to pass these values (e.g. isNull1409 and value1409) to the callers of the generated method.
    
    This PR resolved two cases:
    
    * large code size of left expression
    * large code size of right expression
    
    
    ## How was this patch tested?
    
    Added a new test case into `CodeGenerationSuite`


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/kiszk/spark SPARK-21720

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/18972.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 #18972
    
----
commit b915d417c558d8904fd64fae39f55fb976b495f0
Author: Kazuaki Ishizaki <is...@jp.ibm.com>
Date:   2017-08-17T17:28:54Z

    Initial commit

----


---
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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit pr...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18972#discussion_r133879240
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
    @@ -789,6 +789,36 @@ class CodegenContext {
       }
     
       /**
    +   * Wrap the generated code of expression by a function. ev,isNull and ev.value are passed
    +   * by global variables
    --- End diff --
    
    Not arbitrary codes all can be wrapped by this. The codes must be only created from a row object. We should note this in the comment.


---
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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #83740 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83740/testReport)** for PR 18972 at commit [`bf35498`](https://github.com/apache/spark/commit/bf3549882982bcddd7d4d0b38c61d939c2c8a046).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit pr...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18972#discussion_r150284355
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala ---
    @@ -2081,10 +2081,8 @@ class DataFrameSuite extends QueryTest with SharedSQLContext {
         }
     
         withSQLConf(SQLConf.CODEGEN_FALLBACK.key -> "false") {
    -      val e = intercept[SparkException] {
    -        df.filter(filter).count()
    -      }.getMessage
    -      assert(e.contains("grows beyond 64 KB"))
    +      // SPARK-21720 avoids an exception due to JVM code size limit
    --- End diff --
    
    In general, I agree with you that we should create a config.  
    Although I create a PR to add a config for a constant in `CodeGenerator`, it revealed that we need [additional (large) work](https://github.com/apache/spark/pull/19449#pullrequestreview-67789784) to fix active session management.
    
    Can we introduce a config after fixing active session management?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit pr...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/18972


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #80793 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80793/testReport)** for PR 18972 at commit [`b915d41`](https://github.com/apache/spark/commit/b915d417c558d8904fd64fae39f55fb976b495f0).


---
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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit pr...

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/18972#discussion_r150222142
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
    @@ -809,6 +809,36 @@ class CodegenContext {
       }
     
       /**
    +   * Wrap the generated code of expression, which was created from a row object in INPUT_ROW,
    +   * by a function. ev,isNull and ev.value are passed by global variables
    --- End diff --
    
    typo: `ev.isNull`


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80853/
    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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83693/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80817/
    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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit pr...

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/18972#discussion_r150222364
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
    @@ -809,6 +809,36 @@ class CodegenContext {
       }
     
       /**
    +   * Wrap the generated code of expression, which was created from a row object in INPUT_ROW,
    +   * by a function. ev,isNull and ev.value are passed by global variables
    +   *
    +   * @param ev the code to evaluate expressions.
    +   * @param dataType the data type of ev.value.
    +   * @param baseFuncName the split function name base.
    +   */
    +  def createAndAddFunction(
    +      ev: ExprCode,
    +      dataType: DataType,
    +      baseFuncName: String): (String, String, String) = {
    +    val globalIsNull = freshName("isNull")
    +    addMutableState("boolean", globalIsNull, s"$globalIsNull = false;")
    +    val globalValue = freshName("value")
    +    addMutableState(javaType(dataType), globalValue,
    +      s"$globalValue = ${defaultValue(dataType)};")
    +    val funcName = freshName(baseFuncName)
    +    val funcBody =
    +      s"""
    +         |private void $funcName(InternalRow ${INPUT_ROW}) {
    --- End diff --
    
    does it work with whole stage codegen? the input is not `InternalRow` but some variable.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #82675 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82675/testReport)** for PR 18972 at commit [`569a8bd`](https://github.com/apache/spark/commit/569a8bdd86f89d2077576c0aed591d6afc27d637).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83740/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #80853 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80853/testReport)** for PR 18972 at commit [`569a8bd`](https://github.com/apache/spark/commit/569a8bdd86f89d2077576c0aed591d6afc27d637).
     * 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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #80845 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80845/testReport)** for PR 18972 at commit [`569a8bd`](https://github.com/apache/spark/commit/569a8bdd86f89d2077576c0aed591d6afc27d637).


---
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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82695/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    @bali0019 It depends on the progress of the review.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    @cloud-fan Could you please 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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #83740 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83740/testReport)** for PR 18972 at commit [`bf35498`](https://github.com/apache/spark/commit/bf3549882982bcddd7d4d0b38c61d939c2c8a046).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #83693 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83693/testReport)** for PR 18972 at commit [`2f06555`](https://github.com/apache/spark/commit/2f0655575de285aefafbda79cd05695ac1fa6154).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80845/
    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 issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by bali0019 <gi...@git.apache.org>.
Github user bali0019 commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Hi @kiszk , I have few questions on this PR.
    Is there any time estimate on when this PR is supposed to merge ? 
    Also, are these 3 commits under this PR planned to be pushed to Spark 2.2 as well?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    thanks, merging to master/2.2!


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Jenkins, retest this please


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    retest this please


---
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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    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 issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82675/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    @cloud-fan could you review this if you have time?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #83164 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83164/testReport)** for PR 18972 at commit [`c9bc395`](https://github.com/apache/spark/commit/c9bc39562a8be4ba2d1a430a3346649628b451be).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #83693 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83693/testReport)** for PR 18972 at commit [`2f06555`](https://github.com/apache/spark/commit/2f0655575de285aefafbda79cd05695ac1fa6154).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Jenkins, retest this please


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit pr...

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/18972#discussion_r150223463
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala ---
    @@ -2081,10 +2081,8 @@ class DataFrameSuite extends QueryTest with SharedSQLContext {
         }
     
         withSQLConf(SQLConf.CODEGEN_FALLBACK.key -> "false") {
    -      val e = intercept[SparkException] {
    -        df.filter(filter).count()
    -      }.getMessage
    -      assert(e.contains("grows beyond 64 KB"))
    +      // SPARK-21720 avoids an exception due to JVM code size limit
    --- End diff --
    
    I think we should create a config for the threshold instead of hardcoding `1024`, then we can keep the test case here, by setting the threshold to Long.max.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #83164 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83164/testReport)** for PR 18972 at commit [`c9bc395`](https://github.com/apache/spark/commit/c9bc39562a8be4ba2d1a430a3346649628b451be).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #80845 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80845/testReport)** for PR 18972 at commit [`569a8bd`](https://github.com/apache/spark/commit/569a8bdd86f89d2077576c0aed591d6afc27d637).
     * This patch **fails PySpark 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


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #82695 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82695/testReport)** for PR 18972 at commit [`c9bc395`](https://github.com/apache/spark/commit/c9bc39562a8be4ba2d1a430a3346649628b451be).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #80793 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80793/testReport)** for PR 18972 at commit [`b915d41`](https://github.com/apache/spark/commit/b915d417c558d8904fd64fae39f55fb976b495f0).
     * This patch **fails SparkR 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


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80793/
    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 issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #80817 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80817/testReport)** for PR 18972 at commit [`b915d41`](https://github.com/apache/spark/commit/b915d417c558d8904fd64fae39f55fb976b495f0).
     * 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 pull request #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit pr...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18972#discussion_r150302533
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala ---
    @@ -2067,7 +2067,7 @@ class DataFrameSuite extends QueryTest with SharedSQLContext {
           .count
       }
     
    -  testQuietly("SPARK-19372: Filter can be executed w/o generated code due to JVM code size limit") {
    +  test("SPARK-19372: Filter can be executed w/o generated code due to JVM code size limit") {
    --- End diff --
    
    I see. I will do it on Sunday.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #82675 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82675/testReport)** for PR 18972 at commit [`569a8bd`](https://github.com/apache/spark/commit/569a8bdd86f89d2077576c0aed591d6afc27d637).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    ping @cloud-fan 


---
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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83164/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #80853 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80853/testReport)** for PR 18972 at commit [`569a8bd`](https://github.com/apache/spark/commit/569a8bdd86f89d2077576c0aed591d6afc27d637).


---
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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Jenkins, retest this please


---
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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #80817 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80817/testReport)** for PR 18972 at commit [`b915d41`](https://github.com/apache/spark/commit/b915d417c558d8904fd64fae39f55fb976b495f0).


---
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 #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit pr...

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/18972#discussion_r150299574
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala ---
    @@ -2067,7 +2067,7 @@ class DataFrameSuite extends QueryTest with SharedSQLContext {
           .count
       }
     
    -  testQuietly("SPARK-19372: Filter can be executed w/o generated code due to JVM code size limit") {
    +  test("SPARK-19372: Filter can be executed w/o generated code due to JVM code size limit") {
    --- End diff --
    
    I think this test case becomes invalid as we won't trigger the codegen fallback branch now. Can we just ignore this test and add a TODO to say something about the config?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    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 issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit pr...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18972#discussion_r150282403
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
    @@ -809,6 +809,36 @@ class CodegenContext {
       }
     
       /**
    +   * Wrap the generated code of expression, which was created from a row object in INPUT_ROW,
    +   * by a function. ev,isNull and ev.value are passed by global variables
    +   *
    +   * @param ev the code to evaluate expressions.
    +   * @param dataType the data type of ev.value.
    +   * @param baseFuncName the split function name base.
    +   */
    +  def createAndAddFunction(
    +      ev: ExprCode,
    +      dataType: DataType,
    +      baseFuncName: String): (String, String, String) = {
    +    val globalIsNull = freshName("isNull")
    +    addMutableState("boolean", globalIsNull, s"$globalIsNull = false;")
    +    val globalValue = freshName("value")
    +    addMutableState(javaType(dataType), globalValue,
    +      s"$globalValue = ${defaultValue(dataType)};")
    +    val funcName = freshName(baseFuncName)
    +    val funcBody =
    +      s"""
    +         |private void $funcName(InternalRow ${INPUT_ROW}) {
    --- End diff --
    
    Yes, it works only if `ctx.currentVars == null`.  
    We will follow to support the whole stage codegen as follow-up in other PRs.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #18972: [SPARK-21720][SQL] Fix 64KB JVM bytecode limit problem w...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18972
  
    **[Test build #82695 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82695/testReport)** for PR 18972 at commit [`c9bc395`](https://github.com/apache/spark/commit/c9bc39562a8be4ba2d1a430a3346649628b451be).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org