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

[Jason Zaugg]

On Thu, 6 May 2021 07:41:00 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Jason Zaugg has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove redundant cast from previous commit.
>
> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 1235:
> 
>> 1233:             synchronized(ch) {
>> 1234:                 return ch.position(pos).read(bb);
>> 1235:             }
> 
> I think it's okay to include the update to EntryInputStream, that part looks fine, as does the directly use of the FileChannel positional read.
> 
> I'm still mulling over the case where ch is not a FileChannel as I would expected it to capture the existing position and restore it after the read. I think this is the degenerative case when the zip file is located in a custom file system that doesn't support FileChannel. In that case, positional read has to be implemented on the most basic SeekableByteChannel. It would only be observed when mixing positional read ops with other ops that depend on the current position.

Here are all the references to `ch`.


this.ch = Files.newByteChannel(zfpath, READ);
...
this.ch.close();
...
ch.close();              // close the ch just in case no update
...
if (ch instanceof FileChannel fch) {
    return fch.read(bb, pos);
} else {
    synchronized(ch) {
        return ch.position(pos).read(bb);
    }
}
...
long ziplen = ch.size();
...
ch.close();


It appears the only position-dependent operation called `read(ByteBuffer)`. This is performed together with the `pos` call within the `synchronized(ch)` lock.

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

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