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.s...@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.s...@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
>>>
>>>
>
