Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .agents/skills/debug-perlonjava/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,18 @@ This helps identify operator precedence issues and incorrect parsing.

### 6. Profile with JFR (for performance issues)
```bash
# Resolve JDK tools explicitly; `jfr` is not always on PATH on macOS.
export JAVA_HOME="${JAVA_HOME:-$(/usr/libexec/java_home 2>/dev/null)}"
export JFR_BIN="${JFR_BIN:-$JAVA_HOME/bin/jfr}"
# Local fallback known to exist on this workstation:
# /Users/fglock/Library/Java/JavaVirtualMachines/temurin-24.0.2/Contents/Home/bin/jfr

# Record profile
$JAVA_HOME/bin/java -XX:StartFlightRecording=duration=10s,filename=profile.jfr \
-jar target/perlonjava-3.0.0.jar script.pl

# Analyze hotspots
$JAVA_HOME/bin/jfr print --events jdk.ExecutionSample profile.jfr 2>&1 | \
$JFR_BIN print --events jdk.ExecutionSample profile.jfr 2>&1 | \
grep -E "^\s+[a-z].*line:" | sed 's/line:.*//' | sort | uniq -c | sort -rn | head -20
```

Expand Down
148 changes: 148 additions & 0 deletions dev/design/reachability-walk-cache-plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
# Reachability Walk Cache for DBIC Scope Cleanup

**Status:** Phase 1 design complete; Phase 2 implementation pending
**Date:** 2026-05-12
**Branch / PR:** `fix/math-bigint-method-chain-regression` / PR #709

## Problem

DBIx::Class `t/52leaks.t` spends most of its time in repeated
`ReachabilityWalker.isReachableFromExternalRoot(hash)` calls from
`MortalList.scopeExitCleanupHash()`. Each hash scope cleanup walks the same
package/global/live-lexical graph again. In DBIC this becomes quadratic because
many scope-exiting hashes run the same external-root query while the visible
root graph is large.

The fix is to preserve the existing external-root semantics but cache the
snapshot for the duration of a cleanup / flush boundary.

## Baseline

Measured before Java code changes, from
`cpan_build_dir/DBIx-Class-0.082844`:

```sh
timeout 600 /usr/bin/time -p ../../jperl -I../../src/main/perl/lib -Ilib -It/lib t/52leaks.t > /tmp/dbic_52leaks_before_N.log 2>&1
```

| Run | Result | real |
|---|---:|---:|
| 1 | PASS | 69.90s |
| 2 | PASS | 72.36s |
| 3 | PASS | 73.98s |

Median baseline: **72.36s**.

Acceptance threshold for this fix: `t/52leaks.t` must pass in **24.12s or
less** for at least a 3x wall-clock speedup.

## Design

Add `ReachabilityWalker.ExternalRootSnapshot`, a reusable snapshot for the
reachability query currently implemented by
`isReachableFromExternalRoot(RuntimeBase target)`.

The snapshot preserves these current semantics:

- Package roots are roots: `%`, `@`, `$`, and `&` entries from
`GlobalVariable`.
- `DestroyDispatch` rescued objects are roots.
- Live lexical roots from `MyVarCleanupStack.snapshotLiveVars()` are roots.
- Weak scalars do not create strong reachability edges.
- `RuntimeStash` is not walked.
- `RuntimeHash` values backed by `HashSpecialVariable` are not walked.
- `RuntimeCode` closure captures are not walked.

The one target-specific rule is kept explicitly: a named lexical container
does not make itself externally reachable. The snapshot therefore tracks direct
live-lexical origins. A target that is a direct lexical origin only returns
reachable if it is also reachable from a non-lexical root, or from a different
live lexical root.

`ReachabilityWalker.isReachableFromExternalRoot(target)` remains as a
one-shot compatibility wrapper that builds a snapshot and queries it.

## MortalList Cache

`MortalList.scopeExitCleanupHash()` will query a lazily-built
`ExternalRootSnapshot` instead of launching a fresh root walk for every hash.

The cache is intentionally short-lived and conservative. It is invalidated at:

- `MortalList.flush()`
- `MortalList.flushAboveMark()`
- `MortalList.popAndFlush()`
- `MortalList.drainPendingSince()`
- immediately before and after user `DESTROY` execution in
`DestroyDispatch.doCallDestroy()`

These boundaries cover refcount drains, scope-boundary processing, and user
code that can mutate roots during destruction.

## Test Plan

Build / unit gate:

```sh
timeout 1200 make > /tmp/make_reachability_cache.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/make_reachability_cache.log
```

DBIx::Class targeted gates:

```sh
cd cpan_build_dir/DBIx-Class-0.082844
timeout 600 /usr/bin/time -p ../../jperl -I../../src/main/perl/lib -Ilib -It/lib t/52leaks.t > /tmp/dbic_52leaks_after.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/dbic_52leaks_after.log
timeout 300 ../../jperl -I../../src/main/perl/lib -Ilib -It/lib t/storage/error.t > /tmp/dbic_storage_error.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/dbic_storage_error.log
timeout 300 ../../jperl -I../../src/main/perl/lib -Ilib -It/lib t/88result_set_column.t > /tmp/dbic_88.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/dbic_88.log
timeout 300 ../../jperl -I../../src/main/perl/lib -Ilib -It/lib t/60core.t > /tmp/dbic_60core.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/dbic_60core.log
timeout 300 ../../jperl -I../../src/main/perl/lib -Ilib -It/lib t/64db.t > /tmp/dbic_64db.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/dbic_64db.log
timeout 300 ../../jperl -I../../src/main/perl/lib -Ilib -It/lib t/87ordered.t > /tmp/dbic_87ordered.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/dbic_87ordered.log
timeout 300 ../../jperl -I../../src/main/perl/lib -Ilib -It/lib t/96_is_deteministic_value.t > /tmp/dbic_96.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/dbic_96.log
timeout 300 ../../jperl -I../../src/main/perl/lib -Ilib -It/lib t/storage/txn.t > /tmp/dbic_storage_txn.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/dbic_storage_txn.log
timeout 300 ../../jperl -I../../src/main/perl/lib -Ilib -It/lib t/storage/txn_scope_guard.t > /tmp/dbic_txn_scope_guard.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/dbic_txn_scope_guard.log
timeout 300 ../../jperl -I../../src/main/perl/lib -Ilib -It/lib t/cdbi/sweet/08pager.t > /tmp/dbic_08pager.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/dbic_08pager.log
```

Math-BigInt targeted gates:

```sh
timeout 60 ./jperl -Isrc/main/perl/lib -e 'use Math::BigInt; my $x = Math::BigInt->new("1"); print $x->bexp(), "\n";' > /tmp/math_bigint_bexp_predicate.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/math_bigint_bexp_predicate.log
cd src/test/resources/module/Math-BigInt
timeout 180 ../../../../../jperl -I../../../../../src/main/perl/lib -Ilib t/bare_mbi.t > /tmp/math_bare_mbi.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/math_bare_mbi.log
timeout 180 ../../../../../jperl -I../../../../../src/main/perl/lib -Ilib t/bigintpm.t > /tmp/math_bigintpm.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/math_bigintpm.log
timeout 180 ../../../../../jperl -I../../../../../src/main/perl/lib -Ilib t/bigfltpm.t > /tmp/math_bigfltpm.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/math_bigfltpm.log
timeout 180 ../../../../../jperl -I../../../../../src/main/perl/lib -Ilib t/bigfltrt.t > /tmp/math_bigfltrt.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/math_bigfltrt.log
timeout 180 ../../../../../jperl -I../../../../../src/main/perl/lib -Ilib t/mbimbf.t > /tmp/math_mbimbf.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/math_mbimbf.log
timeout 180 ../../../../../jperl -I../../../../../src/main/perl/lib -Ilib t/bigrat.t > /tmp/math_bigrat.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/math_bigrat.log
```

Final gates:

```sh
timeout 3600 ./jcpan -t DBIx::Class > /tmp/jcpan_dbic_reachability_cache.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/jcpan_dbic_reachability_cache.log
timeout 1800 make test-bundled-modules > /tmp/test_bundled_reachability_cache.log 2>&1; printf 'EXIT: %s\n' $? >> /tmp/test_bundled_reachability_cache.log
ps aux | awk '$3 > 20 {print $2, $3, $11, $12}'
```

## Progress Tracking

### Current Status: Phase 1 Complete

### Completed Phases

- [x] Phase 1: Design and baseline (2026-05-12)
- Recorded three pre-fix `t/52leaks.t` timings.
- Median baseline: 72.36s.
- Target after fix: <= 24.12s.

### Next Steps

1. Implement `ReachabilityWalker.ExternalRootSnapshot`.
2. Use a cached snapshot from `MortalList.scopeExitCleanupHash()`.
3. Add conservative invalidation at flush/drain/DESTROY boundaries.
4. Run the targeted and final verification gates.

### Open Questions

- None. Cache lifetime should stay conservative unless later profiling shows a
broader lifetime is necessary.
3 changes: 3 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,9 @@
<excludes>
<exclude>module-info.class</exclude>
<exclude>META-INF/MANIFEST.MF</exclude>
<exclude>META-INF/*.SF</exclude>
<exclude>META-INF/*.DSA</exclude>
<exclude>META-INF/*.RSA</exclude>
</excludes>
</filter>
</filters>
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/org/perlonjava/app/cli/ArgumentParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ private static void processArgs(String[] args, CompilerOptions parsedArgs) {

if (arg.equals("--")) {
// "--" indicates the end of switch arguments; subsequent arguments are treated as non-switch
if (parsedArgs.rudimentarySwitchParsing) {
processRudimentarySwitchArguments(args, parsedArgs, i + 1);
break;
}
readingArgv = true;
continue;
}
Expand Down Expand Up @@ -1291,6 +1295,19 @@ private static void processRudimentarySwitch(String arg, CompilerOptions parsedA
.append("';\n");
}

private static void processRudimentarySwitchArguments(String[] args, CompilerOptions parsedArgs, int startIndex) {
for (int i = startIndex; i < args.length; i++) {
String arg = args[i];
if (arg.equals("--") || !arg.startsWith("-")) {
for (int j = i; j < args.length; j++) {
RuntimeArray.push(parsedArgs.argumentList, new RuntimeScalar(args[j]));
}
return;
}
processRudimentarySwitch(arg, parsedArgs);
}
}

/**
* Handles debug flags specified with the -D switch.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2745,6 +2745,8 @@ void compileVariableDeclaration(OperatorNode node, String op) {
}
default -> throwCompilerException("Unsupported variable type: " + sigil);
}
emit(Opcodes.REGISTER_MY_VAR);
emitReg(reg);

// Runtime attribute dispatch for my variables with attributes
emitVarAttrsIfNeeded(node, reg, sigil);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ public static RuntimeList execute(InterpretedCode code, RuntimeArray args, int c
if (slot instanceof RuntimeScalar rs) {
RuntimeScalar.scopeExitCleanup(rs);
}
MyVarCleanupStack.unregister(slot);
registers[reg] = null;
}

Expand Down Expand Up @@ -311,6 +312,7 @@ public static RuntimeList execute(InterpretedCode code, RuntimeArray args, int c
if (slot instanceof RuntimeHash rh) {
MortalList.scopeExitCleanupHash(rh);
}
MyVarCleanupStack.unregister(slot);
registers[reg] = null;
}

Expand Down Expand Up @@ -343,6 +345,7 @@ public static RuntimeList execute(InterpretedCode code, RuntimeArray args, int c
if (slot instanceof RuntimeArray ra) {
MortalList.scopeExitCleanupArray(ra);
}
MyVarCleanupStack.unregister(slot);
registers[reg] = null;
}

Expand Down Expand Up @@ -546,6 +549,12 @@ public static RuntimeList execute(InterpretedCode code, RuntimeArray args, int c
RuntimeScalar newScalar = new RuntimeScalar();
registers[rs].addToScalar(newScalar);
registers[rd] = newScalar;
MyVarCleanupStack.register(newScalar);
}

case Opcodes.REGISTER_MY_VAR -> {
int reg = bytecode[pc++];
MyVarCleanupStack.register(registers[reg]);
}

// =================================================================
Expand Down Expand Up @@ -2574,6 +2583,7 @@ public static RuntimeList execute(InterpretedCode code, RuntimeArray args, int c
MortalList.scopeExitCleanupArray(ra);
needsFlush = true;
}
MyVarCleanupStack.unregister(reg);
registers[i] = null;
}
if (needsFlush) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,8 @@ public static void compileAssignmentOperator(BytecodeCompiler bytecodeCompiler,
// may tie the variable), then assign the value so STORE fires.
bytecodeCompiler.emit(Opcodes.LOAD_UNDEF);
bytecodeCompiler.emitReg(reg);
bytecodeCompiler.emit(Opcodes.REGISTER_MY_VAR);
bytecodeCompiler.emitReg(reg);
bytecodeCompiler.emitVarAttrsIfNeeded(leftOp, reg, "$");
bytecodeCompiler.emit(Opcodes.SET_SCALAR);
bytecodeCompiler.emitReg(reg);
Expand Down Expand Up @@ -689,6 +691,8 @@ public static void compileAssignmentOperator(BytecodeCompiler bytecodeCompiler,
// each list slot; dispatching MODIFY_*_ once per RETRIEVE_BEGIN_* slot
// would duplicate calls. Variable attributes + RETRIEVE_BEGIN_* are
// fully handled for `my $x`, `my @x`, `my %x`, and `my $x=` forms above.
bytecodeCompiler.emit(Opcodes.REGISTER_MY_VAR);
bytecodeCompiler.emitReg(varReg);
} else {
varReg = bytecodeCompiler.addVariable(varName, "my");
switch (sigil) {
Expand All @@ -705,6 +709,8 @@ public static void compileAssignmentOperator(BytecodeCompiler bytecodeCompiler,
bytecodeCompiler.emitReg(varReg);
}
}
bytecodeCompiler.emit(Opcodes.REGISTER_MY_VAR);
bytecodeCompiler.emitReg(varReg);
}
varRegs.add(varReg);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,10 @@ public static String disassemble(InterpretedCode interpretedCode) {
src = interpretedCode.bytecode[pc++];
sb.append("MY_SCALAR r").append(rd).append(" = r").append(src).append("\n");
break;
case Opcodes.REGISTER_MY_VAR:
rd = interpretedCode.bytecode[pc++];
sb.append("REGISTER_MY_VAR r").append(rd).append("\n");
break;
case Opcodes.LOAD_GLOBAL_SCALAR:
rd = interpretedCode.bytecode[pc++];
int nameIdx = interpretedCode.bytecode[pc++];
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/org/perlonjava/backend/bytecode/Opcodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -2346,6 +2346,12 @@ public class Opcodes {
*/
public static final short UCFIRST_UNICODE = 489;

/**
* Register a newly-created lexical my-variable with MyVarCleanupStack.
* Format: REGISTER_MY_VAR reg
*/
public static final short REGISTER_MY_VAR = 490;

private Opcodes() {
} // Utility class - no instantiation
}
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,25 @@ private static boolean containsNonAsciiCodePoint(CharSequence s) {
return false;
}

private static boolean isSpecialVariable(String name) {
// Special variables that can't be package-qualified
// These are single-character special variables in Perl
return name.equals("_") || // default variable
name.equals("/") || // input record separator
name.equals("\\") || // output record separator
name.equals("|") || // output field separator
name.equals(",") || // output field separator
name.equals(";") || // statement terminator
name.equals("'") || // last pattern match result
name.equals("`") || // last read line
name.equals("!") || // errno
name.equals("?") || // child error status
name.equals("$") || // process id
name.equals(".") || // input line number
name.equals("%") || // hash slice
name.equals("&"); // last regex match result
}

/**
* Parses the inner part of a complex identifier, handling cases where the identifier
* may be enclosed in braces.
Expand Down Expand Up @@ -438,6 +457,7 @@ public static String parseComplexIdentifierInner(Parser parser, boolean insideBr
// Nothing valid follows ', so return what we have
return variableName.toString();
}
isFirstToken = false;
// Continue the loop to process the next token
continue;
}
Expand Down Expand Up @@ -469,6 +489,7 @@ public static String parseComplexIdentifierInner(Parser parser, boolean insideBr
// Nothing valid follows ::, so return what we have
return variableName.toString();
}
isFirstToken = false;
// Continue the loop to process the next token
continue;
}
Expand Down Expand Up @@ -510,6 +531,14 @@ public static String parseComplexIdentifierInner(Parser parser, boolean insideBr
boolean hasDoubleColon = nextToken.text.equals("::");
boolean hasSingleQuote = false;

// Special variables can't be package-qualified (e.g., $_, $/, $\, etc.)
// If we have a single-letter identifier that's a special variable, don't allow :: qualification
if (isFirstToken && hasDoubleColon && token.text.length() == 1 && isSpecialVariable(token.text)) {
// Special variable - can't have package qualification
parser.tokenIndex++;
return variableName.toString();
}

if (nextToken.text.equals("'")) {
// Look ahead to see what follows the '
LexerToken afterQuote = parser.tokens.get(parser.tokenIndex + 2);
Expand All @@ -527,6 +556,7 @@ public static String parseComplexIdentifierInner(Parser parser, boolean insideBr
parser.tokenIndex++;
token = parser.tokens.get(parser.tokenIndex);
nextToken = parser.tokens.get(parser.tokenIndex + 1);
isFirstToken = false;
continue;
} else if (token.type == LexerTokenType.WHITESPACE || token.type == LexerTokenType.EOF || token.type == LexerTokenType.NEWLINE) {
return variableName.toString();
Expand Down Expand Up @@ -703,4 +733,4 @@ static void validateIdentifier(Parser parser, String varName, int startIndex) {
}
}
}
}
}
Loading
Loading