Hi Paul, Thanks for the suggestion. Webrev has been updated accordingly.
http://cr.openjdk.java.net/~sherman/8170831 Sherman On 12/7/16, 10:19 AM, Paul Sandoz wrote:
Hi, 334 if (lastEntryName != null&& lastEntryName == entry.name) { Can you just do: if (Objects.equals(lastEntryName, entry.name)) { // [*] ? the assumptions being entry.name is never null, which i believe is valid, and one can check the string contents if refs are not equal. Paul. [*] public static boolean equals(Object a, Object b) { return (a == b) || (a != null&& a.equals(b)); }On 6 Dec 2016, at 16:14, Xueming Shen<xueming.s...@oracle.com> wrote: Hi, Please help review.commit the proposed change for JDK8170831 issue: https://bugs.openjdk.java.net/browse/JDK-8170831 webrev: http://cr.openjdk.java.net/~sherman/8170831/ (1) As explained in the issue description, the new implementation now does not have the cache mechanism as the old C implementation does (the C implementation still is still in repo, take a look at "jdk/src/java.base/share/native/libzip/zip_util.c#Zip_GetEntry2(...)". the corresponding cache is stored in zip->cache, "zip_util.h#trypedef struct jzfile/cache". (2) instead of having this cache in class ZipFile.Source, the cached pair is now at ZipFile level for simplicity. (3) I now simply compare the identity of the entry "name", to avoid the unnecessary string comparing operation, with the assumption that it's rare that someone will replace the entry name filed of the returned ZipEntry with a new String object with the same value and come back for the InputStream. (4) given currently there is no easy way to generate a zip/jar file with same entry name in test case (out ZipInputStream does not such use scenario, with an exception), I'm not adding new test case as the regression test, but I do have verified the change with the jar files that have entries with same name but different bytes. opinion? Thanks, Xueming
