AdoptOpenJDK / jitwatch

Log analyser / visualiser for Java HotSpot JIT compiler. Inspect inlining decisions, hot methods, bytecode, and assembly. View results in the JavaFX user interface.
Other
3.04k stars 435 forks source link

Add `Add-Exports` manifest attribute #351

Closed Thihup closed 2 years ago

Thihup commented 2 years ago

When using JDK 16+, we get the following exception when trying to see the Bytecode. The fix via command line is --add-exports=jdk.jdeps/com.sun.tools.javap=ALL-UNNAMED.

However, I think that adding Add-Exports: jdk.jdeps/com.sun.tools.javap in the MANIFEST.MF would make it easier to use.

java.lang.IllegalAccessException: class org.adoptopenjdk.jitwatch.process.javap.ReflectionJavap cannot access class com.sun.tools.javap.JavapTask (in module jdk.jdeps) because module jdk.jdeps does not export com.sun.tools.javap to unnamed module @59d2a08f
        at java.base/jdk.internal.reflect.Reflection.newIllegalAccessException(Reflection.java:392)
        at java.base/java.lang.reflect.AccessibleObject.checkAccess(AccessibleObject.java:674)
        at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:489)
        at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
        at org.adoptopenjdk.jitwatch.process.javap.ReflectionJavap.createJavapTaskFromArguments(ReflectionJavap.java:135)
        at org.adoptopenjdk.jitwatch.process.javap.ReflectionJavap.getBytecode(ReflectionJavap.java:121)
chriswhocodes commented 2 years ago

Hi @Thihup hmm, I put in a catch so that it should fallback to JavapProcess (a ProcessBuilder wrapper around the JDK/bin/javap executable) if the reflection attempt fails.

Please can you let me know what OS and JDK you are using (and are you using the latest JITWatch built from source?)

Cheers,

Chris

Thihup commented 2 years ago

Hi @chriswhocodes. I'm using Linux and JDK 18, but since JDK 16 this exception happens. I'm using the 1.4.0-shaded.jar version, but I can try building it from the source too.

I think if it is possible to add the manifest attribute, it should be a little faster in the JVMs that support that manifest attribute.

chriswhocodes commented 2 years ago
Author: Chris Newland <chris@chrisnewland.com>
Date:   Mon Jan 4 23:05:26 2021 +0000

    Fix JavapTask fallback for JDK17 reflection failure (module system)

Yep, the binary release was on 2020-10-29 so it doesn't include this fix.

I agree my commit was a hack and that the correct module system fix is better.

Let me try your suggestion!

Thanks for reporting and contributing :)

chriswhocodes commented 2 years ago

OK, I updated the shade plugin config to

<plugin>
                                                <groupId>org.apache.maven.plugins</groupId>
                                                <artifactId>maven-shade-plugin</artifactId>
                                                <version>3.2.0</version>
                                                <executions>
                                                        <execution>
                                                                <phase>package</phase>
                                                                <goals>
                                                                        <goal>shade</goal>
                                                                </goals>
                                                                <configuration>
                                                                        <shadedArtifactAttached>true</shadedArtifactAttached>
                                                                        <shadedClassifierName>project-classifier</shadedClassifierName>
                                                                        <outputFile>target/ui-shaded.jar</outputFile>
                                                                        <transformers>
                                                                                <transformer implementation="org.apache.maven.plugins.shade.resource.ManifestResourceTransformer">
                                                                                        <mainClass>${main.class}</mainClass>
                                                                                        <manifestEntries>
                                                                                                <Add-Exports-Private>jdk.jdeps/com.sun.tools.javap</Add-Exports-Private>
                                                                                        </manifestEntries>
                                                                                </transformer>
                                                                        </transformers>
                                                                </configuration>
                                                        </execution>
                                                </executions>
                                        </plugin>

which gives me a MANIFEST.MF of

chris@aurora:~/IdeaProjects/jitwatch/ui/target$ more META-INF/MANIFEST.MF 
Manifest-Version: 1.0
Archiver-Version: Plexus Archiver
Created-By: Apache Maven 3.6.0
Built-By: chris
Build-Jdk: 17
Main-Class: org.adoptopenjdk.jitwatch.launch.LaunchUI
Add-Exports-Private: jdk.jdeps/com.sun.tools.javap

which looks OK but when I run the jar:

chris@aurora:~/IdeaProjects/jitwatch$ /home/chris/jdk-17/bin/java -jar ui/target/ui-shaded.jar 

and then run JITWatch I still get the fallback to ProcessBuilder:

21:30:11.582 [Thread-3] INFO  o.a.j.l.BytecodeLoader - Could not fetch bytecode via reflection, trying Process
21:30:11.583 [Thread-3] INFO  o.a.j.p.r.RuntimeJava - JavapProcess() executablePath: /home/chris/jdk-17/bin/javap

Any ideas?

Thihup commented 2 years ago

From the JEP [1], the manifest attribute is Add-Exports instead of Add-Exports-Private. Could you give it a try?

[1] https://openjdk.java.net/jeps/261#Packaging:-Modular-JAR-files

chriswhocodes commented 2 years ago

image Bingo! That works :) I'll commit the pom fix and then after some testing I'll create some new binaries. Many thanks Thiago!