RFR: 8265448: (zipfs): Reduce read contention in ZipFileSystem [v3]

[Jason Zaugg]

On Wed, 5 May 2021 23:23:21 GMT, Jason Zaugg <jzaugg at openjdk.org> wrote:

>> I think using the positional read on the underlying FileChannel is okay. I'm puzzled by the previous code as I would have expected it to restore the position (make me wonder if there are zipfs tests for this).
>
> My reading of the existing code is that the only position-influenced method called on the channel (either via `ZipFileSystem.ch` or `ZipFileSystem$EntryInputStream.zfch`) is `read`, and this is only called in the `.position(pos).read(...)` idiom. The failure to reset the position doesn't affect correctness. However the `synchronzized` _is_ definitely needed to avoid races.
> 
> Incidentally, regarding this comment:
> 
>     private class EntryInputStream extends InputStream {
>         private final SeekableByteChannel zfch; // local ref to zipfs's "ch". zipfs.ch might
>                                                 // point to a new channel after sync()
> 
> If the file system is writable and updated, the underlying file is deleted and replaced with a temporary file by `close()` / `sync()`, but `ZipFileSystem.ch` is itself final since d581e4f4. I believe the comment is outdated and `EntryInputStream` could just access ch via the outer pointer. That change would simplify this patch marginally.

I've added the simplifying commit for now, but I'm happy to split that to a separate change if you prefer.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3853