Ulf, thanks for the suggestions!

On 5/31/16 6:27 AM, Ulf Zibis wrote:
Hi Sherman,

1.) wouldn't it be better to have a public getBytes() in AbstractStringBuilder?
Then you could save the array copy with sb.toString() here:
 178         return new ZipPath(this, sb.toString(), zc.isUTF8());
 525         return zfs.getBytes(to.toString());



One single use case here probably is not a strong enough reason to
add such a utility method into ASB.

You could simplify even more.

2.) No need for fork on isUTF8 in ZFS.iteratorOf() and for parameter isUTF8:
  55     ZipPath(ZipFileSystem zfs, byte[] path, boolean normalized)
  56     {
  57         this.zfs = zfs;
  58         if (normalized)
  59             this.path = path;
  60         // if zfs is NOT in utf8, normalize the path as "String"
  61         // to avoid incorrectly normalizing byte '0x5c' (as '\')
  62         // to '/'.
  63         else if (zfs.zc.isUTF8())
  64             this.path = normalize(path);
  65         else
  66             this.path = normalize(zfs.getString(path));
  67     }

  64     ZipPath(ZipFileSystem zfs, String path) {
  65         this.zfs = zfs;
  66         // if zfs is NOT in utf8, normalize the path as "String"
  67         // to avoid incorrectly normalizing byte '0x5c' (as '\')
  68         // to '/'.
  69         if (zfs.zc.isUTF8()) {
  70             this.path = normalize(zfs.getBytes(path));
  71         } else {
  72             this.path = normalize(path);
  73         }
  74     }
  75

I was hesitated to exposure zfs.zc to be package private for some reason, that was why have the pair of zfs.getBytes/String(). But sure I can do it if it makes
the communication clearer.

webrev has been updated accordingly.

http://cr.openjdk.java.net/~sherman/8061777/webrev


6.) Avoid String instatiation especially when called from child paths iterator *loop*.
Instead:
 500         if (len > 1 && prevC == '/')
 501             path = path.substring(0, len - 1);
 502         return zfs.getBytes(path);
provide:
500 return zfs.getBytes(path, 0, len - (len > 1 && prevC == '/' ? 1 : 0 ));


This can be a candidate for further optimization. Currently my assumption is that non-utf8 jar/zip files are not the main use cases for zipfs (assume most of them are the result of either the jar tool or j.u.zip apis), so I would wait to add a new
method into zc. Hope this is fine with you.

Sherman

Reply via email to