Hi Claes, I think this looks good. One suggestion before you finalize and push, perhaps update the comment in ZipFile
———— // Use the "oldest ASCII trick in the book" ————— to include something that this is for lowercase comparison. It just might help others when they look at the code and do not know the trick ;-) What made me think about this was the addition of isManifestName() which also uses this means to lowercase. Best Lance > On May 8, 2020, at 6:57 AM, Claes Redestad <claes.redes...@oracle.com> wrote: > > Hi Max, > > On 2020-05-08 09:08, Weijun Wang wrote: >> JarFile.java: >> 734: extra new line? > > Fixed > >> 930: Remove or rewrite the comment. > > Did even better: now that I find the position of the manifest during > initCEN, this method should always call JUZFA.getManifest(this, false); > - which is both a simplification and an optimization. > >> ZipFile.java: >> 49: seems not used > > Fixed > >> Please add links to each other between >> src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java::isBlockOrSF >> and >> src/java.base/share/classes/java/util/zip/ZipFile.java::isSignatureRelated. >> The 2nd method can also be static. >> I assume you cannot put ZipFile::isSignatureRelated into >> SignatureFileVerifier.java, right? > > Right, that wouldn't be great for either since ZipFile operates directly > on UTF-8 encoded bytes for performance, while SignatureFileVerifier > works on Strings. > > What we can do though is to add in an assert that the result of > ZF::isSignatureRelated matches that of SFV::isBlockOrSF - which should > ensure. I also added a note to SignatureFileVerifier::isBlockOfSF to > keep these in sync: > > http://cr.openjdk.java.net/~redestad/8244624/open.01/ > > Testing: re-running tier1+2 > >> Thanks, >> Max >> p.s. How do you run the benchmark test? Both locally and on mach5. > > See doc/testing.md > > Basically: > Configure with --with-jmh (jib does this automatically) > make build-microbenchmark > $JDK/bin/java -jar build/$CONF/images/test/micro/benchmarks.jar JarFileMeta > > Thanks! > > /Claes >>> On May 8, 2020, at 5:28 AM, Claes Redestad <claes.redes...@oracle.com> >>> wrote: >>> >>> Hi, >>> >>> currently during ZipFile creation, we create an int[] array of pointers >>> to META-INF entries. These are then retrieved from three different >>> places in JarFile. >>> >>> However, JarFile is only interested in some combination a few things: >>> the existence of and name of the META-INF/MANIFEST file, the existence >>> of and the names of various signature related files, i.e., files in >>> META-INF that have a suffix such as .EC, .SF, .RSA and .DSA >>> >>> Refactoring the contract between JarFile and ZipFile means we can filter >>> out such entries that we're not interested when opening the file, and >>> also remove the need to create the String for each entries unless we >>> actually want them: >>> >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8244624 >>> Webrev: http://cr.openjdk.java.net/~redestad/8244624/open.00/ >>> >>> This reduces retained footprint of Jar-/ZipFile by slimming down or >>> removing the Source.metanames array entirely, and a significant speed-up >>> in some cases. >>> >>> In the provided microbenchmark, getManifestFromJarWithManifest and >>> getManifestFromJarWithSignatureFiles doesn't call >>> JUZFA.getMetaInfEntryNames but still see small (~5%) speed-up decrease >>> in allocations. >>> >>> getManifestFromJarWithNoManifest exercise JUZFA.getMetaInfEntryNames in >>> the baseline, and now calls JUZFA.getManifestName. Result is a 1.25x >>> speed-up and 30% reduction in allocations. While unrealistic (most JARs >>> have a META-INF/MANIFEST.MF), this speed-up will translate to a few >>> places - such as when loading classes from potentially-signed JAR files. >>> >>> Testing: tier1-2 >>> >>> Thanks! >>> >>> /Claes <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
