You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2022/05/14 11:19:01 UTC
[groovy] branch GROOVY_4_0_X updated: GROOVY-10618: SC: optimize `BooleanExpression` and `NotExpression` codes
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_4_0_X by this push:
new a411221a5b GROOVY-10618: SC: optimize `BooleanExpression` and `NotExpression` codes
a411221a5b is described below
commit a411221a5b07d278aca98c9dea808c4163e917d3
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri May 6 17:09:26 2022 -0500
GROOVY-10618: SC: optimize `BooleanExpression` and `NotExpression` codes
(cherry picked from commit 38f73d7b78192a460d27356a65fe758a29ef5648)
---
.../transformers/BooleanExpressionTransformer.java | 124 +++++----
...StaticCompileNullCompareOptimizationTest.groovy | 301 ++++++++++++---------
2 files changed, 246 insertions(+), 179 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
index 671a261d69..2bfaa50151 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BooleanExpressionTransformer.java
@@ -43,31 +43,37 @@ import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.findDG
import static org.objectweb.asm.Opcodes.DUP;
import static org.objectweb.asm.Opcodes.GOTO;
import static org.objectweb.asm.Opcodes.ICONST_0;
+import static org.objectweb.asm.Opcodes.ICONST_1;
import static org.objectweb.asm.Opcodes.IFNONNULL;
import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL;
import static org.objectweb.asm.Opcodes.POP;
class BooleanExpressionTransformer {
- private final StaticCompilationTransformer scTransformer;
+ private final StaticCompilationTransformer transformer;
- BooleanExpressionTransformer(final StaticCompilationTransformer scTransformer) {
- this.scTransformer = scTransformer;
+ BooleanExpressionTransformer(final StaticCompilationTransformer transformer) {
+ this.transformer = transformer;
}
- Expression transformBooleanExpression(final BooleanExpression be) {
- if (!(be instanceof NotExpression || be.getExpression() instanceof BinaryExpression)) {
- ClassNode type = scTransformer.getTypeChooser().resolveType(be.getExpression(), scTransformer.getClassNode());
- return optimizeBooleanExpression(be, type, scTransformer);
+ Expression transformBooleanExpression(final BooleanExpression boolX) {
+ Expression expr = boolX;
+ boolean reverse = false;
+ do { // undo arbitrary nesting of (Boolean|Not)Expressions
+ if (expr instanceof NotExpression) reverse = !reverse;
+ expr = ((BooleanExpression) expr).getExpression();
+ } while (expr instanceof BooleanExpression);
+
+ if (!(expr instanceof BinaryExpression)) {
+ expr = transformer.transform(expr);
+ ClassNode type = transformer.getTypeChooser().resolveType(expr, transformer.getClassNode());
+ Expression opt = new OptimizingBooleanExpression(expr, type);
+ if (reverse) opt = new NotExpression(opt);
+ opt.setSourcePosition(boolX);
+ return opt;
}
- return scTransformer.superTransform(be);
- }
- private static Expression optimizeBooleanExpression(final BooleanExpression be, final ClassNode targetType, final ExpressionTransformer transformer) {
- Expression opt = new OptimizingBooleanExpression(transformer.transform(be.getExpression()), targetType);
- opt.setSourcePosition(be);
- opt.copyNodeMetaData(be);
- return opt;
+ return transformer.superTransform(boolX);
}
//--------------------------------------------------------------------------
@@ -84,7 +90,10 @@ class BooleanExpressionTransformer {
@Override
public Expression transformExpression(final ExpressionTransformer transformer) {
- return optimizeBooleanExpression(this, type, transformer);
+ Expression opt = new OptimizingBooleanExpression(transformer.transform(getExpression()), type);
+ opt.setSourcePosition(this);
+ opt.copyNodeMetaData(this);
+ return opt;
}
@Override
@@ -94,14 +103,21 @@ class BooleanExpressionTransformer {
MethodVisitor mv = controller.getMethodVisitor();
OperandStack os = controller.getOperandStack();
- if (ClassHelper.isPrimitiveBoolean(type)) {
- getExpression().visit(visitor);
- os.doGroovyCast(ClassHelper.boolean_TYPE);
+ int mark = os.getStackLength();
+ getExpression().visit(visitor);
+
+ if (ClassHelper.isPrimitiveType(type)) {
+ if (ClassHelper.isPrimitiveBoolean(type)) {
+ // TODO: maybe: os.castToBool(mark, true);
+ os.doGroovyCast(ClassHelper.boolean_TYPE);
+ } else {
+ BytecodeHelper.convertPrimitiveToBoolean(mv, type);
+ os.replace(ClassHelper.boolean_TYPE);
+ }
return;
}
if (ClassHelper.isWrapperBoolean(type)) {
- getExpression().visit(visitor);
Label unbox = new Label();
Label exit = new Label();
// check for null
@@ -118,41 +134,55 @@ class BooleanExpressionTransformer {
return;
}
- ClassNode top = type;
- if (ClassHelper.isPrimitiveType(top)) {
- getExpression().visit(visitor);
- // in case of null-safe invocation, it is possible that what
- // was supposed to be a primitive type becomes "null" value,
- // so we need to recheck
- top = os.getTopOperand();
- if (ClassHelper.isPrimitiveType(top)) {
- BytecodeHelper.convertPrimitiveToBoolean(mv, top);
- os.replace(ClassHelper.boolean_TYPE);
- return;
- }
+ mv.visitInsn(DUP);
+ Label end = new Label();
+ Label asBoolean = new Label();
+ mv.visitJumpInsn(IFNONNULL, asBoolean);
+
+ // null => false
+ mv.visitInsn(POP);
+ mv.visitInsn(ICONST_0);
+ mv.visitJumpInsn(GOTO, end);
+
+ mv.visitLabel(asBoolean);
+ ClassLoader loader = controller.getSourceUnit().getClassLoader();
+ if (replaceAsBooleanWithCompareToNull(type, loader)) {
+ // value => true
+ mv.visitInsn(POP);
+ mv.visitInsn(ICONST_1);
+ } else {
+ os.castToBool(mark, true);
}
+ mv.visitLabel(end);
+ os.replace(ClassHelper.boolean_TYPE);
+ return;
+ }
+
+ super.visit(visitor);
+ }
- List<MethodNode> asBoolean = findDGMMethodsByNameAndArguments(controller.getSourceUnit().getClassLoader(), top, "asBoolean", ClassNode.EMPTY_ARRAY);
+ /**
+ * Inline an "expr != null" check instead of boolean conversion iff:
+ * (1) the class doesn't define an asBoolean method (already tested)
+ * (2) no subclass defines an asBoolean method
+ * For (2), check that we are in one of these cases:
+ * (a) a final class
+ * (b) an effectively-final inner class
+ */
+ private static boolean replaceAsBooleanWithCompareToNull(final ClassNode type, final ClassLoader dgmProvider) {
+ if (Modifier.isFinal(type.getModifiers()) || isEffectivelyFinal(type)) {
+ List<MethodNode> asBoolean = findDGMMethodsByNameAndArguments(dgmProvider, type, "asBoolean", ClassNode.EMPTY_ARRAY);
if (asBoolean.size() == 1) {
- MethodNode method = asBoolean.get(0);
- if (method instanceof ExtensionMethodNode) {
- ClassNode selfType = (((ExtensionMethodNode) method).getExtensionMethodNode()).getParameters()[0].getType();
- // we may inline a var!=null check instead of calling a helper method iff
- // (1) the class doesn't define an asBoolean method (already tested)
- // (2) no subclass defines an asBoolean method
- // For (2), check that we are in one of these cases:
- // (a) a final class
- // (b) an effectively-final inner class
- if (ClassHelper.isObjectType(selfType) && (Modifier.isFinal(top.getModifiers()) || isEffectivelyFinal(top))) {
- Expression opt = new CompareToNullExpression(getExpression(), false);
- opt.visit(visitor);
- return;
+ MethodNode theAsBoolean = asBoolean.get(0);
+ if (theAsBoolean instanceof ExtensionMethodNode) {
+ ClassNode selfType = (((ExtensionMethodNode) theAsBoolean).getExtensionMethodNode()).getParameters()[0].getType();
+ if (ClassHelper.isObjectType(selfType)) {
+ return true;
}
}
}
}
-
- super.visit(visitor);
+ return false;
}
private static boolean isEffectivelyFinal(final ClassNode type) {
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileNullCompareOptimizationTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileNullCompareOptimizationTest.groovy
index 3adf4760ea..8adae6e991 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileNullCompareOptimizationTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileNullCompareOptimizationTest.groovy
@@ -19,71 +19,60 @@
package org.codehaus.groovy.classgen.asm.sc
import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase
-import static org.codehaus.groovy.control.CompilerConfiguration.DEFAULT as config
/**
* Unit tests for static compilation: null test optimizations.
*/
-class StaticCompileNullCompareOptimizationTest extends AbstractBytecodeTestCase {
- void testShouldUseIfNonNull() {
+final class StaticCompileNullCompareOptimizationTest extends AbstractBytecodeTestCase {
+
+ void testShouldUseIfNull1() {
def bytecode = compile(method:'m', '''
@groovy.transform.CompileStatic
void m(Object o) {
- o == null
+ o != null
}
''')
- assert bytecode.hasStrictSequence([
- 'IFNONNULL'
- ])
+ assert bytecode.hasStrictSequence(['IFNULL'])
}
- void testShouldUseIfNull() {
+
+ void testShouldUseIfNull2() {
def bytecode = compile(method:'m', '''
@groovy.transform.CompileStatic
void m(Object o) {
- o != null
+ null != o
}
''')
- assert bytecode.hasStrictSequence([
- 'IFNULL'
- ])
+ assert bytecode.hasStrictSequence(['IFNULL'])
}
- void testShouldUseIfNonNull2() {
+ void testShouldUseIfNonNull1() {
def bytecode = compile(method:'m', '''
@groovy.transform.CompileStatic
void m(Object o) {
- null == o
+ o == null
}
''')
- assert bytecode.hasStrictSequence([
- 'IFNONNULL'
- ])
+ assert bytecode.hasStrictSequence(['IFNONNULL'])
}
- void testShouldUseIfNull2() {
+ void testShouldUseIfNonNull2() {
def bytecode = compile(method:'m', '''
@groovy.transform.CompileStatic
void m(Object o) {
- null != o
+ null == o
}
''')
- assert bytecode.hasStrictSequence([
- 'IFNULL'
- ])
+ assert bytecode.hasStrictSequence(['IFNONNULL'])
}
- void testPrimitiveWithNullShouldBeOptimized() {
+ void testPrimitiveWithNullShouldBeOptimized1() {
def bytecode = compile(method:'m', '''
@groovy.transform.CompileStatic
void m(int x) {
null == x
}
''')
- assert bytecode.hasStrictSequence([
- 'ICONST_0',
- 'POP'
- ])
-
+ assert bytecode.hasStrictSequence(['ICONST_0', 'POP'])
}
void testPrimitiveWithNullShouldBeOptimized2() {
@@ -93,185 +82,233 @@ class StaticCompileNullCompareOptimizationTest extends AbstractBytecodeTestCase
x == null
}
''')
- assert bytecode.hasStrictSequence([
- 'ICONST_0',
- 'POP'
+ assert bytecode.hasStrictSequence(['ICONST_0', 'POP'])
+ }
+
+ void testOptimizeGroovyTruthForPrimitiveBoolean1() {
+ def bytecode = compile(method:'m', '''
+ @groovy.transform.CompileStatic
+ void m(boolean x) {
+ if (x) {
+ }
+ }
+ ''')
+ assert bytecode.hasSequence([
+ 'ILOAD 1',
+ 'IFEQ L1',
+ 'L1',
+ 'RETURN'
])
}
+ void testOptimizeGroovyTruthForPrimitiveBoolean2() {
+ def bytecode = compile(method:'m', '''
+ @groovy.transform.CompileStatic
+ void m(boolean x) {
+ if (!x) {
+ }
+ }
+ ''')
+ assert bytecode.hasSequence([
+ 'ILOAD 1',
+ 'IFNE L1',
+ 'ICONST_1',
+ 'GOTO L2',
+ 'L1',
+ 'ICONST_0',
+ 'L2',
+ 'IFEQ L3',
+ 'L3',
+ 'RETURN'
+ ])
+ }
- void testOptimizeGroovyTruthForPrimitiveBoolean() {
+ void testOptimizeGroovyTruthForPrimitiveBoolean3() {
def bytecode = compile(method:'m', '''
@groovy.transform.CompileStatic
void m(boolean x) {
- if (x) {
- println 'ok'
+ if (!!x) {
}
}
''')
- assert bytecode.hasStrictSequence(['ILOAD 1', 'IFEQ'])
+ assert bytecode.hasSequence([
+ 'ILOAD 1',
+ 'IFEQ L1',
+ 'L1',
+ 'RETURN'
+ ])
}
- void testOptimizeGroovyTruthForBoxedBoolean() {
+ void testOptimizeGroovyTruthForNonPrimitiveBoolean() {
def bytecode = compile(method:'m', '''
@groovy.transform.CompileStatic
void m(Boolean x) {
if (x) {
- println 'ok'
}
}
''')
- if (config.indyEnabled) {
- return
- }
- assert bytecode.hasStrictSequence(['ALOAD 1', 'DUP', 'IFNONNULL', 'POP', 'ICONST_0', 'GOTO', 'L1', 'INVOKEVIRTUAL', 'L2', 'IFEQ']) ||
- bytecode.hasStrictSequence(['ALOAD 1', 'DUP', 'IFNONNULL', 'POP', 'ICONST_0', 'GOTO', 'L1', 'FRAME SAME1', 'INVOKEVIRTUAL', 'L2', 'FRAME SAME1', 'IFEQ']) // bytecode with stack map frame
+ assert bytecode.hasSequence([
+ 'ALOAD 1',
+ 'DUP',
+ 'IFNONNULL',
+ 'POP',
+ 'ICONST_0',
+ 'GOTO',
+ 'L1',
+ 'INVOKEVIRTUAL',
+ 'L2',
+ 'IFEQ'
+ ])
}
- void testOptimizeGroovyTruthWithStringShouldNotBeTriggered() {
+ void testOptimizeGroovyTruthForPrimitiveNumberType() {
def bytecode = compile(method:'m', '''
@groovy.transform.CompileStatic
- void m(String x) {
+ void m(int x) {
if (x) {
- println 'ok'
}
}
''')
- if (config.indyEnabled) {
- assert bytecode.hasStrictSequence([
- 'ALOAD 1',
- 'INVOKEDYNAMIC cast(Ljava/lang/String;)Z',
- '',
- '',
- '',
- '',
- '',
- ']',
- 'IFEQ'
- ])
- } else {
- assert bytecode.hasStrictSequence([
- 'ALOAD 1',
- 'INVOKESTATIC org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.booleanUnbox (Ljava/lang/Object;)Z',
- 'IFEQ'
- ])
- }
+ assert bytecode.hasSequence([
+ 'ILOAD 1',
+ 'IFEQ L1',
+ 'ICONST_1',
+ 'GOTO L2',
+ 'L1',
+ 'ICONST_0',
+ 'L2',
+ 'IFEQ L3',
+ 'L3',
+ 'RETURN'
+ ])
}
- void testGroovyTruthOptimizationWithObjectShouldNotBeTriggered() {
+ void testNoGroovyTruthOptimizationForObject() {
def bytecode = compile(method:'m', '''
@groovy.transform.CompileStatic
void m(Object x) {
if (x) {
- println 'ok'
}
}
''')
- if (config.indyEnabled) {
- assert bytecode.hasStrictSequence([
- 'ALOAD 1',
- 'INVOKEDYNAMIC cast(Ljava/lang/Object;)Z',
- '',
- '',
- '',
- '',
- '',
- ']',
- 'IFEQ'
- ])
- } else {
- assert bytecode.hasStrictSequence([
- 'ALOAD 1',
- 'INVOKESTATIC org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.booleanUnbox (Ljava/lang/Object;)Z',
- 'IFEQ'
- ])
- }
+ assert bytecode.hasSequence([
+ 'ALOAD 1',
+ 'DUP',
+ 'IFNONNULL L1',
+ 'POP',
+ 'ICONST_0',
+ 'GOTO L2',
+ 'L1',
+ 'INVOKEDYNAMIC cast(Ljava/lang/Object;)Z'
+ ])
+ }
+
+ void testNoGroovyTruthOptimizationForString() {
+ def bytecode = compile(method:'m', '''
+ @groovy.transform.CompileStatic
+ void m(String x) {
+ if (x) {
+ }
+ }
+ ''')
+ assert bytecode.hasSequence([
+ 'ALOAD 1',
+ 'DUP',
+ 'IFNONNULL L1',
+ 'POP',
+ 'ICONST_0',
+ 'GOTO L2',
+ 'L1',
+ 'INVOKEDYNAMIC cast(Ljava/lang/String;)Z'
+ ])
}
- void testGroovyTruthOptimizationWithFinalClass() {
+ void testGroovyTruthOptimizationForFinalClass() {
def bytecode = compile(method:'m', '''
- final class A {}
+ final class A {
+ }
@groovy.transform.CompileStatic
void m(A x) {
if (x) {
- println 'ok'
}
}
''')
- assert bytecode.hasStrictSequence([
- 'ALOAD 1',
- 'IFNULL',
+ assert bytecode.hasSequence([
+ 'ALOAD 1',
+ 'DUP',
+ 'IFNONNULL L1',
+ 'POP',
+ 'ICONST_0',
+ 'GOTO L2',
+ 'POP',
+ 'ICONST_1'
])
+ assert !bytecode.hasSequence(['INVOKEDYNAMIC cast(LA;)Z'])
}
- void testGroovyTruthOptimizationWithPrivateInnerClass() {
+ void testGroovyTruthOptimizationForPrivateInnerClass() {
def bytecode = compile(method:'m', '''
class A {
- private static class B {}
+ private static class B {
+ }
@groovy.transform.CompileStatic
void m(B x) {
if (x) {
- println 'ok'
}
}
}
''')
- assert bytecode.hasStrictSequence([
- 'ALOAD 1',
- 'IFNULL',
+ assert bytecode.hasSequence([
+ 'ALOAD 1',
+ 'DUP',
+ 'IFNONNULL L1',
+ 'POP',
+ 'ICONST_0',
+ 'GOTO L2',
+ 'POP',
+ 'ICONST_1'
])
+ assert !bytecode.hasSequence(['INVOKEDYNAMIC cast(LA$B;)Z'])
}
- void testGroovyTruthOptimizationWithPublicInnerClass() {
+ void testNoGroovyTruthOptimizationForPublicInnerClass() {
def bytecode = compile(method:'m', '''
class A {
- public static class B {}
+ public static class B {
+ }
@groovy.transform.CompileStatic
void m(B x) {
if (x) {
- println 'ok'
}
}
}
''')
- if (config.indyEnabled) {
- assert bytecode.hasStrictSequence([
- 'ALOAD 1',
- 'INVOKEDYNAMIC cast(LA$B;)Z',
- '',
- '',
- '',
- '',
- '',
- ']',
- 'IFEQ'
- ])
- } else {
- assert bytecode.hasStrictSequence([
- 'ALOAD 1',
- 'INVOKESTATIC org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.booleanUnbox (Ljava/lang/Object;)Z',
- 'IFEQ'
- ])
- }
+ assert bytecode.hasSequence([
+ 'ALOAD 1',
+ 'DUP',
+ 'IFNONNULL L1',
+ 'POP',
+ 'ICONST_0',
+ 'GOTO L2',
+ 'L1',
+ 'INVOKEDYNAMIC cast(LA$B;)Z'
+ ])
}
void testCompare() {
- def bytecode=compile(method:'stat', '''
- class Doc {}
-
+ assertScript '''
+ class Pogo {
+ }
@groovy.transform.CompileStatic
- class A {
- static void foo() {
- Doc doc = null
- def cl = { if (doc) { 1 } else { 0 } }
- assert cl() == 0
+ class C {
+ static test() {
+ Pogo pogo = null
+ def check = { -> if (pogo) { 1 } else { 0 } }
+ assert check() == 0
}
}
- A.foo()
-
- ''')
- clazz.main()
+ C.test()
+ '''
}
-
}