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

[Xueming Shen]

Andrej,

I have pushed in the change before this suggestion came in. I will put
this on my todo list when touch that code again next time.

Thanks,
Sherman

On 1/18/17, 1:16 PM, Andrej Golovnin wrote:
> 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
>>>>
>>>>