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.08k stars 437 forks source link

tableswitch and Method size #381

Closed mayerrobert closed 10 months ago

mayerrobert commented 11 months ago

Hi,

it seems like JarScan miscalulates method sizes. Apparently it considers the first occurrence of the "tableswitch" bytecode as the end of the method and only counts bytecodes up to but not including the first "tableswitch". Here's what I tried to prove my assumption:

C:\robert\jarscanbug>cat Tableswitch.java public class Tableswitch { void f(int n) { System.out.println("Hi!"); switch (n) { case 0: System.out.println("zero"); break; case 1: System.out.println("one"); break; case 2: System.out.println("two"); break; } } } C:\robert\jarscanbug>javac Tableswitch.java

C:\robert\jarscanbug>javap -c Tableswitch.class Compiled from "Tableswitch.java" public class Tableswitch { public Tableswitch(); Code: 0: aload_0 1: invokespecial #1 // Method java/lang/Object."":()V 4: return

void f(int); Code: 0: getstatic #2 // Field java/lang/System.out:Ljava/io/PrintStream; 3: ldc #3 // String Hi! 5: invokevirtual #4 // Method java/io/PrintStream.println:(Ljava/lang/String;)V 8: iload_1 9: tableswitch { // 0 to 2 0: 36 1: 47 2: 58 default: 66 } 36: getstatic #2 // Field java/lang/System.out:Ljava/io/PrintStream; 39: ldc #5 // String zero 41: invokevirtual #4 // Method java/io/PrintStream.println:(Ljava/lang/String;)V 44: goto 66 47: getstatic #2 // Field java/lang/System.out:Ljava/io/PrintStream; 50: ldc #6 // String one 52: invokevirtual #4 // Method java/io/PrintStream.println:(Ljava/lang/String;)V 55: goto 66 58: getstatic #2 // Field java/lang/System.out:Ljava/io/PrintStream; 61: ldc #7 // String two 63: invokevirtual #4 // Method java/io/PrintStream.println:(Ljava/lang/String;)V 66: return }

C:\robert\jarscanbug>java -cp ..\scripts\jitwatch-ui-1.4.7-shaded-win.jar org.adoptopenjdk.jitwatch.jarscan.JarScan --mode=methodLength --length=9 . "","Tableswitch","f","int",9

chriswhocodes commented 11 months ago

Thanks for reporting! I had no idea anyone was still using Jarscan. I will look into this for you.

chriswhocodes commented 10 months ago

Hi @mayerrobert can you check you are using the latest version of JITWatch please? I believe I fixed this in https://github.com/AdoptOpenJDK/jitwatch/commit/8aa10972100a542df8b1a5f9f9d2af91913cd944 and I get the correct result from your test class on the latest code:

java -cp ui/target/jitwatch-ui-shaded.jar org.adoptopenjdk.jitwatch.jarscan.JarScan --mode=methodLength --length=67 . | grep Tableswitch

"","Tableswitch","f","int",67

Kind regards,

Chris

mayerrobert commented 10 months ago

Great, it works now!

I have since written a simple testprogram that displays method sizes using javaassist (but without convenient features of jarscan such as commandline arguments).

Both my simple program as well as the updated jarscan now show the same results.

As far as I'm concerned you could close the issue now.

Cheers, Robert.

PS: if you're courious - I'm currently refactoring a rather old and big project that has methods up to 11k bytecode size, and jarscan is handy to find these offenders which I then split up or otherwise refactor. Number of lines of Java source code is one thing to consider, but jarscan provides useful information, too.

Edit: took me a while to figure out that just updating to HEAD got me your fix that you already did some time ago. Should have thought of that myself, sorry :-D I'll keep that in mind for the future.

chriswhocodes commented 10 months ago

Glad it's working for you and that you're finding it useful 😀