[ 
https://issues.apache.org/jira/browse/DERBY-3770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12626097#action_12626097
 ] 

Knut Anders Hatlen commented on DERBY-3770:
-------------------------------------------

Thanks for the new patch. It basically looks good. A couple of small issues:

1) Package and class name in the header of InputStreamUtilTest should be 
updated.

2) I think Dan's point with skipFully (now skipUntilEOF) was that you don't 
necessarily have to call skipPersistent until it returns 0. It is OK to stop 
calling it once it returns less bytes than requested. So to reduce the number 
of times skipPersistent is called, skipUntilEOF could do something like this:

long bytes = 0;
while (true) {
    long skipped = skipPersistent(is, SKIP_FRAGMENT_SIZE);
    bytes += skipped;
    if (skipped < SKIP_FRAGMENT_SIZE) {
        return bytes;
    }
}

3) I noticed that SKIP_FRAGMENT_SIZE had been lowered from 1024*1024 to 
512*1024 in this patch. I don't think there's any reason to keep this constant 
small. There shouldn't be any disadvantages with having a higher value, so it 
might be better to set it to a very high value, for instance Integer.MAX_VALUE.

> Create a utility class for skipping data in an InputStream
> ----------------------------------------------------------
>
>                 Key: DERBY-3770
>                 URL: https://issues.apache.org/jira/browse/DERBY-3770
>             Project: Derby
>          Issue Type: Improvement
>          Components: Miscellaneous
>    Affects Versions: 10.5.0.0
>            Reporter: Kristian Waagan
>            Assignee: Junjie Peng
>            Priority: Minor
>         Attachments: derby-3770-1.patch, derby-3770-1.stat, 
> derby-3770-2.patch, derby-3770-2.stat, derby-3770-3.patch, derby-3770-3.stat, 
> derby-3770-4.patch, derby-3770-4.patch, derby-3770-4.stat, derby-3770-4.stat
>
>
> The contract of InputStream.skip is somewhat difficult, some would even say 
> broken.
> See http://java.sun.com/javase/6/docs/api/java/io/InputStream.html#skip(long))
> A utility class should be created to ensure that we use the same skip 
> procedure throughout the Derby code base.
> Suggested functionality:
>  - long skipFully(InputStream) : skips until EOF, returns number of bytes 
> skipped
>  - void skipFully(InputStream,long) : skips requested number of bytes, throws 
> EOFException if there is too few bytes in the stream
> I know of two different approaches, both skipping in a loop:
>  a) Verify EOF with a read call when skip returns zero.
>  b) Throw EOFException if skip returns zero before requested number of bytes 
> have been skipped.
> There's related code in iapi.util.UTF8Util. Maybe this class, say StreamUtil, 
> could be put in the same package?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to