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

[Xueming Shen]

Hi Aleksey,

Again, thanks for the review. The webrev has been updated accordingly,
as well as the MyBenchmark.java (to use Blackhole, as suggested)

http://cr.openjdk.java.net/~sherman/8150496/webrev/
http://cr.openjdk.java.net/~sherman/8150496/MyBenchmark.java

-Sherman


On 05/03/2016 05:39 AM, Aleksey Shipilev wrote:
> 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
>