+ * Default true (safe). Flipped to false by + * {@link org.perlonjava.frontend.analysis.CleanupNeededVisitor} when + * the sub body is statically proven to have no bless / weaken / + * local / nested-sub / defer activity — in which case scope-exit + * emissions can be dropped to just the null-store sequence, + * matching the fast path master uses for simple numeric loops. + *
+ * {@link EmitStatement#emitScopeExitNullStores} honours this flag. + * The env var {@code JPERL_FORCE_CLEANUP=1} forces this to true + * globally for debugging suspected correctness regressions. + */ + public boolean cleanupNeeded = true; + /** * JVM local variable indices of my-variables (scalar, hash, array) allocated * inside the eval body. Used by the eval catch handler to emit scope-exit diff --git a/src/main/java/org/perlonjava/core/Configuration.java b/src/main/java/org/perlonjava/core/Configuration.java index e8767d879..e8a6d2754 100644 --- a/src/main/java/org/perlonjava/core/Configuration.java +++ b/src/main/java/org/perlonjava/core/Configuration.java @@ -33,7 +33,7 @@ public final class Configuration { * Automatically populated by Gradle/Maven during build. * DO NOT EDIT MANUALLY - this value is replaced at build time. */ - public static final String gitCommitId = "b7d05b77e"; + public static final String gitCommitId = "078e0b3d7"; /** * Git commit date of the build (ISO format: YYYY-MM-DD). @@ -48,7 +48,7 @@ public final class Configuration { * Parsed by App::perlbrew and other tools via: perl -V | grep "Compiled at" * DO NOT EDIT MANUALLY - this value is replaced at build time. */ - public static final String buildTimestamp = "Apr 21 2026 21:47:23"; + public static final String buildTimestamp = "Apr 21 2026 23:11:19"; // Prevent instantiation private Configuration() { diff --git a/src/main/java/org/perlonjava/frontend/analysis/CleanupNeededVisitor.java b/src/main/java/org/perlonjava/frontend/analysis/CleanupNeededVisitor.java new file mode 100644 index 000000000..141483285 --- /dev/null +++ b/src/main/java/org/perlonjava/frontend/analysis/CleanupNeededVisitor.java @@ -0,0 +1,260 @@ +package org.perlonjava.frontend.analysis; + +import org.perlonjava.frontend.astnode.*; + +/** + * Determines whether a subroutine needs the full per-scope-exit cleanup + * machinery (scopeExitCleanup, MyVarCleanupStack.unregister, full + * MortalList flush) or can safely skip it. + * + *
Ultra-hot workloads (tight numeric loops, life_bitpacked, etc.) + * pay ~1 INVOKESTATIC per {@code my}-variable per scope exit for the + * refcount/DESTROY/weaken bookkeeping — even when the sub's lexicals + * are plain integers and refcount_owned never flips to true. Skipping + * this emission when statically provably unnecessary recovers a large + * fraction of the per-iteration cost. + * + *
A sub is "simple" (cleanup not needed) iff its body contains + * NONE of: + *
Builtins like {@code print}, {@code push}, {@code chr}, + * {@code length}, etc. are parsed as {@link OperatorNode} (not as + * {@link BinaryOperatorNode} with the {@code "("} operator), so they + * don't hit this visitor's sub-call branch — they return non-blessed + * values and don't need cleanup. Overrideable builtins that + * the user imported via {@code use subs} are already resolved by the + * parser to user sub calls ({@code BinaryOperatorNode("(", ...)}), + * which DO get flagged here, so the compile-time override decision + * is handled correctly without extra work from this visitor. + * + *
This is the "simple leaf function" heuristic. It's deliberately + * conservative; false positives (marking needsCleanup when it wasn't + * strictly required) just revert to current behavior. False negatives + * (marking skip when cleanup IS needed) would be a correctness bug — + * hence the env-var escape hatch {@code JPERL_FORCE_CLEANUP=1} in + * {@code EmitStatement} to force the slow path for debugging. + */ +public class CleanupNeededVisitor implements Visitor { + + private boolean needsCleanup = false; + + /** + * @return true iff scope-exit cleanup emission is required for + * correctness. Callers should only skip cleanup when this is false. + */ + public boolean needsCleanup() { + return needsCleanup; + } + + public void reset() { + needsCleanup = false; + } + + // Short-circuit: once we've decided cleanup is needed, don't bother + // walking further subtrees (but we still have to satisfy the visitor + // contract; the recursion is short in practice). + private void mark() { + needsCleanup = true; + } + + @Override + public void visit(OperatorNode node) { + if (needsCleanup) return; + // local operator is a scope-exit bookkeeping trigger. + if ("local".equals(node.operator)) { + mark(); + return; + } + if (node.operand != null) node.operand.accept(this); + } + + @Override + public void visit(BinaryOperatorNode node) { + if (needsCleanup) return; + // bless always taints a sub: the blessed target needs refCount + // tracking for DESTROY. + if ("bless".equals(node.operator)) { + mark(); + return; + } + // User sub call: func(args) → BinaryOperatorNode("(", callee, args). + // Callee might return a blessed-with-DESTROY that lands in a + // lexical in this sub, so cleanup is required even if this sub + // itself does no bless. Builtins (push, chr, length, etc.) are + // parsed as OperatorNode, so only user subs hit this branch. + // + // Special-case weaken/isweak: they flip global state too, but + // marking via the general sub-call path handles them + // automatically. Kept explicit for documentation clarity. + if ("(".equals(node.operator)) { + if (node.left instanceof IdentifierNode id) { + String name = id.name; + if (name != null && ( + name.equals("weaken") || name.equals("isweak") + || name.equals("Scalar::Util::weaken") + || name.equals("Scalar::Util::isweak"))) { + mark(); + return; + } + } + // Any other user sub call. + mark(); + return; + } + // Method call: $obj->method or $obj->method(args). + // In AST form, the RHS is either IdentifierNode(method_name) or + // BinaryOperatorNode("(", IdentifierNode(method_name), args). + // Array/hash derefs ($x->[i], $x->{k}) have ArrayLiteralNode / + // HashLiteralNode on the RHS — those are safe (no user code runs). + if ("->".equals(node.operator)) { + Node right = node.right; + if (right instanceof IdentifierNode + || right instanceof BinaryOperatorNode binOp && "(".equals(binOp.operator)) { + mark(); + return; + } + // Array/hash deref — recurse into children only. + if (node.left != null) node.left.accept(this); + if (node.right != null) node.right.accept(this); + return; + } + if (node.left != null) node.left.accept(this); + if (node.right != null) node.right.accept(this); + } + + @Override + public void visit(SubroutineNode node) { + // Nested subroutines might capture our lexicals via closure. + // Conservatively assume they do. + mark(); + // No need to recurse into the body — inner subs run their own + // CleanupNeededVisitor. + } + + @Override + public void visit(BlockNode node) { + if (needsCleanup) return; + for (Node element : node.elements) { + if (needsCleanup) return; + if (element != null) element.accept(this); + } + } + + @Override + public void visit(ListNode node) { + if (needsCleanup) return; + for (Node element : node.elements) { + if (needsCleanup) return; + if (element != null) element.accept(this); + } + } + + @Override + public void visit(HashLiteralNode node) { + if (needsCleanup) return; + for (Node element : node.elements) { + if (needsCleanup) return; + if (element != null) element.accept(this); + } + } + + @Override + public void visit(ArrayLiteralNode node) { + if (needsCleanup) return; + for (Node element : node.elements) { + if (needsCleanup) return; + if (element != null) element.accept(this); + } + } + + @Override + public void visit(IfNode node) { + if (needsCleanup) return; + if (node.condition != null) node.condition.accept(this); + if (node.thenBranch != null) node.thenBranch.accept(this); + if (node.elseBranch != null) node.elseBranch.accept(this); + } + + @Override + public void visit(TernaryOperatorNode node) { + if (needsCleanup) return; + if (node.condition != null) node.condition.accept(this); + if (node.trueExpr != null) node.trueExpr.accept(this); + if (node.falseExpr != null) node.falseExpr.accept(this); + } + + @Override + public void visit(For1Node node) { + if (needsCleanup) return; + if (node.variable != null) node.variable.accept(this); + if (node.list != null) node.list.accept(this); + if (node.body != null) node.body.accept(this); + } + + @Override + public void visit(For3Node node) { + if (needsCleanup) return; + if (node.initialization != null) node.initialization.accept(this); + if (node.condition != null) node.condition.accept(this); + if (node.increment != null) node.increment.accept(this); + if (node.body != null) node.body.accept(this); + } + + @Override + public void visit(TryNode node) { + // try/catch is common without being refcount-touching, but + // catch handlers often do bless/warn/die things. Be conservative. + if (needsCleanup) return; + if (node.tryBlock != null) node.tryBlock.accept(this); + if (node.catchBlock != null) node.catchBlock.accept(this); + if (node.finallyBlock != null) node.finallyBlock.accept(this); + } + + @Override + public void visit(DeferNode node) { + // defer blocks execute at scope exit — mark conservatively. + mark(); + } + + @Override + public void visit(IdentifierNode node) { + } + + @Override + public void visit(NumberNode node) { + } + + @Override + public void visit(StringNode node) { + } + + @Override + public void visit(LabelNode node) { + } + + @Override + public void visit(CompilerFlagNode node) { + } + + @Override + public void visit(FormatNode node) { + } +}