Skip to content
Permalink
Browse files

8237528: Inefficient compilation of Pattern Matching for instanceof

Avoiding unnecessary cast and comparison in type test pattern desugaring.

Reviewed-by: forax, mcimadamore
  • Loading branch information
Jan Lahoda
Jan Lahoda committed Jan 29, 2020
1 parent 41f962d commit 2f45d46640326d1f713ce8e5d6baf42d4dfb7f81
@@ -170,8 +170,11 @@ public void visitTypeTest(JCInstanceOf tree) {
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;
@@ -96,27 +96,27 @@ public void run() throws Exception {
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 @@ public void run() throws Exception {
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 @@ public void run() throws Exception {
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]
)
@@ -146,7 +146,9 @@ void error(String msg) {
@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();
}
}
}

@@ -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();
}
}

0 comments on commit 2f45d46

Please sign in to comment.