cassandra-pr mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aweisberg <...@git.apache.org>
Subject [GitHub] cassandra pull request #239: Optimize Streaming
Date Fri, 06 Jul 2018 16:27:15 GMT
Github user aweisberg commented on a diff in the pull request:

    https://github.com/apache/cassandra/pull/239#discussion_r200704746
  
    --- Diff: src/java/org/apache/cassandra/net/async/RebufferingByteBufDataInputPlus.java
---
    @@ -249,4 +253,50 @@ public ByteBufAllocator getAllocator()
         {
             return channelConfig.getAllocator();
         }
    +
    +    /**
    +     * Consumes bytes in the stream until the given length
    +     *
    +     * @param writer
    +     * @param len
    +     * @return
    +     * @throws IOException
    +     */
    +    public long consumeUntil(BufferedDataOutputStreamPlus writer, long len) throws IOException
    +    {
    +        long copied = 0; // number of bytes copied
    +        while (copied < len)
    +        {
    +            int position = buffer.position();
    +            int remaining = buffer.remaining();
    +            if (remaining == 0)
    +            {
    +                try
    +                {
    +                    reBuffer();
    +                } catch (EOFException e)
    +                {
    +                    throw new EOFException("EOF after " + copied + " bytes out of " +
len);
    +                }
    +                position = buffer.position();
    +                remaining = buffer.remaining();
    +                if (remaining == 0)
    +                    return copied == 0 ? -1 : copied;
    +            }
    +
    +            int toCopy = (int) Math.min(len - copied, remaining);
    +
    +            ByteBuffer dup = buffer.duplicate();
    --- End diff --
    
    I don't see why you would store position on the stack outside of the buffer it really
just seems like an opportunity to get it wrong by not going direct to the buffer. Same with
remaining it's only ever used one after it is ever stored on the stack so why store it on
the stack at all?
    I hate to keep bikeshedding this, but I don't even see why you need the dup or to manually
update the position? 
    
    ```
    if (buffer.remaining() == 0)
    {
        try 
       {
            reBuffer();
        }
        catch (EOFException e)
        {
            throw new EOFException("EOF after " + copied + " bytes out of " + len);
        }
        if (buffer.remaining() == 0)
        {
            return copied == 0 ? -1 : copied;
        }
    }
    
    int originalLimit = buffer.limit();
    buffer.limit(Math.min(len - copied, remaining);
    int written = writer.applyToChannel(c -> c.write(buffer));
    buffer.limit(originalLimit);
    copied += written;
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


Mime
View raw message