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 2026/06/06 13:04:32 UTC
(groovy) 01/01: GROOVY-12065: Implement peephole optimization for constant loading in bytecode generation
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch GROOVY-12065
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit eb007e7c0465e96ef860a1f902c55fc2b7fa807c
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sat Jun 6 22:04:10 2026 +0900
GROOVY-12065: Implement peephole optimization for constant loading in bytecode generation
---
.../groovy/classgen/AsmClassGenerator.java | 10 +-
.../groovy/classgen/asm/BytecodeHelper.java | 4 +
.../codehaus/groovy/classgen/asm/OperandStack.java | 57 +------
.../asm/PeepholeOptimizingMethodVisitor.java | 166 +++++++++++++++++++++
.../classgen/asm/BinaryOperationsTest.groovy | 27 +++-
.../asm/PeepholeOptimizingMethodVisitorTest.groovy | 113 ++++++++++++++
6 files changed, 321 insertions(+), 56 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 5a74c2cd24..29e9166d36 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -103,6 +103,7 @@ import org.codehaus.groovy.classgen.asm.MethodCallerMultiAdapter;
import org.codehaus.groovy.classgen.asm.MopWriter;
import org.codehaus.groovy.classgen.asm.OperandStack;
import org.codehaus.groovy.classgen.asm.OptimizingStatementWriter;
+import org.codehaus.groovy.classgen.asm.PeepholeOptimizingMethodVisitor;
import org.codehaus.groovy.classgen.asm.WriterController;
import org.codehaus.groovy.classgen.asm.WriterControllerFactory;
import org.codehaus.groovy.control.SourceUnit;
@@ -627,7 +628,8 @@ public class AsmClassGenerator extends ClassGenerator {
BytecodeHelper.getMethodDescriptor(node.getReturnType(), parameters),
BytecodeHelper.getGenericsMethodSignature(node),
buildExceptions(node.getExceptions()));
- controller.setMethodVisitor(mv);
+ MethodVisitor codeVisitor = new PeepholeOptimizingMethodVisitor(mv);
+ controller.setMethodVisitor(codeVisitor);
controller.resetLineNumber();
visitAnnotations(node, mv);
@@ -669,13 +671,13 @@ public class AsmClassGenerator extends ClassGenerator {
Statement code = node.getCode();
BytecodeInstruction instruction; // fast path for getters, setters, etc.
if (code instanceof BytecodeSequence && (instruction = ((BytecodeSequence) code).getBytecodeInstruction()) != null) {
- instruction.visit(mv);
+ instruction.visit(codeVisitor);
} else {
visitStdMethod(node, isConstructor, parameters, code);
}
try {
- mv.visitMaxs(0, 0);
+ codeVisitor.visitMaxs(0, 0);
} catch (Throwable t) {
Writer writer = null;
if (mv instanceof TraceMethodVisitor) {
@@ -696,7 +698,7 @@ public class AsmClassGenerator extends ClassGenerator {
throw new GroovyRuntimeException(message.toString(), t);
}
}
- mv.visitEnd();
+ codeVisitor.visitEnd();
}
private void visitStdMethod(final MethodNode node, final boolean isConstructor, final Parameter[] parameters, final Statement code) {
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/BytecodeHelper.java b/src/main/java/org/codehaus/groovy/classgen/asm/BytecodeHelper.java
index 4dc160afc8..1187065a13 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/BytecodeHelper.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/BytecodeHelper.java
@@ -69,6 +69,7 @@ import static org.objectweb.asm.Opcodes.ICONST_2;
import static org.objectweb.asm.Opcodes.ICONST_3;
import static org.objectweb.asm.Opcodes.ICONST_4;
import static org.objectweb.asm.Opcodes.ICONST_5;
+import static org.objectweb.asm.Opcodes.ICONST_M1;
import static org.objectweb.asm.Opcodes.IFEQ;
import static org.objectweb.asm.Opcodes.IFNE;
import static org.objectweb.asm.Opcodes.ILOAD;
@@ -249,6 +250,9 @@ public class BytecodeHelper {
*/
public static void pushConstant(MethodVisitor mv, int value) {
switch (value) {
+ case -1:
+ mv.visitInsn(ICONST_M1);
+ break;
case 0:
mv.visitInsn(ICONST_0);
break;
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java b/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java
index acd6f5d99f..4f46a57a1e 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java
@@ -49,13 +49,10 @@ import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveShort;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveVoid;
import static org.objectweb.asm.Opcodes.ACONST_NULL;
import static org.objectweb.asm.Opcodes.ALOAD;
-import static org.objectweb.asm.Opcodes.BIPUSH;
import static org.objectweb.asm.Opcodes.CHECKCAST;
import static org.objectweb.asm.Opcodes.D2F;
import static org.objectweb.asm.Opcodes.D2I;
import static org.objectweb.asm.Opcodes.D2L;
-import static org.objectweb.asm.Opcodes.DCONST_0;
-import static org.objectweb.asm.Opcodes.DCONST_1;
import static org.objectweb.asm.Opcodes.DUP;
import static org.objectweb.asm.Opcodes.DUP2;
import static org.objectweb.asm.Opcodes.DUP2_X1;
@@ -64,9 +61,6 @@ import static org.objectweb.asm.Opcodes.DUP_X2;
import static org.objectweb.asm.Opcodes.F2D;
import static org.objectweb.asm.Opcodes.F2I;
import static org.objectweb.asm.Opcodes.F2L;
-import static org.objectweb.asm.Opcodes.FCONST_0;
-import static org.objectweb.asm.Opcodes.FCONST_1;
-import static org.objectweb.asm.Opcodes.FCONST_2;
import static org.objectweb.asm.Opcodes.GETSTATIC;
import static org.objectweb.asm.Opcodes.I2B;
import static org.objectweb.asm.Opcodes.I2C;
@@ -74,15 +68,11 @@ import static org.objectweb.asm.Opcodes.I2D;
import static org.objectweb.asm.Opcodes.I2F;
import static org.objectweb.asm.Opcodes.I2L;
import static org.objectweb.asm.Opcodes.I2S;
-import static org.objectweb.asm.Opcodes.ICONST_0;
-import static org.objectweb.asm.Opcodes.ICONST_1;
import static org.objectweb.asm.Opcodes.INVOKESPECIAL;
import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL;
import static org.objectweb.asm.Opcodes.L2D;
import static org.objectweb.asm.Opcodes.L2F;
import static org.objectweb.asm.Opcodes.L2I;
-import static org.objectweb.asm.Opcodes.LCONST_0;
-import static org.objectweb.asm.Opcodes.LCONST_1;
import static org.objectweb.asm.Opcodes.NEW;
import static org.objectweb.asm.Opcodes.POP;
import static org.objectweb.asm.Opcodes.POP2;
@@ -165,11 +155,7 @@ public class OperandStack {
MethodVisitor mv = controller.getMethodVisitor();
if (mark == size) {
// no element, so use emptyDefault
- if (emptyDefault) {
- mv.visitIntInsn(BIPUSH, 1);
- } else {
- mv.visitIntInsn(BIPUSH, 0);
- }
+ mv.visitLdcInsn(emptyDefault ? 1 : 0);
stack.add(null);
} else if (mark == size - 1) {
ClassNode last = stack.get(size - 1);
@@ -585,42 +571,11 @@ public class OperandStack {
boolean isChar = isPrimitiveChar(type);
if (isInt || isShort || isByte || isChar) {
int val = isInt ? (Integer) value : isShort ? (Short) value : isChar ? (Character) value : (Byte) value;
- BytecodeHelper.pushConstant(mv, val);
- } else if (isPrimitiveLong(type)) {
- if ((Long) value == 0L) {
- mv.visitInsn(LCONST_0);
- } else if ((Long) value == 1L) {
- mv.visitInsn(LCONST_1);
- } else {
- mv.visitLdcInsn(value);
- }
- } else if (isPrimitiveFloat(type)) {
- // GROOVY-9797: Use Float.equals to differentiate between positive and negative zero
- if (value.equals(0f)) {
- mv.visitInsn(FCONST_0);
- } else if ((Float) value == 1f) {
- mv.visitInsn(FCONST_1);
- } else if ((Float) value == 2f) {
- mv.visitInsn(FCONST_2);
- } else {
- mv.visitLdcInsn(value);
- }
- } else if (isPrimitiveDouble(type)) {
- // GROOVY-9797: Use Double.equals to differentiate between positive and negative zero
- if (value.equals(0d)) {
- mv.visitInsn(DCONST_0);
- } else if ((Double) value == 1d) {
- mv.visitInsn(DCONST_1);
- } else {
- mv.visitLdcInsn(value);
- }
+ mv.visitLdcInsn(val);
+ } else if (isPrimitiveLong(type) || isPrimitiveFloat(type) || isPrimitiveDouble(type)) {
+ mv.visitLdcInsn(value);
} else if (isPrimitiveBoolean(type)) {
- boolean b = (Boolean) value;
- if (b) {
- mv.visitInsn(ICONST_1);
- } else {
- mv.visitInsn(ICONST_0);
- }
+ mv.visitLdcInsn((Boolean) value ? 1 : 0);
} else {
mv.visitLdcInsn(value);
}
@@ -729,7 +684,7 @@ public class OperandStack {
*/
public void pushBool(final boolean value) {
MethodVisitor mv = controller.getMethodVisitor();
- mv.visitInsn(value ? ICONST_1 : ICONST_0);
+ mv.visitLdcInsn(value ? 1 : 0);
push(ClassHelper.boolean_TYPE);
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java b/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java
new file mode 100644
index 0000000000..875aeb741a
--- /dev/null
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.classgen.asm;
+
+import org.codehaus.groovy.control.CompilerConfiguration;
+import org.objectweb.asm.MethodVisitor;
+
+import static org.objectweb.asm.Opcodes.ACONST_NULL;
+import static org.objectweb.asm.Opcodes.BIPUSH;
+import static org.objectweb.asm.Opcodes.DCONST_0;
+import static org.objectweb.asm.Opcodes.DCONST_1;
+import static org.objectweb.asm.Opcodes.FCONST_0;
+import static org.objectweb.asm.Opcodes.FCONST_1;
+import static org.objectweb.asm.Opcodes.FCONST_2;
+import static org.objectweb.asm.Opcodes.ICONST_0;
+import static org.objectweb.asm.Opcodes.ICONST_1;
+import static org.objectweb.asm.Opcodes.ICONST_2;
+import static org.objectweb.asm.Opcodes.ICONST_3;
+import static org.objectweb.asm.Opcodes.ICONST_4;
+import static org.objectweb.asm.Opcodes.ICONST_5;
+import static org.objectweb.asm.Opcodes.ICONST_M1;
+import static org.objectweb.asm.Opcodes.LCONST_0;
+import static org.objectweb.asm.Opcodes.LCONST_1;
+import static org.objectweb.asm.Opcodes.SIPUSH;
+
+/**
+ * Safe bytecode compaction pass inspired by Groovy++'s method-visitor pipeline.
+ * It only rewrites individual constant-load instructions into shorter
+ * equivalents; it does not change stack shape across instructions.
+ */
+public final class PeepholeOptimizingMethodVisitor extends MethodVisitor {
+
+ public PeepholeOptimizingMethodVisitor(final MethodVisitor delegate) {
+ super(CompilerConfiguration.ASM_API_VERSION, delegate);
+ }
+
+ @Override
+ public void visitIntInsn(final int opcode, final int operand) {
+ if (opcode == BIPUSH || opcode == SIPUSH) {
+ emitConstant(operand);
+ return;
+ }
+ super.visitIntInsn(opcode, operand);
+ }
+
+ @Override
+ public void visitLdcInsn(final Object value) {
+ emitConstant(value);
+ }
+
+ private void emitConstant(final Object value) {
+ if (value == null) {
+ super.visitInsn(ACONST_NULL);
+ return;
+ }
+
+ if (value instanceof Integer intValue) {
+ emitIntConstant(intValue);
+ return;
+ }
+
+ if (value instanceof Long longValue) {
+ emitLongConstant(longValue);
+ return;
+ }
+
+ if (value instanceof Float floatValue) {
+ emitFloatConstant(floatValue);
+ return;
+ }
+
+ if (value instanceof Double doubleValue) {
+ emitDoubleConstant(doubleValue);
+ return;
+ }
+
+ super.visitLdcInsn(value);
+ }
+
+ private void emitIntConstant(final int value) {
+ switch (value) {
+ case -1:
+ super.visitInsn(ICONST_M1);
+ return;
+ case 0:
+ super.visitInsn(ICONST_0);
+ return;
+ case 1:
+ super.visitInsn(ICONST_1);
+ return;
+ case 2:
+ super.visitInsn(ICONST_2);
+ return;
+ case 3:
+ super.visitInsn(ICONST_3);
+ return;
+ case 4:
+ super.visitInsn(ICONST_4);
+ return;
+ case 5:
+ super.visitInsn(ICONST_5);
+ return;
+ default:
+ }
+
+ if (value >= Byte.MIN_VALUE && value <= Byte.MAX_VALUE) {
+ super.visitIntInsn(BIPUSH, value);
+ } else if (value >= Short.MIN_VALUE && value <= Short.MAX_VALUE) {
+ super.visitIntInsn(SIPUSH, value);
+ } else {
+ super.visitLdcInsn(value);
+ }
+ }
+
+ private void emitLongConstant(final long value) {
+ if (value == 0L) {
+ super.visitInsn(LCONST_0);
+ } else if (value == 1L) {
+ super.visitInsn(LCONST_1);
+ } else {
+ super.visitLdcInsn(value);
+ }
+ }
+
+ private void emitFloatConstant(final float value) {
+ // Preserve signed zero by comparing raw bit patterns.
+ int rawBits = Float.floatToRawIntBits(value);
+ if (rawBits == Float.floatToRawIntBits(0f)) {
+ super.visitInsn(FCONST_0);
+ } else if (rawBits == Float.floatToRawIntBits(1f)) {
+ super.visitInsn(FCONST_1);
+ } else if (rawBits == Float.floatToRawIntBits(2f)) {
+ super.visitInsn(FCONST_2);
+ } else {
+ super.visitLdcInsn(value);
+ }
+ }
+
+ private void emitDoubleConstant(final double value) {
+ // Preserve signed zero by comparing raw bit patterns.
+ long rawBits = Double.doubleToRawLongBits(value);
+ if (rawBits == Double.doubleToRawLongBits(0d)) {
+ super.visitInsn(DCONST_0);
+ } else if (rawBits == Double.doubleToRawLongBits(1d)) {
+ super.visitInsn(DCONST_1);
+ } else {
+ super.visitLdcInsn(value);
+ }
+ }
+}
diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/BinaryOperationsTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/BinaryOperationsTest.groovy
index 2e8f751783..cc857c5761 100644
--- a/src/test/groovy/org/codehaus/groovy/classgen/asm/BinaryOperationsTest.groovy
+++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/BinaryOperationsTest.groovy
@@ -87,7 +87,12 @@ final class BinaryOperationsTest extends AbstractBytecodeTestCase {
"ICONST_$it",
])
}
- [-1, 6,Byte.MIN_VALUE,Byte.MAX_VALUE].each {
+ assert compile("""\
+ int a = -1
+ """).hasStrictSequence([
+ "ICONST_M1",
+ ])
+ [6,Byte.MIN_VALUE,Byte.MAX_VALUE].each {
assert compile("""\
int a = $it
""").hasStrictSequence([
@@ -110,6 +115,26 @@ final class BinaryOperationsTest extends AbstractBytecodeTestCase {
}
}
+ @Test
+ void testPrimitiveZeroConstants() {
+ assumeFalse(config.indyEnabled)
+ assert compile("""\
+ long a = 0L
+ """).hasStrictSequence([
+ 'LCONST_0',
+ ])
+ assert compile("""\
+ float a = 0f
+ """).hasStrictSequence([
+ 'FCONST_0',
+ ])
+ assert compile("""\
+ double a = 0d
+ """).hasStrictSequence([
+ 'DCONST_0',
+ ])
+ }
+
@Test
void testCharXor() {
assumeFalse(config.indyEnabled)
diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy
new file mode 100644
index 0000000000..9c27404395
--- /dev/null
+++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.classgen.asm
+
+import org.codehaus.groovy.control.CompilerConfiguration
+import org.junit.jupiter.api.Test
+import org.objectweb.asm.MethodVisitor
+import org.objectweb.asm.tree.AbstractInsnNode
+import org.objectweb.asm.tree.IntInsnNode
+import org.objectweb.asm.tree.LdcInsnNode
+import org.objectweb.asm.tree.MethodNode
+import org.objectweb.asm.util.Printer
+
+import static org.objectweb.asm.Opcodes.ACC_PUBLIC
+import static org.objectweb.asm.Opcodes.ACC_STATIC
+import static org.objectweb.asm.Opcodes.RETURN
+
+final class PeepholeOptimizingMethodVisitorTest {
+
+ @Test
+ void compactsNumericConstants() {
+ def instructions = collectInstructions {
+ visitLdcInsn(-1)
+ visitLdcInsn(5)
+ visitLdcInsn(6)
+ visitIntInsn(org.objectweb.asm.Opcodes.SIPUSH, 120)
+ visitLdcInsn(0L)
+ visitLdcInsn(1L)
+ visitLdcInsn(2L)
+ visitLdcInsn(0f)
+ visitLdcInsn(1f)
+ visitLdcInsn(2f)
+ visitLdcInsn(0d)
+ visitLdcInsn(1d)
+ visitInsn(RETURN)
+ }
+
+ assert instructions == [
+ 'ICONST_M1',
+ 'ICONST_5',
+ 'BIPUSH 6',
+ 'BIPUSH 120',
+ 'LCONST_0',
+ 'LCONST_1',
+ 'LDC 2',
+ 'FCONST_0',
+ 'FCONST_1',
+ 'FCONST_2',
+ 'DCONST_0',
+ 'DCONST_1',
+ 'RETURN',
+ ]
+ }
+
+ @Test
+ void preservesSignedZeroConstants() {
+ def instructions = collectInsnNodes {
+ visitLdcInsn(-0.0f)
+ visitLdcInsn(-0.0d)
+ visitInsn(RETURN)
+ }
+
+ assert instructions[0] instanceof LdcInsnNode
+ assert instructions[0].cst.equals(-0.0f)
+ assert instructions[1] instanceof LdcInsnNode
+ assert instructions[1].cst.equals(-0.0d)
+ assert render(instructions[2]) == 'RETURN'
+ }
+
+ private static List<String> collectInstructions(@DelegatesTo(MethodVisitor) Closure emitter) {
+ collectInsnNodes(emitter).collect { AbstractInsnNode insn -> render(insn) }
+ }
+
+ private static List<AbstractInsnNode> collectInsnNodes(@DelegatesTo(MethodVisitor) Closure emitter) {
+ def sink = new MethodNode(CompilerConfiguration.ASM_API_VERSION, ACC_PUBLIC | ACC_STATIC, 'sample', '()V', null, null)
+ def visitor = new PeepholeOptimizingMethodVisitor(sink)
+ visitor.visitCode()
+ emitter.delegate = visitor
+ emitter.resolveStrategy = Closure.DELEGATE_FIRST
+ emitter.call()
+ visitor.visitMaxs(0, 0)
+ visitor.visitEnd()
+ sink.instructions.toArray()
+ .findAll { AbstractInsnNode insn -> insn.opcode >= 0 }
+ }
+
+ private static String render(AbstractInsnNode insn) {
+ switch (insn) {
+ case IntInsnNode:
+ return "${Printer.OPCODES[insn.opcode]} ${insn.operand}"
+ case LdcInsnNode:
+ return "LDC ${insn.cst}"
+ default:
+ return Printer.OPCODES[insn.opcode]
+ }
+ }
+}