8227080: (fs) Files.newInputStream(...).skip(n) is slow

[Brian Burkhalter]

> On Jul 2, 2019, at 1:01 PM, Simone Bordet <simone.bordet at gmail.com> wrote:
> 
> On Tue, Jul 2, 2019 at 9:42 PM Brian Burkhalter
> <brian.burkhalter at oracle.com <mailto:brian.burkhalter at oracle.com>> wrote:
>> 
>> Oops, I think that skip() needs to be synchronized. Webrev updated in place.
>> 
>>> On Jul 2, 2019, at 12:18 PM, Brian Burkhalter <brian.burkhalter at oracle.com <mailto:brian.burkhalter at oracle.com>> wrote:
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8227080 <https://bugs.openjdk.java.net/browse/JDK-8227080>
>>> http://cr.openjdk.java.net/~bpb/8227080/webrev.00/ <http://cr.openjdk.java.net/~bpb/8227080/webrev.00/>
>>> 
>>> This patch overrides ChannelInputStream.skip(long) to use SeekableByteChannel.position(long) if the ReadableByteChannel instance is a SeekableByteChannel. The performance improvement on my dev machine for skipping the first 2 GB of a local file is about 3.6 x 10^4.
>>> 
> 
> That's great!
> 
> I'm not a reviewer, but is there a policy to handle long overflows?

Good point. Maybe Math.addExact(long,long) could be used and if an ArithmeticException occurs it could be wrapped in an IOException. Needs investigation.

> Also, ChannelInputStream should be reviewed with respect to synchronization.
> For example, you fixed skip(), but available() is not synchronized,
> and read(ByteBuffer) should perhaps be (as subclasses may call it
> without grabbing the lock)?

I did look it over but maybe it needs another glance. I noticed that available() is non-sync as well. It looked as if for users of an instance, read() is OK but perhaps you have a point for subclasses.

> Thanks!

You’re welcome!

Brian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20190702/c3a62ced/attachment.html>