RFR: JDK-8150496,(zipfs) Fix performance issues in zip-fs

[Aleksey Shipilev]

Hi Sherman,

On 05/03/2016 02:28 AM, Xueming Shen wrote:
> Please help review the performance cleanup for zipfs
> 
> issue: https://bugs.openjdk.java.net/browse/JDK-8150496
> webrev: http://cr.openjdk.java.net/~sherman/8150496/webrev

ZipFileSystem.java:

*) These should be removed?

 2032             //writeShort(os, name.length);
 2033             writeShort(os, nlen);

 2220             //writeShort(os, name.length);
 2221             writeShort(os, nlen);

 2591         //inodes.forEach( (k,v) -> { System.out.printf(" %s  ->
%s%n", new String(k.name), new String(v.name)); });

 1080             /*
 1081             byte[] name = Arrays.copyOfRange(cen, pos + CENHDR,
pos + CENHDR + nlen);
 1082             */

*) There is ever-so-subtle difference in doing either:

 171             byte[] tmp = new byte[path.length + 1];
 172             System.arraycopy(path, 0, tmp, 1, path.length);
 173             tmp[0] = '/';

...or:

1083             byte[] name = new byte[nlen + 1];
1084             name[0] = '/';
1085             System.arraycopy(cen, pos + CENHDR, name, 1, nlen);

Excess op between allocation and arraycopy breaks zeroing elimination,
see e.g.:
  http://cr.openjdk.java.net/~shade/scratch/ZeroingFirstBench.java


*) Missing braces:

1483                 if (e2 == null)
1484                     throw new ZipException("invalid loc for entry
<" + e.name + ">");

*) Probably deserves a comment why these are commented out? Are these
implied to be zeroes?

1713         // int  disknum;
1714         // int  sdisknum;


> Here are the result of the sample benchmark that measures the improvement.
> http://cr.openjdk.java.net/~sherman/8150496/MyBenchmark.java

This benchmark is not very reliable:

 *) Sink everything in Blackhole. I.e. instead of, say:

    @Benchmark
    public void ZFS_Open() throws IOException {
        cnt = 0;
        for (int i = 0; i < 100; i++) {
            try (FileSystem fs = newFileSystem(path)) {
                if (Files.exists(fs.getPath(entriesS.get(0))))
		    cnt++;
            }
        }
    }

do:

    @Benchmark
    public void ZFS_Open(Blackhole bh) throws IOException {
        for (int i = 0; i < 100; i++) {
            try (FileSystem fs = newFileSystem(path)) {
                bh.consume(fs.getPath(entriesS.get(0)));
            }
        }
    }

This applies to all other methods.

Thanks,
-Aleksey