You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by paul-rogers <gi...@git.apache.org> on 2016/12/22 03:02:58 UTC

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

GitHub user paul-rogers opened a pull request:

    https://github.com/apache/drill/pull/704

    DRILL-5125: Provide option to use generic code for sv remover

    Performance tests showed that, for queries with a large number of
    columns, it is faster to use a \u201cgeneric\u201d implementation of the
    selection vector remover \u201ccopier\u201d than to use a generated version.
    
    This PR provides "generic" versions of the SV2 and SV4 copiers
    used by the selection vector remover. The generic forms are
    enabled using a new boot-time config parameter that is disabled
    by default (preserving the traditional generated code.)
    
    The generic form relies on a "virtual function" (really, just a
    plain Java function) defined in the base ValueVector class and
    implemented by each concrete vector: both the pre-defined and
    generated forms. This form "does the right thing" for the copy
    operation so that we don't need to generate code just to handle
    the method dispatch operation (which Java does quite well on its
    own.)
    
    A unit tests validates that the generic form works by runing
    the existing SV remover tests with the generic option turned on.
    
    See the DRILL-5125 for details.
    
    Add test

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

    $ git pull https://github.com/paul-rogers/drill DRILL-5125

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

    https://github.com/apache/drill/pull/704.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 #704
    
----
commit ba3a38a403b140149d1605decefae088765ead56
Author: Paul Rogers <pr...@maprtech.com>
Date:   2016-12-12T18:06:43Z

    DRILL-5125: Provide option to use generic code for sv remover
    
    Performance tests showed that, for queries with a large number of
    columns, it is faster to use a \u201cgeneric\u201d implementation of the
    selection vector remover \u201ccopier\u201d than to use a generated version.
    
    This PR provides "generic" versions of the SV2 and SV4 copiers
    used by the selection vector remover. The generic forms are
    enabled using a new boot-time config parameter that is disabled
    by default (preserving the traditional generated code.)
    
    The generic form relies on a "virtual function" (really, just a
    plain Java function) defined in the base ValueVector class and
    implemented by each concrete vector: both the pre-defined and
    generated forms. This form "does the right thing" for the copy
    operation so that we don't need to generate code just to handle
    the method dispatch operation (which Java does quite well on its
    own.)
    
    A unit tests validates that the generic form works by runing
    the existing SV remover tests with the generic option turned on.
    
    See the DRILL-5125 for details.
    
    Add test

----


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

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

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

    https://github.com/apache/drill/pull/704#discussion_r109051237
  
    --- Diff: exec/vector/src/main/codegen/templates/FixedValueVectors.java ---
    @@ -281,6 +282,11 @@ public void copyFromSafe(int fromIndex, int thisIndex, ${minor.class}Vector from
         copyFrom(fromIndex, thisIndex, from);
       }
     
    +  @Override
    +  public void copyEntry(int toIndex, ValueVector from, int fromIndex) {
    +    ((${minor.class}Vector) from).data.getBytes(fromIndex * ${type.width}, data, toIndex * ${type.width}, ${type.width});
    --- End diff --
    
    What if the dest value vector does not have enough big DrillBuf?  toIndex >= getValueCapacity() ? 


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

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

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

    https://github.com/apache/drill/pull/704


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

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

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

    https://github.com/apache/drill/pull/704#discussion_r109051934
  
    --- Diff: exec/vector/src/main/codegen/templates/NullableValueVectors.java ---
    @@ -374,6 +374,19 @@ public void copyFromSafe(int fromIndex, int thisIndex, Nullable${minor.class}Vec
         values.copyFromSafe(fromIndex, thisIndex, from.values);
       }
     
    +  @Override
    +  public void copyEntry(int toIndex, ValueVector from, int fromIndex) {
    +    Nullable${minor.class}Vector fromVector = (Nullable${minor.class}Vector) from;
    +    <#if type.major == "VarLen">
    +
    +    // This method is to be called only for loading the vector
    --- End diff --
    
    Will it make sense to put this comment as part of java doc for this copyEntry() method, so that people knows this method should be called sequentially ? 


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

[GitHub] drill issue #704: DRILL-5125: Provide option to use generic code for sv remo...

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

    https://github.com/apache/drill/pull/704
  
    Let's do more research on this one. Closing for now since that research is not a priority at the moment.


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

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

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

    https://github.com/apache/drill/pull/704#discussion_r97992610
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/GenericSV4Copier.java ---
    @@ -0,0 +1,67 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.drill.exec.physical.impl.svremover;
    +
    +import org.apache.drill.exec.ops.FragmentContext;
    +import org.apache.drill.exec.record.RecordBatch;
    +import org.apache.drill.exec.record.VectorWrapper;
    +import org.apache.drill.exec.vector.ValueVector;
    +
    +/**
    + * Generic selection vector 4 copier implementation that can
    + * be used in place of the generated version. Relies on a
    + * virtual function in each value vector to choose the proper
    + * implementation. Tests suggest that this version performs
    + * better than the generated version for queries with many columns.
    + */
    +
    +public class GenericSV4Copier extends CopierTemplate4 {
    +
    +  private ValueVector[] vvOut;
    +  private ValueVector[][] vvIn;
    +
    +  @SuppressWarnings("unused")
    +  @Override
    +  public void doSetup(FragmentContext context, RecordBatch incoming,
    +      RecordBatch outgoing) {
    --- End diff --
    
    fix alignment


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

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

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

    https://github.com/apache/drill/pull/704#discussion_r97992444
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/GenericSV2Copier.java ---
    @@ -0,0 +1,65 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.drill.exec.physical.impl.svremover;
    +
    +import org.apache.drill.exec.ops.FragmentContext;
    +import org.apache.drill.exec.record.RecordBatch;
    +import org.apache.drill.exec.record.VectorWrapper;
    +import org.apache.drill.exec.vector.ValueVector;
    +
    +/**
    + * Generic selection vector 2 copier implementation that can
    + * be used in place of the generated version. Relies on a
    + * virtual function in each value vector to choose the proper
    + * implementation. Tests suggest that this version performs
    + * better than the generated version for queries with many columns.
    + */
    +
    +public class GenericSV2Copier extends CopierTemplate2 {
    +
    +  private ValueVector[] vvOut;
    +  private ValueVector[] vvIn;
    +
    +  @SuppressWarnings("unused")
    +  @Override
    +  public void doSetup(FragmentContext context, RecordBatch incoming,
    +      RecordBatch outgoing) {
    --- End diff --
    
    fix alignment


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

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

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

    https://github.com/apache/drill/pull/704#discussion_r109052570
  
    --- Diff: exec/vector/src/main/codegen/templates/FixedValueVectors.java ---
    @@ -281,6 +282,11 @@ public void copyFromSafe(int fromIndex, int thisIndex, ${minor.class}Vector from
         copyFrom(fromIndex, thisIndex, from);
       }
     
    +  @Override
    +  public void copyEntry(int toIndex, ValueVector from, int fromIndex) {
    --- End diff --
    
    From the name "copyEntry", it's not clear this new method will provide a semantics like the existing method copyFrom(), or copyFromSafe(). 
    
    In value vector code, Drill's convention (in my understanding) is xxxSafe() means value vector takes care of re-alloc() itself, in case of running out of buffer, while xxx() does not have this guarantee. I think we had better follow this convention in new added method. 
      


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

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

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

    https://github.com/apache/drill/pull/704#discussion_r98299585
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/svremover/TestSVRemover.java ---
    @@ -34,4 +38,33 @@ public void testSVRWithNoFilter() throws Exception {
         int numOutputRecords = testPhysical(getFile("remover/sv_with_no_filter.json"));
         assertEquals(100, numOutputRecords);
       }
    +
    +  /**
    +   * Test the generic version of the selection vector remover copier
    +   * class. The code uses the traditional generated version by default.
    +   * This test sets the option to use the generic version, then runs
    +   * a query that exercises that version.
    +   * <p>
    +   * Note that the tests here exercise only the SV2 version of the
    +   * selection remover; no tests exist for the SV4 version.
    +   */
    +
    +  // TODO: Add an SV4 test once the improved mock data generator
    +  // is available.
    +
    +  @Test
    +  public void testGenericCopier() throws Exception {
    +    // TODO: replace this with new setup once revised test framework
    +    // is available.
    +    Properties config = new Properties( );
    +    config.put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, "false");
    +    config.put(ExecConstants.HTTP_ENABLE, "false");
    +    config.put(ExecConstants.REMOVER_ENABLE_GENERIC_COPIER, "true");
    +    updateTestCluster(1, DrillConfig.create(config));
    +
    +    int numOutputRecords = testPhysical(getFile("remover/test1.json"));
    +    assertEquals(50, numOutputRecords);
    +    numOutputRecords = testPhysical(getFile("remover/sv_with_no_filter.json"));
    +    assertEquals(100, numOutputRecords);
    +  }
     }
    --- End diff --
    
    Do these tests cover all the value vector types ?


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

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

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

    https://github.com/apache/drill/pull/704#discussion_r106789976
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/GenericSV4Copier.java ---
    @@ -0,0 +1,67 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.drill.exec.physical.impl.svremover;
    +
    +import org.apache.drill.exec.ops.FragmentContext;
    +import org.apache.drill.exec.record.RecordBatch;
    +import org.apache.drill.exec.record.VectorWrapper;
    +import org.apache.drill.exec.vector.ValueVector;
    +
    +/**
    + * Generic selection vector 4 copier implementation that can
    + * be used in place of the generated version. Relies on a
    + * virtual function in each value vector to choose the proper
    + * implementation. Tests suggest that this version performs
    + * better than the generated version for queries with many columns.
    + */
    +
    +public class GenericSV4Copier extends CopierTemplate4 {
    +
    +  private ValueVector[] vvOut;
    +  private ValueVector[][] vvIn;
    +
    +  @SuppressWarnings("unused")
    +  @Override
    +  public void doSetup(FragmentContext context, RecordBatch incoming,
    +      RecordBatch outgoing) {
    --- End diff --
    
    Fixed


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

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

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

    https://github.com/apache/drill/pull/704#discussion_r106790613
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/svremover/TestSVRemover.java ---
    @@ -34,4 +38,33 @@ public void testSVRWithNoFilter() throws Exception {
         int numOutputRecords = testPhysical(getFile("remover/sv_with_no_filter.json"));
         assertEquals(100, numOutputRecords);
       }
    +
    +  /**
    +   * Test the generic version of the selection vector remover copier
    +   * class. The code uses the traditional generated version by default.
    +   * This test sets the option to use the generic version, then runs
    +   * a query that exercises that version.
    +   * <p>
    +   * Note that the tests here exercise only the SV2 version of the
    +   * selection remover; no tests exist for the SV4 version.
    +   */
    +
    +  // TODO: Add an SV4 test once the improved mock data generator
    +  // is available.
    +
    +  @Test
    +  public void testGenericCopier() throws Exception {
    +    // TODO: replace this with new setup once revised test framework
    +    // is available.
    +    Properties config = new Properties( );
    +    config.put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, "false");
    +    config.put(ExecConstants.HTTP_ENABLE, "false");
    +    config.put(ExecConstants.REMOVER_ENABLE_GENERIC_COPIER, "true");
    +    updateTestCluster(1, DrillConfig.create(config));
    +
    +    int numOutputRecords = testPhysical(getFile("remover/test1.json"));
    +    assertEquals(50, numOutputRecords);
    +    numOutputRecords = testPhysical(getFile("remover/sv_with_no_filter.json"));
    +    assertEquals(100, numOutputRecords);
    +  }
     }
    --- End diff --
    
    No, which is why the feature is off by default. More work is needed to provide a reliable way to generate data for all (supported) value vector types.


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

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

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

    https://github.com/apache/drill/pull/704#discussion_r109058076
  
    --- Diff: exec/vector/src/main/codegen/templates/FixedValueVectors.java ---
    @@ -281,6 +282,11 @@ public void copyFromSafe(int fromIndex, int thisIndex, ${minor.class}Vector from
         copyFrom(fromIndex, thisIndex, from);
       }
     
    +  @Override
    +  public void copyEntry(int toIndex, ValueVector from, int fromIndex) {
    +    ((${minor.class}Vector) from).data.getBytes(fromIndex * ${type.width}, data, toIndex * ${type.width}, ${type.width});
    --- End diff --
    
    Throughout all the value vector classes, this new "copyEntry" method has to explicitly downcast from interface ValueVector to a subclass. Such cast happens for each value copied. I' not sure how performance overhead it could possibly incur : JVM has to check the input type, and raise ClassCastException in case of error. 
    
    Compared with the existing method,  I think cast only happens in doSetup(), which is called for each new schema. 
    
    Also, "copyEntry" uses data.getBytes(), while copyFromSafe() uses setInt(...,   from.data.getInt()).  I'm not sure the performance difference comes from the difference of generated code vs generic class, or the difference of the underneath implementation.
    
     


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

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

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

    https://github.com/apache/drill/pull/704#discussion_r109051708
  
    --- Diff: exec/vector/src/main/codegen/templates/NullableValueVectors.java ---
    @@ -374,6 +374,19 @@ public void copyFromSafe(int fromIndex, int thisIndex, Nullable${minor.class}Vec
         values.copyFromSafe(fromIndex, thisIndex, from.values);
       }
     
    +  @Override
    +  public void copyEntry(int toIndex, ValueVector from, int fromIndex) {
    +    Nullable${minor.class}Vector fromVector = (Nullable${minor.class}Vector) from;
    +    <#if type.major == "VarLen">
    +
    +    // This method is to be called only for loading the vector
    +    // sequentially, so there should be no empties to fill.
    +
    +    </#if>
    +    bits.copyFromSafe(fromIndex, toIndex, fromVector.bits);
    +    values.copyFromSafe(fromIndex, toIndex, fromVector.values);
    --- End diff --
    
    IntVector.copyEntry() will call DrillBuf.getBytes(....), yet NullableIntVector.copyEntry() will call IntVector.copyFromSafe() which seems to go through different code path.  Is there a reason for such difference?



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

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

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

    https://github.com/apache/drill/pull/704#discussion_r107219761
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/svremover/TestSVRemover.java ---
    @@ -34,4 +38,33 @@ public void testSVRWithNoFilter() throws Exception {
         int numOutputRecords = testPhysical(getFile("remover/sv_with_no_filter.json"));
         assertEquals(100, numOutputRecords);
       }
    +
    +  /**
    +   * Test the generic version of the selection vector remover copier
    +   * class. The code uses the traditional generated version by default.
    +   * This test sets the option to use the generic version, then runs
    +   * a query that exercises that version.
    +   * <p>
    +   * Note that the tests here exercise only the SV2 version of the
    +   * selection remover; no tests exist for the SV4 version.
    +   */
    +
    +  // TODO: Add an SV4 test once the improved mock data generator
    +  // is available.
    +
    +  @Test
    +  public void testGenericCopier() throws Exception {
    +    // TODO: replace this with new setup once revised test framework
    +    // is available.
    +    Properties config = new Properties( );
    +    config.put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, "false");
    +    config.put(ExecConstants.HTTP_ENABLE, "false");
    +    config.put(ExecConstants.REMOVER_ENABLE_GENERIC_COPIER, "true");
    +    updateTestCluster(1, DrillConfig.create(config));
    +
    +    int numOutputRecords = testPhysical(getFile("remover/test1.json"));
    +    assertEquals(50, numOutputRecords);
    +    numOutputRecords = testPhysical(getFile("remover/sv_with_no_filter.json"));
    +    assertEquals(100, numOutputRecords);
    +  }
     }
    --- End diff --
    
    It would be good to have the tests that cover all the vector types.  But,  since it is off by default and you are exercising the code through other unit tests,  this is fine. 
    +1


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

[GitHub] drill pull request #704: DRILL-5125: Provide option to use generic code for ...

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

    https://github.com/apache/drill/pull/704#discussion_r106789982
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/GenericSV2Copier.java ---
    @@ -0,0 +1,65 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.drill.exec.physical.impl.svremover;
    +
    +import org.apache.drill.exec.ops.FragmentContext;
    +import org.apache.drill.exec.record.RecordBatch;
    +import org.apache.drill.exec.record.VectorWrapper;
    +import org.apache.drill.exec.vector.ValueVector;
    +
    +/**
    + * Generic selection vector 2 copier implementation that can
    + * be used in place of the generated version. Relies on a
    + * virtual function in each value vector to choose the proper
    + * implementation. Tests suggest that this version performs
    + * better than the generated version for queries with many columns.
    + */
    +
    +public class GenericSV2Copier extends CopierTemplate2 {
    +
    +  private ValueVector[] vvOut;
    +  private ValueVector[] vvIn;
    +
    +  @SuppressWarnings("unused")
    +  @Override
    +  public void doSetup(FragmentContext context, RecordBatch incoming,
    +      RecordBatch outgoing) {
    --- End diff --
    
    Fixed.


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