RFR JDK-8172921: Zip filesystem performance improvement and code cleanup

[Andrej Golovnin]

Hi Sherman,

thanks for the update. It looks much better now.
One more thing:

src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipCoder.java

70     private static ZipCoder utf8 = new UTF8();

I think this field can be final as it never changes.

Best regards,
Andrej Golovnin

> On 18 Jan 2017, at 19:43, Xueming Shen <xueming.shen at oracle.com> wrote:
> 
> Hi Andrej,
> 
> Thanks for the review.
> 
> I was debating with myself if it's really a good idea to "silently" go with the
> default charset in case the specified charset can not be obtained. Given the
> charset name is passed via the env map, it might be better to simply let the
> runtime exception  get thrown to communicate to the caller that the encoding
> property is wrong.
> 
> The webrev has been updated accordingly.
> 
> Sherman
> 
> On 01/17/2017 11:56 PM, Andrej Golovnin wrote:
>> Hi Sherman,
>> 
>> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipCoder.java
>> 
>>   72     public static ZipCoder get(String csn) {
>>   73         try {
>>   74         Charset cs = Charset.forName(csn);
>>   75         if (cs.name().equals("UTF-8")) {
>>   76             return utf8;
>>   77         }
>>   78         return new ZipCoder(cs);
>>   79         } catch (Throwable t) {
>>   80         t.printStackTrace();
>>   81         }
>>   82         return new ZipCoder(Charset.defaultCharset());
>>   83     }
>> 
>> Wouldn't it be better to use System.Logger instead of printStackTrace
>> in the line 80?
>> 
>> Best regards,
>> Andrej Golovnin
>> 
>> On Tue, Jan 17, 2017 at 11:39 PM, Xueming Shen<xueming.shen at oracle.com>  wrote:
>>> Hi,
>>> 
>>> Please review the following changes for zipfs implementation.
>>> 
>>> issue: https://bugs.openjdk.java.net/browse/JDK-8172921
>>> webrev: http://cr.openjdk.java.net/~sherman/8172921/webrev/
>>> 
>>> 
>>> javac has moved to use zipfs (instead of ZipFile) to access jar files in
>>> jdk9. Here are
>>> some changes to improve the performance of access time and footprint (reduce
>>> the
>>> unnecessary object allocation ...) The improvement has been measured by the
>>> jmh
>>> benchmark test as
>>> 
>>> http://cr.openjdk.java.net/~sherman/8172921/MyBenchmark.java
>>> 
>>> with the benchmark sores (before-OLD/after-NEW) at:
>>> 
>>> http://cr.openjdk.java.net/~sherman/8172921/scores
>>> 
>>> 
>>> The main change is to change the internal directory path representation from
>>> the
>>> zip specific format (directory name ends with "/", "/dir/" for directory
>>> ".dir" for
>>> example) to the "normalized" form with the tailing "/", which reduces the
>>> back and
>>> forth  conversion between the normal "unix style" path and the "zip style"
>>> path when
>>> doing path creation, path lookup and entry access, which also simplified the
>>> entry
>>> lookup logic.
>>> 
>>> We are seeing pretty good performance improvement.
>>> 
>>> There is another change, which involves the API change, for the
>>> "non-existing" path
>>> look up (which shows pretty bad numbers as ZFS_ExistsNG"), is not included
>>> in this
>>> patch. I hope we can address that as well in jdk9, but probably in another
>>> patch.
>>> 
>>> Thanks,
>>> Sherman
>>> 
>>> 
>