RFR: 8339332: Clean up the java launcher code
Jaikiran Pai
jpai at openjdk.org
Thu Sep 12 06:49:07 UTC 2024
On Tue, 10 Sep 2024 06:15:57 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
> Can I please get a review of this change which cleans up the code in the `java` launcher? The original motivation for the change was to prevent the `java` launcher's C code from parsing a jar's manifest when launching an application through `java -jar foo.jar`. The jar parsing code in C currently doesn't have the same level of robustness and features as what's available in the Java side implementation of `Zip/JarFile` API in `java.util.zip`. Among them, one is the lack of ZIP64 support in the launcher's jar parsing code. That issue was noticed in https://bugs.openjdk.org/browse/JDK-8328995 and a PR (https://github.com/openjdk/jdk/pull/18479) was proposed to enhance this C code in the launcher to support ZIP64 jar files. Even before the proposed changes in that PR, there already were a few concerns about long term maintainability of the jar parsing code in the launcher given that it duplicates what we already do in the Java side implementation, so adding new C code here isn't prefer
able.
>
> The change in this current PR removes the part where the launcher's C code was parsing the jar's manifest file to identify "Main-Class" and "SplashScreen-Image" attributes from the manifest of the jar that was being launched. This was being done in the C code to support a long outdated "JRE selection" feature (mJRE https://bugs.openjdk.org/browse/JDK-8058407) which required it to parse the jar's manifest. After that feature was removed, the sole reason the jar's manifest was continued to be parsed in this launcher code was for convenience.
>
> With the changes proposed in this PR, the launcher will use the jar parsing C code only if the jar has a "SplashScreen-Image" in its manifest. The number of such jars, based on an experiment against a large corpus of jar files, is very minimal (we found just 26 jars in around 900K odd jar files with a "SplashScreen-Image"). The updated code in this PR also delegates the identification of the "SplashScreen-Image" attribute to the java code (in `LauncherHelper`) thus removing the need to parse the manifest in C code. The only time the C code will now do the jar parsing is to display a splash screen image (if any is present) from the jar file. I think even that can be avoided, but I haven't experimented with it and I would like to pursue that separately in future, instead of this PR.
>
> Finally, a few of the utility functions that were introduced in the launcher C code in a recent JEP to support "Implicitly Declared Classes and Instance Main...
Hello David,
> This is all rather complex from a reviewers perspective - it is very hard to discern what code change relates to what you described in the PR. Can it be broken up into smaller chunks perhaps?
That was my initial idea - to break this up into two or even three separate tasks. But those attempts did not lead to a clean state which I thought could be integrated independently. If you and others think that this still needs to be broken down, then I can revive those efforts to change this into separate tasks.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/20929#issuecomment-2345403699
More information about the hotspot-runtime-dev
mailing list