drill-dev mailing list archives

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

    https://github.com/apache/drill/pull/1058#discussion_r154499915
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorSerializer.java
---
    @@ -40,20 +52,18 @@
        */
     
       public static class Writer {
    +    static final MetricRegistry metrics = DrillMetrics.getRegistry();
    +    static final String WRITER_TIMER = MetricRegistry.name(VectorAccessibleSerializable.class,
"writerTime");
     
    -    private final OutputStream stream;
    -    private final BufferAllocator allocator;
    -    private boolean retain;
    +    private final SpillSet spillSet;
    +    private final WritableByteChannel channel;
    +    private final OutputStream output;
         private long timeNs;
     
    -    public Writer(BufferAllocator allocator, OutputStream stream) {
    -      this.allocator = allocator;
    -      this.stream = stream;
    -    }
    -
    -    public Writer retain() {
    -      retain = true;
    -      return this;
    +    private Writer(SpillSet spillSet, String path) throws IOException {
    --- End diff --
    
    The `Writer` in `VectorSerializer` is used only for spilling.  I'd suggest moving `VectorSerializer`
to the `spill`package to make it more explicit and keep the dependency on the `SpillSet`.
I am not sure if there is a need for generic `Writer` that can handle multiple use cases.
I think that `Writer` needs to be optimized to handle spill use case.
    
    `SpillSet` is also needed to get `WritableByteChannel` and encapsulates what `Writer`
uses from operators.


---

Mime
View raw message