changeset 57860:2a46b7b81e78

8237528: Inefficient compilation of Pattern Matching for instanceof Summary: Avoiding unnecessary cast and comparison in type test pattern desugaring. Reviewed-by: forax, mcimadamore
author jlahoda
date Wed, 29 Jan 2020 10:37:22 +0100
parents d37576456de2
children 9fb094231eee
files src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java test/langtools/tools/javac/annotations/typeAnnotations/classfile/Patterns.java test/langtools/tools/javac/patterns/LocalVariableTable.java test/langtools/tools/javac/patterns/NoUnnecessaryCast.java
diffstat 4 files changed, 133 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java	Wed Jan 29 09:29:22 2020 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java	Wed Jan 29 10:37:22 2020 +0100
@@ -170,8 +170,11 @@
             if (bindingVar != null) { //TODO: cannot be null here?
                 JCAssign fakeInit = (JCAssign)make.at(tree.pos).Assign(
                         make.Ident(bindingVar), convert(make.Ident(temp), castTargetType)).setType(bindingVar.erasure(types));
-                result = makeBinary(Tag.AND, (JCExpression)result,
-                        makeBinary(Tag.EQ, fakeInit, convert(make.Ident(temp), castTargetType)));
+                LetExpr nestedLE = make.LetExpr(List.of(make.Exec(fakeInit)),
+                                                make.Literal(true));
+                nestedLE.needsCond = true;
+                nestedLE.setType(syms.booleanType);
+                result = makeBinary(Tag.AND, (JCExpression)result, nestedLE);
             }
             result = make.at(tree.pos).LetExpr(make.VarDef(temp, translatedExpr), (JCExpression)result).setType(syms.booleanType);
             ((LetExpr) result).needsCond = true;
--- a/test/langtools/tools/javac/annotations/typeAnnotations/classfile/Patterns.java	Wed Jan 29 09:29:22 2020 +0100
+++ b/test/langtools/tools/javac/annotations/typeAnnotations/classfile/Patterns.java	Wed Jan 29 10:37:22 2020 +0100
@@ -96,27 +96,27 @@
                               descriptor: ()V
                               flags: (0x0001) ACC_PUBLIC
                                 RuntimeInvisibleTypeAnnotations:
-                                  0: #_A_(): LOCAL_VARIABLE, {start_pc=257, length=18, index=2}
+                                  0: #_A_(): LOCAL_VARIABLE, {start_pc=206, length=11, index=2}
                                     Patterns$SimpleBindingPattern$A
-                                  1: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=297, length=19, index=3}
+                                  1: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=238, length=11, index=3}
                                     Patterns$SimpleBindingPattern$CA(
                                       value=[@Patterns$SimpleBindingPattern$A,@Patterns$SimpleBindingPattern$A]
                                     )
-                                  2: #_A_(): LOCAL_VARIABLE, {start_pc=22, length=18, index=1}
+                                  2: #_A_(): LOCAL_VARIABLE, {start_pc=21, length=11, index=1}
                                     Patterns$SimpleBindingPattern$A
-                                  3: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=62, length=18, index=1}
+                                  3: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=53, length=11, index=1}
                                     Patterns$SimpleBindingPattern$CA(
                                       value=[@Patterns$SimpleBindingPattern$A,@Patterns$SimpleBindingPattern$A]
                                     )
-                                  4: #_A_(): LOCAL_VARIABLE, {start_pc=101, length=18, index=2}
+                                  4: #_A_(): LOCAL_VARIABLE, {start_pc=84, length=11, index=2}
                                     Patterns$SimpleBindingPattern$A
-                                  5: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=141, length=19, index=3}
+                                  5: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=116, length=11, index=3}
                                     Patterns$SimpleBindingPattern$CA(
                                       value=[@Patterns$SimpleBindingPattern$A,@Patterns$SimpleBindingPattern$A]
                                     )
-                                  6: #_A_(): LOCAL_VARIABLE, {start_pc=179, length=18, index=2}
+                                  6: #_A_(): LOCAL_VARIABLE, {start_pc=145, length=11, index=2}
                                     Patterns$SimpleBindingPattern$A
-                                  7: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=219, length=19, index=3}
+                                  7: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=177, length=11, index=3}
                                     Patterns$SimpleBindingPattern$CA(
                                       value=[@Patterns$SimpleBindingPattern$A,@Patterns$SimpleBindingPattern$A]
                                     )
@@ -125,9 +125,9 @@
                               descriptor: ()V
                               flags: (0x0000)
                                 RuntimeInvisibleTypeAnnotations:
-                                  0: #_A_(): LOCAL_VARIABLE, {start_pc=17, length=18, index=2}
+                                  0: #_A_(): LOCAL_VARIABLE, {start_pc=16, length=11, index=2}
                                     Patterns$SimpleBindingPattern$A
-                                  1: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=57, length=19, index=3}
+                                  1: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=48, length=11, index=3}
                                     Patterns$SimpleBindingPattern$CA(
                                       value=[@Patterns$SimpleBindingPattern$A,@Patterns$SimpleBindingPattern$A]
                                     )
@@ -143,15 +143,15 @@
                               descriptor: ()V
                               flags: (0x0008) ACC_STATIC
                                 RuntimeInvisibleTypeAnnotations:
-                                  0: #_A_(): LOCAL_VARIABLE, {start_pc=22, length=18, index=0}
+                                  0: #_A_(): LOCAL_VARIABLE, {start_pc=21, length=11, index=0}
                                     Patterns$SimpleBindingPattern$A
-                                  1: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=61, length=18, index=0}
+                                  1: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=52, length=11, index=0}
                                     Patterns$SimpleBindingPattern$CA(
                                       value=[@Patterns$SimpleBindingPattern$A,@Patterns$SimpleBindingPattern$A]
                                     )
-                                  2: #_A_(): LOCAL_VARIABLE, {start_pc=100, length=18, index=1}
+                                  2: #_A_(): LOCAL_VARIABLE, {start_pc=83, length=11, index=1}
                                     Patterns$SimpleBindingPattern$A
-                                  3: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=137, length=18, index=2}
+                                  3: #_CA_(#_value_=[@#_A_(),@#_A_()]): LOCAL_VARIABLE, {start_pc=112, length=11, index=2}
                                     Patterns$SimpleBindingPattern$CA(
                                       value=[@Patterns$SimpleBindingPattern$A,@Patterns$SimpleBindingPattern$A]
                                     )
--- a/test/langtools/tools/javac/patterns/LocalVariableTable.java	Wed Jan 29 09:29:22 2020 +0100
+++ b/test/langtools/tools/javac/patterns/LocalVariableTable.java	Wed Jan 29 10:37:22 2020 +0100
@@ -146,7 +146,9 @@
     @Expect({ "o", "s" })
     static class Pattern_Simple {
         public static void test(Object o) {
-            if (o instanceof String s) {}
+            if (o instanceof String s) {
+                s.length();
+            }
         }
     }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/patterns/NoUnnecessaryCast.java	Wed Jan 29 10:37:22 2020 +0100
@@ -0,0 +1,111 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8237528
+ * @summary Verify there are no unnecessary checkcasts and conditions generated
+ *          for the pattern matching in instanceof.
+ * @modules jdk.jdeps/com.sun.tools.classfile
+ * @compile --enable-preview -source ${jdk.version} NoUnnecessaryCast.java
+ * @run main/othervm --enable-preview NoUnnecessaryCast
+ */
+
+import java.io.File;
+import java.io.IOException;
+
+import com.sun.tools.classfile.Attribute;
+import com.sun.tools.classfile.ClassFile;
+import com.sun.tools.classfile.Code_attribute;
+import com.sun.tools.classfile.Code_attribute.InvalidIndex;
+import com.sun.tools.classfile.ConstantPool;
+import com.sun.tools.classfile.ConstantPoolException;
+import com.sun.tools.classfile.Descriptor.InvalidDescriptor;
+import com.sun.tools.classfile.Instruction;
+import com.sun.tools.classfile.Method;
+import java.util.Arrays;
+import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
+
+public class NoUnnecessaryCast {
+    public static void main(String[] args)
+            throws IOException, ConstantPoolException, InvalidDescriptor, InvalidIndex {
+        new NoUnnecessaryCast()
+                .checkClassFile(new File(System.getProperty("test.classes", "."),
+                    NoUnnecessaryCast.class.getName() + ".class"));
+    }
+
+    void checkClassFile(File file)
+            throws IOException, ConstantPoolException, InvalidDescriptor, InvalidIndex {
+        ClassFile classFile = ClassFile.read(file);
+        ConstantPool constantPool = classFile.constant_pool;
+
+        Method method = Arrays.stream(classFile.methods)
+                              .filter(m -> getName(m, constantPool)
+                                               .equals("test"))
+                              .findAny()
+                              .get();
+        String expectedInstructions = """
+                                      aload_1
+                                      astore_3
+                                      aload_3
+                                      instanceof
+                                      ifeq
+                                      aload_3
+                                      checkcast
+                                      astore_2
+                                      aload_2
+                                      invokevirtual
+                                      ifeq
+                                      iconst_1
+                                      goto
+                                      iconst_0
+                                      ireturn
+                                      """;
+        Code_attribute code = (Code_attribute) method.attributes
+                .get(Attribute.Code);
+        String actualInstructions = printCode(code);
+        if (!expectedInstructions.equals(actualInstructions)) {
+            throw new AssertionError("Unexpected instructions found:\n" +
+                                     actualInstructions);
+        }
+    }
+
+    String printCode(Code_attribute code) {
+        return StreamSupport.stream(code.getInstructions().spliterator(), false)
+                            .map(Instruction::getMnemonic)
+                            .collect(Collectors.joining("\n", "", "\n"));
+    }
+
+    String getName(Method m, ConstantPool constantPool) {
+        try {
+            return m.getName(constantPool);
+        } catch (ConstantPoolException ex) {
+            throw new IllegalStateException(ex);
+        }
+    }
+
+    boolean test(Object o) {
+        return o instanceof String s && s.isEmpty();
+    }
+}