drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From paul-rogers <...@git.apache.org>
Subject [GitHub] drill pull request #1058: DRILL-6002: Avoid memory copy from direct buffer t...
Date Sat, 02 Dec 2017 00:26:56 GMT
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1058#discussion_r154476694
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorSerializer.java
---
    @@ -62,27 +72,65 @@ public Writer write(VectorAccessible va) throws IOException {
     
         @SuppressWarnings("resource")
         public Writer write(VectorAccessible va, SelectionVector2 sv2) throws IOException
{
    +      checkNotNull(va);
           WritableBatch batch = WritableBatch.getBatchNoHVWrap(
               va.getRecordCount(), va, sv2 != null);
           return write(batch, sv2);
         }
     
         public Writer write(WritableBatch batch, SelectionVector2 sv2) throws IOException
{
    -      VectorAccessibleSerializable vas;
    -      if (sv2 == null) {
    -        vas = new VectorAccessibleSerializable(batch, allocator);
    -      } else {
    -        vas = new VectorAccessibleSerializable(batch, sv2, allocator);
    -      }
    -      if (retain) {
    -        vas.writeToStreamAndRetain(stream);
    -      } else {
    -        vas.writeToStream(stream);
    +      return write(batch, sv2, false);
    +    }
    +
    +    public Writer write(WritableBatch batch, SelectionVector2 sv2, boolean retain) throws
IOException {
    --- End diff --
    
    Retain is never actually used. That's why, in the earlier version, it was a flag that
was not passed into the write method. Further, whether retain is not likely to be decided
batch-by-batch, rather it is a general policy set at the writer level. So, if we don't like
the `retain()` method, perhaps pass this option into the constructor rather than into each
`write()` call.


---

Mime
View raw message