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_r154477573
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/spill/SpillSet.java
---
    @@ -331,6 +337,43 @@ public long getReadBytes(InputStream inputStream) {
         }
       }
     
    +  private static class WritableByteChannelImpl implements WritableByteChannel
    +  {
    +    private static final int TRANSFER_SIZE = 32 * 1024;
    +
    +    private OutputStream out;
    +    private final byte buffer[] = new byte[TRANSFER_SIZE];
    --- End diff --
    
    Unfortunately, this is one of the problems that the current design was intended to solve.
As we can see in the hash agg code; multiple of these objects are active in memory at any
one time. But, no more than one is ever writing. By putting the buffer here, we create one
per (potentially thousands) of retained channels. But, by putting the buffer on the allocator,
we hold one buffer per operator.
    
    Being the guy who wrote the current allocator code, I realize it is clunky. But, it is
clunky because we favored memory frugality over elegance. (If there is a cleaner way to have
one buffer per thread, I'm all for it!)
    
    Isn't the purpose of this change to avoid memory buffers? Seems, for the above reasons,
we're moving backwards...


---

Mime
View raw message