RFR JDK-8172921: Zip filesystem performance improvement and code cleanup
Xueming Shen
xueming.shen at oracle.com
Wed Jan 18 18:43:10 UTC 2017
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
>>
>>
More information about the core-libs-dev
mailing list