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/03/27 06:27:06 UTC

[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

GitHub user kiszk opened a pull request:

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

    [Spark-14138][SQL] Fix generated SpecificColumnarIterator code can exceed JVM size limit for cached DataFrames

    ## What changes were proposed in this pull request?
    
    This PR reduces Java byte code size of method in ```SpecificColumnarIterator``` by using two approaches:
    1. Generate and call ```getTYPEColumnAccessor()``` for each type, which is actually used, for instantiating accessors
    2. Group a lot of method calls (more than 4000) into a method
    
    ## How was this patch tested?
    
    Added a new unit test to ```InMemoryColumnarQuerySuite```
    
    Here is generate code
    
    ```java
    /* 033 */   private org.apache.spark.sql.execution.columnar.CachedBatch batch = null;
    /* 034 */
    /* 035 */   private org.apache.spark.sql.execution.columnar.IntColumnAccessor accessor;
    /* 036 */   private org.apache.spark.sql.execution.columnar.IntColumnAccessor accessor1;
    /* 037 */
    /* 038 */   public SpecificColumnarIterator() {
    /* 039 */     this.nativeOrder = ByteOrder.nativeOrder();
    /* 030 */     this.mutableRow = new MutableUnsafeRow(rowWriter);
    /* 041 */   }
    /* 042 */
    /* 043 */   public void initialize(Iterator input, DataType[] columnTypes, int[] columnIndexes,
    /* 044 */     boolean columnNullables[]) {
    /* 044 */     this.input = input;
    /* 046 */     this.columnTypes = columnTypes;
    /* 047 */     this.columnIndexes = columnIndexes;
    /* 048 */   }
    /* 049 */
    /* 050 */
    /* 051 */   private org.apache.spark.sql.execution.columnar.IntColumnAccessor getIntColumnAccessor(int idx) {
    /* 052 */     byte[] buffer = batch.buffers()[columnIndexes[idx]];
    /* 053 */     return new org.apache.spark.sql.execution.columnar.IntColumnAccessor(ByteBuffer.wrap(buffer).order(nativeOrder));
    /* 054 */   }
    /* 055 */
    /* 056 */
    /* 057 */
    /* 058 */
    /* 059 */
    /* 060 */
    /* 061 */   public boolean hasNext() {
    /* 062 */     if (currentRow < numRowsInBatch) {
    /* 063 */       return true;
    /* 064 */     }
    /* 065 */     if (!input.hasNext()) {
    /* 066 */       return false;
    /* 067 */     }
    /* 068 */
    /* 069 */     batch = (org.apache.spark.sql.execution.columnar.CachedBatch) input.next();
    /* 070 */     currentRow = 0;
    /* 071 */     numRowsInBatch = batch.numRows();
    /* 072 */     accessor = getIntColumnAccessor(0);
    /* 073 */     accessor1 = getIntColumnAccessor(1);
    /* 074 */
    /* 075 */     return hasNext();
    /* 076 */   }
    /* 077 */
    /* 078 */   public InternalRow next() {
    /* 079 */     currentRow += 1;
    /* 080 */     bufferHolder.reset();
    /* 081 */     rowWriter.zeroOutNullBytes();
    /* 082 */     accessor.extractTo(mutableRow, 0);
    /* 083 */     accessor1.extractTo(mutableRow, 1);
    /* 084 */     unsafeRow.setTotalSize(bufferHolder.totalSize());
    /* 085 */     return unsafeRow;
    /* 086 */   }
    ```
    
    (If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
    


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

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

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

    https://github.com/apache/spark/pull/11984.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 #11984
    
----
commit ab67d33787e568245c9e2ab30e51b471f21fa2ed
Author: Kazuaki Ishizaki <is...@jp.ibm.com>
Date:   2016-03-27T04:15:06Z

    make code size of hasNext() smaller by preparing get*Acceessor() methods
    
    group a lot of calls into a method

commit fea2a524bbd5b1d0d285e02e6eda590d1f7d67e3
Author: Kazuaki Ishizaki <is...@jp.ibm.com>
Date:   2016-03-27T04:15:38Z

    add test case

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-201989658
  
    **[Test build #54274 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54274/consoleFull)** for PR 11984 at commit [`fea2a52`](https://github.com/apache/spark/commit/fea2a524bbd5b1d0d285e02e6eda590d1f7d67e3).
     * This patch **fails Scala style 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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203570445
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54538/
    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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57925823
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -114,6 +123,57 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
           (createCode, extract + patch)
         }.unzip
     
    +    val accessorCode = accessorClasses.map { case (accessorCls, shortAccCls) =>
    +      s"""
    +         private $accessorCls get${shortAccCls}(int idx) {
    +           byte[] buffer = batch.buffers()[columnIndexes[idx]];
    +           return new $accessorCls(ByteBuffer.wrap(buffer).order(nativeOrder));
    +         }
    +      """
    +    }
    +    val accessorStructCode = accessorStructClasses.map {
    +      case ((accessorCls, dtCls), (shortAccCls, shortDTCls)) =>
    +        s"""
    +           private $accessorCls get${shortAccCls}_${shortDTCls}(int idx) {
    +             byte[] buffer = batch.buffers()[columnIndexes[idx]];
    +             return new $accessorCls(ByteBuffer.wrap(buffer).order(nativeOrder),
    +               (${dtCls}) columnTypes[idx]);
    +           }
    +        """
    +    }
    +
    +    /* 4000 = 64000 bytes / 16 (up to 16 bytes per one call)) */
    +    val numberOfStatementsThreshold = 4000
    --- End diff --
    
    @davies , thank you for your comment. I did not know the limitation. Now, I confirmed these methods are compiled as follows:
    
    ```
    hotspot_pid19296.log:<nmethod compile_id='10059' compiler='C1' level='3' entry='0x00007f03a9574500' size='3024' address='0x00007f03a95742d0' relocation_offset='296' insts_offset='560' stub_offset='2096' scopes_data_offset='2472' scopes_pcs_offset='2680' dependencies_offset='2984' nul_chk_table_offset='2992' oops_offset='2424' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator hasNext ()Z' bytes='92' count='384' iicount='384' stamp='25.140'/>
    ```
    
    ```
    hotspot_pid19296.log:<nmethod compile_id='11143' compiler='C1' level='3' entry='0x00007f03a8ec0680' size='3656' address='0x00007f03a8ec0450' relocation_offset='296' insts_offset='560' stub_offset='2352' scopes_data_offset='2752' scopes_pcs_offset='3192' dependencies_offset='3592' nul_chk_table_offset='3600' oops_offset='2664' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator next ()Lorg/apache/spark/sql/catalyst/InternalRow;' bytes='88' count='384' iicount='384' stamp='34.011'/>
    ```
    
    ```
    hotspot_pid19296.log:<nmethod compile_id='11144' compiler='C1' level='3' entry='0x00007f03a9a5dbc0' size='226544' address='0x00007f03a9a5a890' relocation_offset='296' insts_offset='13104' stub_offset='155280' scopes_data_offset='163488' scopes_pcs_offset='198456' dependencies_offset='222520' nul_chk_table_offset='222528' oops_offset='163432' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator extractors0$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='6867' count='391' iicount='391' stamp='34.105'/>
    hotspot_pid19296.log:<nmethod compile_id='11255' compiler='C1' level='3' entry='0x00007f03aa327e00' size='226632' address='0x00007f03aa324ad0' relocation_offset='296' insts_offset='13104' stub_offset='155280' scopes_data_offset='163472' scopes_pcs_offset='198544' dependencies_offset='222608' nul_chk_table_offset='222616' oops_offset='163432' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator extractors2$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='7001' count='521' iicount='521' stamp='37.163'/>
    hotspot_pid19296.log:<nmethod compile_id='11256' compiler='C1' level='3' entry='0x00007f03aa35f380' size='226664' address='0x00007f03aa35c050' relocation_offset='296' insts_offset='13104' stub_offset='155280' scopes_data_offset='163472' scopes_pcs_offset='198552' dependencies_offset='222632' nul_chk_table_offset='222640' oops_offset='163432' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator extractors4$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='7001' count='530' iicount='530' stamp='37.286'/>
    hotspot_pid19296.log:<nmethod compile_id='11257' compiler='C1' level='3' entry='0x00007f03aa396900' size='226664' address='0x00007f03aa3935d0' relocation_offset='296' insts_offset='13104' stub_offset='155280' scopes_data_offset='163472' scopes_pcs_offset='198552' dependencies_offset='222632' nul_chk_table_offset='222640' oops_offset='163432' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator extractors5$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='7001' count='541' iicount='541' stamp='37.427'/>
    hotspot_pid19296.log:<nmethod compile_id='11263' compiler='C1' level='3' entry='0x00007f03aa3cde80' size='226632' address='0x00007f03aa3cab50' relocation_offset='296' insts_offset='13104' stub_offset='155280' scopes_data_offset='163472' scopes_pcs_offset='198544' dependencies_offset='222608' nul_chk_table_offset='222616' oops_offset='163432' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator extractors1$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='7001' count='547' iicount='547' stamp='37.516'/>
    hotspot_pid19296.log:<nmethod compile_id='11264' compiler='C1' level='3' entry='0x00007f03aa405400' size='226664' address='0x00007f03aa4020d0' relocation_offset='296' insts_offset='13104' stub_offset='155280' scopes_data_offset='163472' scopes_pcs_offset='198552' dependencies_offset='222632' nul_chk_table_offset='222640' oops_offset='163432' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator extractors3$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='7001' count='555' iicount='555' stamp='37.607'/>
    ```
    
    ```
    hotspot_pid19296.log:<nmethod compile_id='10750' compiler='C1' level='3' entry='0x00007f03a9fe9dc0' size='340208' address='0x00007f03a9fe5790' relocation_offset='296' insts_offset='17968' stub_offset='182384' scopes_data_offset='193120' scopes_pcs_offset='304680' dependencies_offset='335480' handler_table_offset='335488' nul_chk_table_offset='337840' oops_offset='192920' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator accessors0$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='5367' count='231' iicount='231' stamp='29.373'/>
    hotspot_pid19296.log:<nmethod compile_id='10751' compiler='C1' level='3' entry='0x00007f03aa03cec0' size='340528' address='0x00007f03aa038890' relocation_offset='296' insts_offset='17968' stub_offset='182608' scopes_data_offset='193296' scopes_pcs_offset='305000' dependencies_offset='335800' handler_table_offset='335808' nul_chk_table_offset='338160' oops_offset='193144' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator accessors1$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='5501' count='239' iicount='239' stamp='29.464'/>
    hotspot_pid19296.log:<nmethod compile_id='11053' compiler='C1' level='3' entry='0x00007f03aa1c74c0' size='340528' address='0x00007f03aa1c2e90' relocation_offset='296' insts_offset='17968' stub_offset='182608' scopes_data_offset='193296' scopes_pcs_offset='305000' dependencies_offset='335800' handler_table_offset='335808' nul_chk_table_offset='338160' oops_offset='193144' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator accessors2$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='5501' count='367' iicount='367' stamp='32.552'/>
    hotspot_pid19296.log:<nmethod compile_id='11056' compiler='C1' level='3' entry='0x00007f03aa227c40' size='340528' address='0x00007f03aa223610' relocation_offset='296' insts_offset='17968' stub_offset='182608' scopes_data_offset='193296' scopes_pcs_offset='305000' dependencies_offset='335800' handler_table_offset='335808' nul_chk_table_offset='338160' oops_offset='193144' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator accessors5$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='5501' count='370' iicount='370' stamp='32.699'/>
    hotspot_pid19296.log:<nmethod compile_id='11054' compiler='C1' level='3' entry='0x00007f03aa27ec40' size='340528' address='0x00007f03aa27a610' relocation_offset='296' insts_offset='17968' stub_offset='182608' scopes_data_offset='193296' scopes_pcs_offset='305000' dependencies_offset='335800' handler_table_offset='335808' nul_chk_table_offset='338160' oops_offset='193144' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator accessors3$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='5501' count='370' iicount='370' stamp='32.948'/>
    hotspot_pid19296.log:<nmethod compile_id='11055' compiler='C1' level='3' entry='0x00007f03aa2d1e80' size='340528' address='0x00007f03aa2cd850' relocation_offset='296' insts_offset='17968' stub_offset='182608' scopes_data_offset='193296' scopes_pcs_offset='305000' dependencies_offset='335800' handler_table_offset='335808' nul_chk_table_offset='338160' oops_offset='193144' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator accessors4$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='5501' count='369' iicount='369' stamp='33.083'/>
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203766948
  
    **[Test build #54599 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54599/consoleFull)** for PR 11984 at commit [`16cf602`](https://github.com/apache/spark/commit/16cf602b506c37c9e5cd9cbb48d577b7e66f14d4).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57995703
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -88,16 +92,21 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
             case array: ArrayType => classOf[ArrayColumnAccessor].getName
             case t: MapType => classOf[MapColumnAccessor].getName
           }
    -      ctx.addMutableState(accessorCls, accessorName, s"$accessorName = null;")
    -
    -      val createCode = dt match {
    -        case t if ctx.isPrimitiveType(dt) =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case NullType | StringType | BinaryType =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case other =>
    -          s"""$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder),
    -             (${dt.getClass.getName}) columnTypes[$index]);"""
    +      ctx.addMutableState(accessorCls, accessorName, "")
    +
    +      val createCode = {
    +        val shortCls = accessorCls.substring(accessorCls.lastIndexOf(".") + 1)
    +        dt match {
    +          case t if ctx.isPrimitiveType(dt) =>
    +            s"$accessorName = get${accessorClasses.getOrElseUpdate(accessorCls, shortCls)}($index);"
    --- End diff --
    
    When you directly call `ColumnAccessor.apply`, I think we don't need `getXXXColumnAccessor` anymore?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202065449
  
    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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202065450
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54287/
    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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203527913
  
    **[Test build #54538 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54538/consoleFull)** for PR 11984 at commit [`beb9840`](https://github.com/apache/spark/commit/beb98401d8435d300f1eb7dd475defb2a860f679).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203570442
  
    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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203816545
  
    **[Test build #54606 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54606/consoleFull)** for PR 11984 at commit [`a310bfc`](https://github.com/apache/spark/commit/a310bfc960f34f0ac6a61937a5519ac05ca52f94).
     * 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: [Spark-14138][SQL] Fix generated SpecificColum...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203822347
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54607/
    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: [Spark-14138][SQL] Fix generated SpecificColum...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202094157
  
    **[Test build #54288 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54288/consoleFull)** for PR 11984 at commit [`9346793`](https://github.com/apache/spark/commit/9346793cd2e0f0f58db503382aa51bd4ee2b39e4).
     * 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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57958304
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -88,16 +92,21 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
             case array: ArrayType => classOf[ArrayColumnAccessor].getName
             case t: MapType => classOf[MapColumnAccessor].getName
           }
    -      ctx.addMutableState(accessorCls, accessorName, s"$accessorName = null;")
    -
    -      val createCode = dt match {
    -        case t if ctx.isPrimitiveType(dt) =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case NullType | StringType | BinaryType =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case other =>
    -          s"""$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder),
    -             (${dt.getClass.getName}) columnTypes[$index]);"""
    +      ctx.addMutableState(accessorCls, accessorName, "")
    +
    +      val createCode = {
    +        val shortCls = accessorCls.substring(accessorCls.lastIndexOf(".") + 1)
    +        dt match {
    +          case t if ctx.isPrimitiveType(dt) =>
    +            s"$accessorName = get${accessorClasses.getOrElseUpdate(accessorCls, shortCls)}($index);"
    --- End diff --
    
    $accessorName = ($accessorCls) ColumnAccessor.apply(columnTypes[$index], ByteBuffer.wrap(batch.buffers()[columnIndexes[$index]]));


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57958641
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -88,16 +92,21 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
             case array: ArrayType => classOf[ArrayColumnAccessor].getName
             case t: MapType => classOf[MapColumnAccessor].getName
           }
    -      ctx.addMutableState(accessorCls, accessorName, s"$accessorName = null;")
    -
    -      val createCode = dt match {
    -        case t if ctx.isPrimitiveType(dt) =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case NullType | StringType | BinaryType =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case other =>
    -          s"""$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder),
    -             (${dt.getClass.getName}) columnTypes[$index]);"""
    +      ctx.addMutableState(accessorCls, accessorName, "")
    +
    +      val createCode = {
    +        val shortCls = accessorCls.substring(accessorCls.lastIndexOf(".") + 1)
    +        dt match {
    +          case t if ctx.isPrimitiveType(dt) =>
    +            s"$accessorName = get${accessorClasses.getOrElseUpdate(accessorCls, shortCls)}($index);"
    --- End diff --
    
    This may change the number of bytes per columns, you need to redo the calculation and test 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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202559168
  
    @davies , would it be possible to have a chance to look this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203816695
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54606/
    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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202014375
  
    **[Test build #54279 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54279/consoleFull)** for PR 11984 at commit [`60f6719`](https://github.com/apache/spark/commit/60f6719dbbaf772436b9576b401fda6f8e673100).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202002010
  
    **[Test build #54279 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54279/consoleFull)** for PR 11984 at commit [`60f6719`](https://github.com/apache/spark/commit/60f6719dbbaf772436b9576b401fda6f8e673100).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-201991965
  
    **[Test build #54276 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54276/consoleFull)** for PR 11984 at commit [`e56406e`](https://github.com/apache/spark/commit/e56406ed5173ae0c196705abcb8f7f28f0be0387).
     * This patch **fails Scala style 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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57995435
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -88,16 +92,21 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
             case array: ArrayType => classOf[ArrayColumnAccessor].getName
             case t: MapType => classOf[MapColumnAccessor].getName
           }
    -      ctx.addMutableState(accessorCls, accessorName, s"$accessorName = null;")
    -
    -      val createCode = dt match {
    -        case t if ctx.isPrimitiveType(dt) =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case NullType | StringType | BinaryType =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case other =>
    -          s"""$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder),
    -             (${dt.getClass.getName}) columnTypes[$index]);"""
    +      ctx.addMutableState(accessorCls, accessorName, "")
    +
    +      val createCode = {
    +        val shortCls = accessorCls.substring(accessorCls.lastIndexOf(".") + 1)
    +        dt match {
    +          case t if ctx.isPrimitiveType(dt) =>
    +            s"$accessorName = get${accessorClasses.getOrElseUpdate(accessorCls, shortCls)}($index);"
    --- End diff --
    
    I understand your motivation. I will revert my changes to avoid these complicity for reducing bytecode size.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r58010624
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -114,6 +117,42 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
           (createCode, extract + patch)
         }.unzip
     
    +    /*
    +     * 200 = 6000 bytes / 30 (up to 30 bytes per one call))
    +     * the maximum byte code size to be compiled for HotSpot is 8000.
    +     * We should keep less than 8000
    +     */
    +    val numberOfStatementsThreshold = 200
    +    val (initializerAccessorFuncs, initializerAccessorCalls, extractorFuncs, extractorCalls) =
    +      if (initializeAccessors.length <= numberOfStatementsThreshold) {
    +        ("", initializeAccessors.mkString("\n"), "", extractors.mkString("\n"))
    +      } else {
    +        val groupedAccessorsItr = initializeAccessors.grouped(numberOfStatementsThreshold)
    +        var groupedAccessorsLength = 0
    +        val groupedExtractorsItr = extractors.grouped(numberOfStatementsThreshold)
    +        var groupedExtractorsLength = 0
    +        (
    +          groupedAccessorsItr.zipWithIndex.map { case (body, i) =>
    +            groupedAccessorsLength += 1
    +            s"""
    +               |private void accessors$i() {
    +               |  ${body.mkString("\n")}
    +               |}
    +             """.stripMargin
    +          }.mkString(""),
    +          (0 to groupedAccessorsLength - 1).map { i => s"accessors$i();" }.mkString("\n"),
    +          groupedExtractorsItr.zipWithIndex.map { case (body, i) =>
    +            groupedExtractorsLength += 1
    --- End diff --
    
    this should be the same as groupedAccessorsLength, right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202014652
  
    **[Test build #54277 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54277/consoleFull)** for PR 11984 at commit [`60f6719`](https://github.com/apache/spark/commit/60f6719dbbaf772436b9576b401fda6f8e673100).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-201993844
  
    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 pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203570198
  
    **[Test build #54538 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54538/consoleFull)** for PR 11984 at commit [`beb9840`](https://github.com/apache/spark/commit/beb98401d8435d300f1eb7dd475defb2a860f679).
     * 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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57928597
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -114,6 +123,57 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
           (createCode, extract + patch)
         }.unzip
     
    +    val accessorCode = accessorClasses.map { case (accessorCls, shortAccCls) =>
    +      s"""
    +         private $accessorCls get${shortAccCls}(int idx) {
    +           byte[] buffer = batch.buffers()[columnIndexes[idx]];
    +           return new $accessorCls(ByteBuffer.wrap(buffer).order(nativeOrder));
    +         }
    +      """
    +    }
    +    val accessorStructCode = accessorStructClasses.map {
    +      case ((accessorCls, dtCls), (shortAccCls, shortDTCls)) =>
    +        s"""
    +           private $accessorCls get${shortAccCls}_${shortDTCls}(int idx) {
    +             byte[] buffer = batch.buffers()[columnIndexes[idx]];
    +             return new $accessorCls(ByteBuffer.wrap(buffer).order(nativeOrder),
    +               (${dtCls}) columnTypes[idx]);
    +           }
    +        """
    +    }
    +
    +    /* 4000 = 64000 bytes / 16 (up to 16 bytes per one call)) */
    +    val numberOfStatementsThreshold = 4000
    --- End diff --
    
    I confirmed this by running the following program:
    
    ```
        val df = sc.parallelize(1 to 100).toDF()
        val aggr = {1 to 3000}.map(colnum => avg(df.col("_1")).as(s"col_$colnum"))
        val res = df.groupBy("_1").agg(count("_1"), aggr: _*).cache()
        var i = 0
        for (i <- 0 to 110)
          res.collect()
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203816692
  
    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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-204099804
  
    **[Test build #54659 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54659/consoleFull)** for PR 11984 at commit [`3a05ddf`](https://github.com/apache/spark/commit/3a05ddf3919b267e5ba593d82949b9b409060f5a).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-204085643
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202052493
  
    **[Test build #54287 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54287/consoleFull)** for PR 11984 at commit [`226bad5`](https://github.com/apache/spark/commit/226bad5ee0c0253cad958fa812fd5b274b8c6be0).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203789700
  
    **[Test build #54606 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54606/consoleFull)** for PR 11984 at commit [`a310bfc`](https://github.com/apache/spark/commit/a310bfc960f34f0ac6a61937a5519ac05ca52f94).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203822187
  
    **[Test build #54607 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54607/consoleFull)** for PR 11984 at commit [`c1acf82`](https://github.com/apache/spark/commit/c1acf825cc0dfdefc1074ad4ce3309743a3240b5).
     * 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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r58010740
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -114,6 +117,42 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
           (createCode, extract + patch)
         }.unzip
     
    +    /*
    +     * 200 = 6000 bytes / 30 (up to 30 bytes per one call))
    +     * the maximum byte code size to be compiled for HotSpot is 8000.
    +     * We should keep less than 8000
    +     */
    +    val numberOfStatementsThreshold = 200
    +    val (initializerAccessorFuncs, initializerAccessorCalls, extractorFuncs, extractorCalls) =
    +      if (initializeAccessors.length <= numberOfStatementsThreshold) {
    --- End diff --
    
    we could always put them in groups


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202002007
  
    **[Test build #54277 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54277/consoleFull)** for PR 11984 at commit [`60f6719`](https://github.com/apache/spark/commit/60f6719dbbaf772436b9576b401fda6f8e673100).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r58035019
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -114,6 +117,42 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
           (createCode, extract + patch)
         }.unzip
     
    +    /*
    +     * 200 = 6000 bytes / 30 (up to 30 bytes per one call))
    +     * the maximum byte code size to be compiled for HotSpot is 8000.
    +     * We should keep less than 8000
    +     */
    +    val numberOfStatementsThreshold = 200
    +    val (initializerAccessorFuncs, initializerAccessorCalls, extractorFuncs, extractorCalls) =
    +      if (initializeAccessors.length <= numberOfStatementsThreshold) {
    --- End diff --
    
    Yes we could do. But, (another PR)[https://github.com/apache/spark/pull/7076#issuecomment-122176653] intentionally avoid putting them into a group if they can be put into one group.
    Which is better?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57994632
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -88,16 +92,21 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
             case array: ArrayType => classOf[ArrayColumnAccessor].getName
             case t: MapType => classOf[MapColumnAccessor].getName
           }
    -      ctx.addMutableState(accessorCls, accessorName, s"$accessorName = null;")
    -
    -      val createCode = dt match {
    -        case t if ctx.isPrimitiveType(dt) =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case NullType | StringType | BinaryType =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case other =>
    -          s"""$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder),
    -             (${dt.getClass.getName}) columnTypes[$index]);"""
    +      ctx.addMutableState(accessorCls, accessorName, "")
    +
    +      val createCode = {
    +        val shortCls = accessorCls.substring(accessorCls.lastIndexOf(".") + 1)
    +        dt match {
    +          case t if ctx.isPrimitiveType(dt) =>
    +            s"$accessorName = get${accessorClasses.getOrElseUpdate(accessorCls, shortCls)}($index);"
    --- End diff --
    
    It's better to call `ColumnAccessor.apply()` to avoid these complicity.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-201988732
  
    **[Test build #54274 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54274/consoleFull)** for PR 11984 at commit [`fea2a52`](https://github.com/apache/spark/commit/fea2a524bbd5b1d0d285e02e6eda590d1f7d67e3).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203768128
  
    **[Test build #54599 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54599/consoleFull)** for PR 11984 at commit [`16cf602`](https://github.com/apache/spark/commit/16cf602b506c37c9e5cd9cbb48d577b7e66f14d4).
     * This patch **fails Scala style 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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202014664
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57994067
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -88,16 +92,21 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
             case array: ArrayType => classOf[ArrayColumnAccessor].getName
             case t: MapType => classOf[MapColumnAccessor].getName
           }
    -      ctx.addMutableState(accessorCls, accessorName, s"$accessorName = null;")
    -
    -      val createCode = dt match {
    -        case t if ctx.isPrimitiveType(dt) =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case NullType | StringType | BinaryType =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case other =>
    -          s"""$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder),
    -             (${dt.getClass.getName}) columnTypes[$index]);"""
    +      ctx.addMutableState(accessorCls, accessorName, "")
    +
    +      val createCode = {
    +        val shortCls = accessorCls.substring(accessorCls.lastIndexOf(".") + 1)
    +        dt match {
    +          case t if ctx.isPrimitiveType(dt) =>
    +            s"$accessorName = get${accessorClasses.getOrElseUpdate(accessorCls, shortCls)}($index);"
    --- End diff --
    
    For example, a generated method `getIntColumnAccessor()` still calls ```ColumnAccessor.apply()``.
    
    Do you want to directly call  ```ColumnAccessor.apply()`` from a method ```hasNext()``` instead of calling it thru `getIntColumnAccessor`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202094244
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54288/
    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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57931156
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -114,6 +123,61 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
           (createCode, extract + patch)
         }.unzip
     
    +    val accessorCode = accessorClasses.map { case (accessorCls, shortAccCls) =>
    +      s"""
    +         private $accessorCls get${shortAccCls}(int idx) {
    +           byte[] buffer = batch.buffers()[columnIndexes[idx]];
    +           return new $accessorCls(ByteBuffer.wrap(buffer).order(nativeOrder));
    +         }
    +      """
    +    }
    +    val accessorStructCode = accessorStructClasses.map {
    +      case ((accessorCls, dtCls), (shortAccCls, shortDTCls)) =>
    +        s"""
    +           private $accessorCls get${shortAccCls}_${shortDTCls}(int idx) {
    +             byte[] buffer = batch.buffers()[columnIndexes[idx]];
    +             return new $accessorCls(ByteBuffer.wrap(buffer).order(nativeOrder),
    +               (${dtCls}) columnTypes[idx]);
    +           }
    +        """
    +    }
    +
    +    /*
    +     * 500 = 7500 bytes / 15 (up to 15 bytes per one call))
    +     * the maximum byte code size to be compiled for HotSpot is 8000.
    +     * We should keep less than 8000
    +     */
    +    val numberOfStatementsThreshold = 500
    --- End diff --
    
    To be safe, we could decrease this to 400.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202014392
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-204084945
  
    **[Test build #54654 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54654/consoleFull)** for PR 11984 at commit [`60cebd5`](https://github.com/apache/spark/commit/60cebd5ad0913699d0a683912e93d5334ce211ab).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203768142
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202065412
  
    **[Test build #54287 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54287/consoleFull)** for PR 11984 at commit [`226bad5`](https://github.com/apache/spark/commit/226bad5ee0c0253cad958fa812fd5b274b8c6be0).
     * 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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-201991966
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203792626
  
    Again, I confirmed whether these method are compiled.
    
    ```
    hotspot_pid30419.log:<nmethod compile_id='8066' compiler='C1' level='2' entry='0x00007fa2fa4d5a40' size='2480' address='0x00007fa2fa4d5850' relocation_offset='296' insts_offset='496' stub_offset='1328' scopes_data_offset='1664' scopes_pcs_offset='2008' dependencies_offset='2408' nul_chk_table_offset='2416' oops_offset='1624' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator hasNext ()Z' bytes='116' 
    ```
    
    ```
    hotspot_pid30419.log:<nmethod compile_id='10941' compiler='C1' level='3' entry='0x00007fa2fad77b20' size='3552' address='0x00007fa2fad77910' relocation_offset='296' insts_offset='528' stub_offset='2288' scopes_data_offset='2672' scopes_pcs_offset='3104' dependencies_offset='3488' nul_chk_table_offset='3496' oops_offset='2584' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator next ()Lorg/apache/spark/sql/catalyst/InternalRow;' bytes='84' count='638' iicount='638' stamp='13.693'/>
    ```
    
    ```
    hotspot_pid30419.log:<nmethod compile_id='10335' compiler='C1' level='3' entry='0x00007fa2fa9b8fe0' size='412608' address='0x00007fa2fa9b3e10' relocation_offset='296' insts_offset='20944' stub_offset='240080' scopes_data_offset='251056' scopes_pcs_offset='361672' dependencies_offset='404712' handler_table_offset='404720' nul_chk_table_offset='407792' oops_offset='250888' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator accessors0$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='5667' count='333' iicount='333' stamp='11.349'/>
    hotspot_pid30419.log:<nmethod compile_id='10336' compiler='C1' level='3' entry='0x00007fa2f9ffe220' size='9792' address='0x00007fa2f9ffdf50' relocation_offset='296' insts_offset='720' stub_offset='5840' scopes_data_offset='6256' scopes_pcs_offset='8760' dependencies_offset='9624' handler_table_offset='9632' nul_chk_table_offset='9728' oops_offset='6120' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator accessors5$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='59' count='340' iicount='340' stamp='11.369'/>
    hotspot_pid30419.log:<nmethod compile_id='10340' compiler='C1' level='3' entry='0x00007fa2faa1dbe0' size='412848' address='0x00007fa2faa18a10' relocation_offset='296' insts_offset='20944' stub_offset='240336' scopes_data_offset='251280' scopes_pcs_offset='361912' dependencies_offset='404952' handler_table_offset='404960' nul_chk_table_offset='408032' oops_offset='251144' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator accessors1$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='5801' count='368' iicount='368' stamp='11.481'/>
    hotspot_pid30419.log:<nmethod compile_id='10341' compiler='C1' level='3' entry='0x00007fa2faa828a0' size='412848' address='0x00007fa2faa7d6d0' relocation_offset='296' insts_offset='20944' stub_offset='240336' scopes_data_offset='251280' scopes_pcs_offset='361912' dependencies_offset='404952' handler_table_offset='404960' nul_chk_table_offset='408032' oops_offset='251144' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator accessors2$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='5801' count='368' iicount='368' stamp='11.634'/>
    hotspot_pid30419.log:<nmethod compile_id='10601' compiler='C1' level='3' entry='0x00007fa2fab22a60' size='412848' address='0x00007fa2fab1d890' relocation_offset='296' insts_offset='20944' stub_offset='240336' scopes_data_offset='251280' scopes_pcs_offset='361912' dependencies_offset='404952' handler_table_offset='404960' nul_chk_table_offset='408032' oops_offset='251144' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator accessors4$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='5801' count='459' iicount='459' stamp='12.269'/>
    hotspot_pid30419.log:<nmethod compile_id='10602' compiler='C1' level='3' entry='0x00007fa2fab89360' size='412848' address='0x00007fa2fab84190' relocation_offset='296' insts_offset='20944' stub_offset='240336' scopes_data_offset='251280' scopes_pcs_offset='361912' dependencies_offset='404952' handler_table_offset='404960' nul_chk_table_offset='408032' oops_offset='251144' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator accessors3$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='5801' count='465' iicount='465' stamp='12.353'/>
    ```
    
    ```
    hotspot_pid30419.log:<nmethod compile_id='10548' compiler='C1' level='3' entry='0x00007fa2fa477d60' size='1696' address='0x00007fa2fa477bd0' relocation_offset='296' insts_offset='400' stub_offset='1136' scopes_data_offset='1360' scopes_pcs_offset='1496' dependencies_offset='1656' nul_chk_table_offset='1664' oops_offset='1320' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator extractors5$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='29' count='385' iicount='385' stamp='11.913'/>
    hotspot_pid30419.log:<nmethod compile_id='10550' compiler='C1' level='3' entry='0x00007fa2fa23f440' size='90160' address='0x00007fa2fa23df90' relocation_offset='296' insts_offset='5296' stub_offset='62256' scopes_data_offset='65648' scopes_pcs_offset='78872' dependencies_offset='88536' nul_chk_table_offset='88544' oops_offset='65608' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator extractors2$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='2801' count='388' iicount='388' stamp='11.935'/>
    hotspot_pid30419.log:<nmethod compile_id='10552' compiler='C1' level='3' entry='0x00007fa2faae3840' size='90160' address='0x00007fa2faae2390' relocation_offset='296' insts_offset='5296' stub_offset='62256' scopes_data_offset='65648' scopes_pcs_offset='78872' dependencies_offset='88536' nul_chk_table_offset='88544' oops_offset='65608' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator extractors1$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='2801' count='395' iicount='395' stamp='11.958'/>
    hotspot_pid30419.log:<nmethod compile_id='10553' compiler='C1' level='3' entry='0x00007fa2faaf9880' size='90160' address='0x00007fa2faaf83d0' relocation_offset='296' insts_offset='5296' stub_offset='62256' scopes_data_offset='65648' scopes_pcs_offset='78872' dependencies_offset='88536' nul_chk_table_offset='88544' oops_offset='65608' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator extractors3$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='2801' count='398' iicount='398' stamp='11.981'/>
    hotspot_pid30419.log:<nmethod compile_id='10786' compiler='C1' level='3' entry='0x00007fa2facdf700' size='90160' address='0x00007fa2facde250' relocation_offset='296' insts_offset='5296' stub_offset='62256' scopes_data_offset='65648' scopes_pcs_offset='78872' dependencies_offset='88536' nul_chk_table_offset='88544' oops_offset='65608' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator extractors4$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='2801' count='542' iicount='542' stamp='12.974'/>
    hotspot_pid30419.log:<nmethod compile_id='10943' compiler='C1' level='3' entry='0x00007fa2fad7e140' size='90064' address='0x00007fa2fad7cc90' relocation_offset='296' insts_offset='5296' stub_offset='62256' scopes_data_offset='65664' scopes_pcs_offset='78776' dependencies_offset='88440' nul_chk_table_offset='88448' oops_offset='65608' method='org/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator extractors0$ (Lorg/apache/spark/sql/catalyst/expressions/GeneratedClass$SpecificColumnarIterator;)V' bytes='2667' count='647' iicount='647' stamp='13.722'/>
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57931259
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -114,6 +123,61 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
           (createCode, extract + patch)
         }.unzip
     
    +    val accessorCode = accessorClasses.map { case (accessorCls, shortAccCls) =>
    +      s"""
    +         private $accessorCls get${shortAccCls}(int idx) {
    +           byte[] buffer = batch.buffers()[columnIndexes[idx]];
    +           return new $accessorCls(ByteBuffer.wrap(buffer).order(nativeOrder));
    +         }
    +      """
    +    }
    +    val accessorStructCode = accessorStructClasses.map {
    +      case ((accessorCls, dtCls), (shortAccCls, shortDTCls)) =>
    +        s"""
    +           private $accessorCls get${shortAccCls}_${shortDTCls}(int idx) {
    +             byte[] buffer = batch.buffers()[columnIndexes[idx]];
    +             return new $accessorCls(ByteBuffer.wrap(buffer).order(nativeOrder),
    +               (${dtCls}) columnTypes[idx]);
    +           }
    +        """
    +    }
    +
    +    /*
    +     * 500 = 7500 bytes / 15 (up to 15 bytes per one call))
    +     * the maximum byte code size to be compiled for HotSpot is 8000.
    +     * We should keep less than 8000
    +     */
    +    val numberOfStatementsThreshold = 500
    +    val (initializerAccessorFuncs, initializerAccessorCalls, extractorFuncs, extractorCalls) =
    +      if (initializeAccessors.length <= numberOfStatementsThreshold) {
    +        ("", initializeAccessors.mkString("\n"), "", extractors.mkString("\n"))
    +      } else {
    +        val groupedAccessorsItr = initializeAccessors.grouped(numberOfStatementsThreshold)
    +        var groupedAccessorsLength = 0
    --- End diff --
    
    Should this just be `groupedAccessorsItr.size()` ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-201991876
  
    **[Test build #54276 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54276/consoleFull)** for PR 11984 at commit [`e56406e`](https://github.com/apache/spark/commit/e56406ed5173ae0c196705abcb8f7f28f0be0387).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202071503
  
    **[Test build #54288 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54288/consoleFull)** for PR 11984 at commit [`9346793`](https://github.com/apache/spark/commit/9346793cd2e0f0f58db503382aa51bd4ee2b39e4).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57955113
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -88,16 +92,21 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
             case array: ArrayType => classOf[ArrayColumnAccessor].getName
             case t: MapType => classOf[MapColumnAccessor].getName
           }
    -      ctx.addMutableState(accessorCls, accessorName, s"$accessorName = null;")
    -
    -      val createCode = dt match {
    -        case t if ctx.isPrimitiveType(dt) =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case NullType | StringType | BinaryType =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case other =>
    -          s"""$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder),
    -             (${dt.getClass.getName}) columnTypes[$index]);"""
    +      ctx.addMutableState(accessorCls, accessorName, "")
    +
    +      val createCode = {
    +        val shortCls = accessorCls.substring(accessorCls.lastIndexOf(".") + 1)
    +        dt match {
    +          case t if ctx.isPrimitiveType(dt) =>
    +            s"$accessorName = get${accessorClasses.getOrElseUpdate(accessorCls, shortCls)}($index);"
    --- End diff --
    
    Could you please explain this in detail? I cannot understand your suggestion. Should this call we put ```ColumnAccessor.apply()`` in this method or generated java 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 pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203794122
  
    **[Test build #54607 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54607/consoleFull)** for PR 11984 at commit [`c1acf82`](https://github.com/apache/spark/commit/c1acf825cc0dfdefc1074ad4ce3309743a3240b5).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-202094242
  
    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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57624829
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -114,6 +123,57 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
           (createCode, extract + patch)
         }.unzip
     
    +    val accessorCode = accessorClasses.map { case (accessorCls, shortAccCls) =>
    +      s"""
    +         private $accessorCls get${shortAccCls}(int idx) {
    +           byte[] buffer = batch.buffers()[columnIndexes[idx]];
    +           return new $accessorCls(ByteBuffer.wrap(buffer).order(nativeOrder));
    +         }
    +      """
    +    }
    +    val accessorStructCode = accessorStructClasses.map {
    +      case ((accessorCls, dtCls), (shortAccCls, shortDTCls)) =>
    +        s"""
    +           private $accessorCls get${shortAccCls}_${shortDTCls}(int idx) {
    +             byte[] buffer = batch.buffers()[columnIndexes[idx]];
    +             return new $accessorCls(ByteBuffer.wrap(buffer).order(nativeOrder),
    +               (${dtCls}) columnTypes[idx]);
    +           }
    +        """
    +    }
    +
    +    /* 4000 = 64000 bytes / 16 (up to 16 bytes per one call)) */
    +    val numberOfStatementsThreshold = 4000
    --- End diff --
    
    A java method will not be JITted if it's over 8K, so we may need smaller threshold here. Could you also manual check that (for performance)?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-204085636
  
    **[Test build #54654 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54654/consoleFull)** for PR 11984 at commit [`60cebd5`](https://github.com/apache/spark/commit/60cebd5ad0913699d0a683912e93d5334ce211ab).
     * This patch **fails Scala style 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: [Spark-14138][SQL] Fix generated SpecificColum...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57930675
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -88,16 +92,21 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
             case array: ArrayType => classOf[ArrayColumnAccessor].getName
             case t: MapType => classOf[MapColumnAccessor].getName
           }
    -      ctx.addMutableState(accessorCls, accessorName, s"$accessorName = null;")
    -
    -      val createCode = dt match {
    -        case t if ctx.isPrimitiveType(dt) =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case NullType | StringType | BinaryType =>
    -          s"$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder));"
    -        case other =>
    -          s"""$accessorName = new $accessorCls(ByteBuffer.wrap(buffers[$index]).order(nativeOrder),
    -             (${dt.getClass.getName}) columnTypes[$index]);"""
    +      ctx.addMutableState(accessorCls, accessorName, "")
    +
    +      val createCode = {
    +        val shortCls = accessorCls.substring(accessorCls.lastIndexOf(".") + 1)
    +        dt match {
    +          case t if ctx.isPrimitiveType(dt) =>
    +            s"$accessorName = get${accessorClasses.getOrElseUpdate(accessorCls, shortCls)}($index);"
    --- End diff --
    
    I think we should just call `ColumnAccessor.apply()` (making it accessible in generated java 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 pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-201989665
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#issuecomment-203822344
  
    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: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r58010571
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -114,6 +117,42 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
           (createCode, extract + patch)
         }.unzip
     
    +    /*
    +     * 200 = 6000 bytes / 30 (up to 30 bytes per one call))
    +     * the maximum byte code size to be compiled for HotSpot is 8000.
    +     * We should keep less than 8000
    +     */
    +    val numberOfStatementsThreshold = 200
    +    val (initializerAccessorFuncs, initializerAccessorCalls, extractorFuncs, extractorCalls) =
    --- End diff --
    
    we could use `ctx.addFunction` to simplify these 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57933130
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -114,6 +123,57 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
           (createCode, extract + patch)
         }.unzip
     
    +    val accessorCode = accessorClasses.map { case (accessorCls, shortAccCls) =>
    +      s"""
    +         private $accessorCls get${shortAccCls}(int idx) {
    +           byte[] buffer = batch.buffers()[columnIndexes[idx]];
    +           return new $accessorCls(ByteBuffer.wrap(buffer).order(nativeOrder));
    +         }
    +      """
    +    }
    +    val accessorStructCode = accessorStructClasses.map {
    +      case ((accessorCls, dtCls), (shortAccCls, shortDTCls)) =>
    +        s"""
    +           private $accessorCls get${shortAccCls}_${shortDTCls}(int idx) {
    +             byte[] buffer = batch.buffers()[columnIndexes[idx]];
    +             return new $accessorCls(ByteBuffer.wrap(buffer).order(nativeOrder),
    +               (${dtCls}) columnTypes[idx]);
    +           }
    +        """
    +    }
    +
    +    /* 4000 = 64000 bytes / 16 (up to 16 bytes per one call)) */
    +    val numberOfStatementsThreshold = 4000
    --- End diff --
    
    They could be jitted after decrease it to 500, right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [Spark-14138][SQL] Fix generated SpecificColum...

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

    https://github.com/apache/spark/pull/11984#discussion_r57931820
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/GenerateColumnAccessor.scala ---
    @@ -114,6 +123,61 @@ object GenerateColumnAccessor extends CodeGenerator[Seq[DataType], ColumnarItera
           (createCode, extract + patch)
         }.unzip
     
    +    val accessorCode = accessorClasses.map { case (accessorCls, shortAccCls) =>
    +      s"""
    +         private $accessorCls get${shortAccCls}(int idx) {
    --- End diff --
    
    ctx.addFunction ?


---
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