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

[Jason Zaugg]

On Wed, 5 May 2021 16:03:17 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Yes, I plan to look at this.  It would also be good to have a couple of additional reviews as well :-)
>
> 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.

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

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