Bytecode for Java 14 instanceof

42 views
Skip to first unread message
Assigned to mand...@gmail.com by me

Marc Hoffmann

unread,
Jan 14, 2020, 11:11:31 AM1/14/20
to jacoc...@googlegroups.com
Hi Evgeny,

I created a validation test for JEP 305:


In this simple example as a developer I would expect 2 branches:

if (obj instanceof String s) { // assertFullyCovered(0, 2)
nop(s); // assertFullyCovered()
}


In this simple example as a developer I would expect 2 branches. But actually the complier creates a IMHO redundant check which results in four branches (see below).

What do you think? Is that something that should be reported as a bug to OpenJDK?

Cheers,
-marc


  // Method descriptor #12 (Ljava/lang/Object;)V
  // Stack: 2, Locals: 3
  private static void ifInstanceOf(java.lang.Object obj);
     0  aload_0 [obj]
     1  astore_2
     2  aload_2
     3  instanceof java.lang.String [15]
     6  ifeq 26
     9  aload_2
    10  checkcast java.lang.String [15]
    13  dup
    14  astore_1 [s]
    15  aload_2
    16  checkcast java.lang.String [15]
    19  if_acmpne 26
    22  aload_1 [s]
    23  invokestatic org.jacoco.core.test.validation.targets.Stubs.nop(java.lang.Object) : void [17]
    26  return
      Line numbers:
        [pc: 0, line: 29]
        [pc: 22, line: 30]
        [pc: 26, line: 32]
      Local variable table:
        [pc: 15, pc: 26] local: s index: 1 type: java.lang.String
        [pc: 0, pc: 27] local: obj index: 0 type: java.lang.Object
      Stack map table: number of frames 1
        [pc: 26, same]
}

 

Rory O'Donnell

unread,
Jan 15, 2020, 5:25:08 AM1/15/20
to jacoc...@googlegroups.com, Marc Hoffmann, rory.o...@oracle.com

Hi Marc,

You could ask if this is a bug on the openjdk amber-dev mailing list (ambe...@openjdk.java.net), subscribe first.

Rgds,Rory

--
You received this message because you are subscribed to the Google Groups "JaCoCo Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jacoco-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jacoco-dev/3534ADCD-50BC-43EC-8AA2-E45E6890E6DC%40mountainminds.com.
-- 
Rgds, Rory O'Donnell
Quality Engineering Manager
Oracle EMEA, Dublin, Ireland

Evgeny Mandrikov

unread,
Jan 15, 2020, 8:50:56 AM1/15/20
to JaCoCo Developers
Hi Marc, Rory,

Thank you, Marc, for raising this point!

As far as I can see changes
for Pattern Matching for instanceof ( https://openjdk.java.net/jeps/305 )
do not describe source-to-bytecode translation strategy.

However in javac it is implemented
not as AST to bytecode transformation,
but as AST to AST transformation - see

compilation of

void example(Object e) {
  if (e instanceof String s) {
    nop(s);
  }
}

produces the same bytecode as compilation of

void example(Object e) {
    String s;
    Object temp = e;
    if (
        (temp instanceof String)
         && ((s = (String) temp) == (String) temp)
    ) {
        nop(s);
    }
}

so to me seems that this additional comparison is a way to place assignment into a condition
to simplify AST to AST transformation for more complex conditions such as

if (e instanceof String s && s.length() > 0)

Seems that you're right that comparison can be avoided
and as far as I can see ECJ plans to implement AST to bytecode transformation without this additional comparison -
see method generateOptimizedBoolean of InstanceOfExpression class

Compilation of following Example.java

class Example {
    void example(Object e) {
        if (e instanceof String s) {
            nop(s);
        }
    }
    
    void nop(String s) {
    }
}

using build of ECJ with this change

cd eclipse.jdt.core
git fetch https://git.eclipse.org/r/jdt/eclipse.jdt.core refs/changes/60/155460/6 && git checkout FETCH_HEAD
mvn package -Pbuild-individual-bundles -DskipTests
java -jar org.eclipse.jdt.core/target/org.eclipse.jdt.core-3.20.100-SNAPSHOT-batch-compiler.jar --enable-preview --release 14 Example.java

produces following bytecode that contains only one comparison

         0: aconst_null
         1: astore_2
         2: aload_1
         3: instanceof    #13                 // class java/lang/String
         6: ifeq          19
         9: aload_1
        10: checkcast     #13                 // class java/lang/String
        13: astore_2
        14: aload_0
        15: aload_2
        16: invokevirtual #15                 // Method nop:(Ljava/lang/String;)V
        19: return

But I guess that such AST to AST transformation
was/is easier to implement in javac than AST to bytecode.

Do you want me to ask confirmation in amber-dev?


Regards,
Evgeny
To unsubscribe from this group and stop receiving emails from it, send an email to jacoco-dev+unsubscribe@googlegroups.com.

Marc Hoffmann

unread,
Jan 15, 2020, 4:05:46 PM1/15/20
to jacoc...@googlegroups.com
Hi Evgeny,

thanks for the detailed analysis and the background information!

Yes, please directly raise the issue at the Amber ML. Maybe there is still a chance to improve the generated bytecode. It would come with smaller class files, less runtime overhead and less workarounds in JaCoCo ;)

Cheers,
-marc

git fetch https://git.eclipse.org/r/jdt/eclipse.jdt.corerefs/changes/60/155460/6 && git checkout FETCH_HEAD
mvn package -Pbuild-individual-bundles -DskipTests
To unsubscribe from this group and stop receiving emails from it, send an email to jacoco-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jacoco-dev/6f280dc1-b566-4b3f-be45-307e0845c627%40googlegroups.com.

Evgeny Mandrikov

unread,
Jan 18, 2020, 8:57:19 AM1/18/20
to JaCoCo Developers


On Wednesday, January 15, 2020 at 10:05:46 PM UTC+1, Marc R. Hoffmann wrote:
Yes, please directly raise the issue at the Amber ML.

Rory O'Donnell

unread,
Jan 18, 2020, 11:21:37 AM1/18/20
to jacoc...@googlegroups.com, Evgeny Mandrikov, rory.o...@oracle.com

Thanks Evgeny, looks like a bug is required - can you send the ID ?

Rgds,Rory

--
You received this message because you are subscribed to the Google Groups "JaCoCo Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jacoco-dev+...@googlegroups.com.

Evgeny Mandrikov

unread,
Jan 20, 2020, 9:54:20 AM1/20/20
to JaCoCo Developers
Hi Rory,


On Saturday, January 18, 2020 at 5:21:37 PM UTC+1, rory.odonnell wrote:

Thanks Evgeny, looks like a bug is required - can you send the ID ?



but let me ask you too - I left "fix version" field empty, but maybe it should be set to "15-pool"?


Regards,
Evgeny

Rory O'Donnell

unread,
Jan 20, 2020, 11:03:36 AM1/20/20
to jacoc...@googlegroups.com, rory.o...@oracle.com

15 should be fine.

Rgds,Rory



Regards,
Evgeny

--
You received this message because you are subscribed to the Google Groups "JaCoCo Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jacoco-dev+...@googlegroups.com.

Evgeny Mandrikov

unread,
Jan 21, 2020, 5:07:29 AM1/21/20
to JaCoCo Developers

On Monday, January 20, 2020 at 5:03:36 PM UTC+1, rory.odonnell wrote:


15 should be fine.



Thanks!


Regards,
Evgeny

Evgeny Mandrikov

unread,
Jan 21, 2020, 6:50:35 AM1/21/20
to JaCoCo Developers
Hi Marc,

I think that we anyway should have validation test on our side - https://github.com/jacoco/jacoco/pull/999


Regards,
Evgeny

Marc Hoffmann

unread,
Jan 21, 2020, 7:21:26 AM1/21/20
to jacoc...@googlegroups.com
Hi Evgeny,

absolutely! That’s why I pushed https://github.com/jacoco/jacoco/tree/experimental-instanceof in the first place ;) 

Will merge your PR once the build is done.

Cheers,
-marc




--
You received this message because you are subscribed to the Google Groups "JaCoCo Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jacoco-dev+...@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages