Github user parthchandra commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1144#discussion_r180598658
  
    --- Diff: exec/vector/src/main/codegen/templates/VariableLengthVectors.java 
---
    @@ -534,15 +534,11 @@ public void setSafe(int index, byte[] bytes) {
           assert index >= 0;
     
           final int currentOffset = offsetVector.getAccessor().get(index);
    -      offsetVector.getMutator().setSafe(index + 1, currentOffset + 
bytes.length);
    -      try {
    -        data.setBytes(currentOffset, bytes, 0, bytes.length);
    -      } catch (IndexOutOfBoundsException e) {
    -        while (data.capacity() < currentOffset + bytes.length) {
    -          reAlloc();
    -        }
    -        data.setBytes(currentOffset, bytes, 0, bytes.length);
    +      while (data.capacity() < currentOffset + bytes.length) {
    --- End diff --
    
    I'm on the fence on this one. The change from 18 months ago resulted in a 
performance improvement of a few percent for variable length columns. This 
change essentially undoes that and takes us back to where we were. It seems 
that with the recent changes to batch sizing, we are on the path to eliminating 
the realloc behaviour that is part of the performance bottleneck for the 
setSafe methods. So, perhaps, it might be ok to let this one stay.  


---

Reply via email to