On Mon, 19 Feb 2024 10:41:03 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 821: >> >>> 819: // In LM_JAR mode, put the underlying file in the >>> JarFile/ZipFile cache. >>> 820: // This will avoid needing to re-parse the manifest when the >>> JAR file >>> 821: // is opened on the class path, triggered by Class.forName >>> below. >> >> Nice improvement to have. would it be ok to pad out this comment a bit more >> ? I found it difficult to understand the specifics. Perhaps : >> >> >> // In LM_JAR mode, keep the underlying file open to retain it >> in >> // the ZipFile HashMap cache. This will avoid needing to >> re-parse >> // the central directory when the file is opened on the >> class path, >> // triggered by Class.forName below. > >> Nice improvement to have. would it be ok to pad out this comment a bit more >> ? I found it difficult to understand the specifics. Perhaps : >> >> ``` >> // In LM_JAR mode, keep the underlying file open to retain it >> in >> // the ZipFile HashMap cache. This will avoid needing to >> re-parse >> // the central directory when the file is opened on the >> class path, >> // triggered by Class.forName below. >> ``` > > No issue with expanding the comment but I think leave it as "ZipFile cache" > as a more specific comment will quickly bit rot and confuse reader, and of > course the Launcher code doesn't care which collection is used internally in > the ZipFile code. I'll expand the comment as suggest, keeping the "JarFile/ZipFile cache" phrase. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17843#discussion_r1494722308