Skip to content

Fix interpreter parity: \&{expr}, caller(), and use with ExportLevel#353

Closed
fglock wants to merge 14 commits into
masterfrom
feature/superoperator-analysis-2
Closed

Fix interpreter parity: \&{expr}, caller(), and use with ExportLevel#353
fglock wants to merge 14 commits into
masterfrom
feature/superoperator-analysis-2

Conversation

@fglock
Copy link
Copy Markdown
Owner

@fglock fglock commented Mar 23, 2026 •
edited
Loading

Summary

This PR includes interpreter parity fixes and performance optimizations.

Interpreter Parity Fixes

  1. Fix \&{expr} to return CODE type directly - interpreter was wrapping in REFERENCE type
  2. Fix caller() stack trace ordering - InterpreterState.getPcStack() returned wrong order
  3. Fix caller() for use statements with $Exporter::ExportLevel

Interpreter Performance Optimizations (~19% speedup)

  1. Phase 1: ThreadLocal Caching - Cache RuntimeScalar reference to avoid repeated ThreadLocal.get()
  2. Phase 2: Lazy CallerStack - Defer line number computation until caller() is actually called
    • Added CallerStack.pushLazy() with lambda-based resolution
    • Benchmark: 127s → 103s = ~19% speedup

Skill File Updates

  1. Fix JAR paths in skill files - Update from target/perlonjava-3.0.0.jar to dynamic paths

Test plan

  • ./jperl --interpreter -e 'use Time::HiRes "time"; print time()' returns numeric timestamp
  • ./jperl --interpreter -e 'use Getopt::Long; GetOptions("v" => \my $v); print "OK"' works
  • caller(N) returns identical values in interpreter and JVM modes
  • make passes all unit tests
  • Benchmark shows ~19% improvement (48.75/s vs 39.44/s)

Generated with Devin

fglock and others added 5 commits March 23, 2026 09:00
@fglock @devin-ai-integration
Fix interpreter: backslash-ampersand-{expr} should return CODE direct…
ee2295d 
…ly, not REFERENCE

The interpreter was wrapping backslash-ampersand-{expr} results in CREATE_REF, causing
*{$name} = backslash-ampersand-{$func} to pass REFERENCE->CODE to RuntimeGlob.set().
This broke Time::HiRes import because the glob CODE slot received a constant sub
instead of the actual code reference.

The JVM backend uses RuntimeCode.createCodeReference() which returns type=CODE
directly. Now the interpreter matches: backslash-ampersand-{expr} compiles to just
CODE_DEREF_NONSTRICT without CREATE_REF wrapping.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock @devin-ai-integration
Fix interpreter: hash slice assignment with block dereference @{$ref}…
7445a73 
…{keys}

Added BlockNode handling alongside OperatorNode for hash slice assignment.
This enables patterns like @{$hash_ref}{qw(a b)} = (1, 2) which are used
by Exporter/Heavy.pm and other CPAN modules.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock @devin-ai-integration
Fix interpreter: hash slice with block ref + defensive SUPER:: handling
be493ae 
1. Hash slice assignment with block dereference @{$ref}{keys}:
   Added BlockNode handling alongside OperatorNode for hash slice assignment.
   This enables patterns like @{$hash_ref}{qw(a b)} = (1, 2) which are used
   by Exporter/Heavy.pm.

2. Defensive null check in superMethod for SUPER:: resolution:
   When currentSub is null (no __SUB__ set), fall back to InterpreterState.currentPackage.
   This prevents NPE when SUPER::method is called from contexts without __SUB__.

Note: Full fix for Getopt::Long SUPER::import still pending - requires investigation
of __SUB__ initialization for subroutines in loaded .pm modules.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock @devin-ai-integration
Fix caller() stack trace ordering in interpreter mode
bd558f4 
Two issues were causing caller() to return incorrect package/line info
in interpreter mode:

1. InterpreterState.getPcStack() returned PCs in oldest-first order, but
   getStack() returns frames in newest-first order. Fixed by reversing
   the iteration order in getPcStack().

2. ExceptionFormatter was using CallerStack[interpreterFrameIndex] but
   CallerStack stores CALL SITES, not execution positions. For frame N,
   we need CallerStack[N-1] because CallerStack[M] stores the call site
   of the Mth call, which is the execution position of frame M+1.

This fixes caller(0) and caller(1) returning incorrect line numbers
when called from subroutines loaded via require/use.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock @devin-ai-integration
Fix caller() for use statements with ExportLevel in interpreter mode
7c0890c 
The parseUseDeclaration CallerStack entry was being accessed with the
wrong index when modules like Getopt::Long use $Exporter::ExportLevel.

The issue: CallerStack contains entries from both parseUseDeclaration
(pushed first) and interpreter CALL_SUB/CALL_METHOD (pushed later).
When building the stack trace, parseUseDeclaration handling used
callerStackIndex=0, but the actual entry was at a higher index after
accounting for interpreter frames.

The fix: Use Math.max(interpreterFrameIndex - 1, callerStackIndex) to
find the correct CallerStack entry for parseUseDeclaration.

This fixes use Getopt::Long and similar modules that rely on
$Exporter::ExportLevel to determine the target package for exports.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock fglock changed the title Fix interpreter: \&{expr} should return CODE directly Fix interpreter parity: \&{expr}, caller(), and use with ExportLevel Mar 23, 2026
fglock and others added 9 commits March 23, 2026 12:05
@fglock @devin-ai-integration
Fix caller() in signature defaults with eval package change
7ff1184 
The previous change introduced a regression where caller() in
signature default values would return the wrong package when
called from an eval with a package statement.

Test case: eval("package T121::Z; ::t121()") where sub t121 has
signature ($a = caller). Expected: T121::Z, was returning: main

The issue was the change to use CallerStack[interpreterFrameIndex-1]
for frames > 0. This was incorrect because:
- CallerStack[N] contains the call site info for the Nth call
- For interpreterFrameIndex=0, we need CallerStack[0] (the most
  recent call site), not null

Reverted to using CallerStack[interpreterFrameIndex] directly,
which correctly returns T121::Z for the innermost interpreter frame.

The parseUseDeclaration fix for Exporter is retained separately.

Fixes regression in op/signatures.t test 217.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock @devin-ai-integration
Update skill files: fix JAR paths and improve documentation
9ceecdf 
- Update all skill files to use dynamic JAR path (build/libs/perlonjava-*.jar)
  instead of hardcoded target/perlonjava-3.0.0.jar
- Use ./jperl wrapper script in examples instead of direct java -jar
- Fix profile-perlonjava paths (PerlOnJava2 -> PerlOnJava)
- Add interpreter profiling documentation
- Fix git stash warning consistency (never use stash)

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock @devin-ai-integration
Fix profile-perlonjava skill: consistent git stash warning
edd69f0 
- Add emoji warning icons to match other skill files
- Remove contradictory 'commit or stash' instruction

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock @devin-ai-integration
Add interpreter performance optimization design doc
f4529b3 
Profile analysis of benchmark_closure.pl in interpreter mode:
- Identified ThreadLocal lookup overhead in CALL opcode
- CallerStack push/pop on every call even when caller() unused
- Deep call chain indirection for subroutine dispatch
- TreeMap lookup for line numbers

Optimization plan with 4 phases documented.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock @devin-ai-integration
Interpreter optimization Phase 1: Cache ThreadLocal RuntimeScalar ref…
b755526 
…erence

- Cache InterpreterState.currentPackage.get() at start of execute()
- Reuse cached RuntimeScalar for SET_PACKAGE opcode
- Avoid repeated ThreadLocal lookups in CALL_SUB opcodes

No measurable speedup on benchmark_closure.pl, but cleaner code.
Profile shows ~10% of time in getCallSiteInfo + getSourceLocationAccurate
for caller() support - Phase 2 target.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock @devin-ai-integration
Interpreter optimization Phase 2: Lazy CallerStack (~19% speedup)
c05eefc 
Defer caller() info computation until actually needed:
- Add CallerStack.pushLazy() with lambda-based resolution
- CALL_SUB/CALL_METHOD now push lazy entries
- Line number computation only happens when caller() is called
- pop() skips resolution for unneeded entries

Benchmark improvement: 127s -> 103s = ~19% speedup on benchmark_closure.pl

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock @devin-ai-integration
Interpreter optimization Phase 3: Inline InterpretedCode calls
b4fad9c 
- Add fast path in CALL_SUB for InterpretedCode: call execute() directly
- Bypass RuntimeCode.apply() indirection chain for interpreter-to-interpreter calls
- Pass null for subroutineName to enable InterpreterFrame caching
- Apply same optimization to TAILCALL handling

Small improvement (~2%) combined with previous optimizations.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock @devin-ai-integration
Add register array pooling for interpreter (97s from 101s)
13322ef 
- InterpretedCode.getRegisters() caches register arrays per-code-object
- Uses ThreadLocal for thread safety with recursion detection
- Recursive calls fallback to fresh allocation (no contention)
- BytecodeInterpreter.execute() releases registers in finally block

Benchmark: 97s from 101s baseline (4% improvement from allocation reduction)

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock @devin-ai-integration
Update design doc with Phase 3 & 4 results
70ef173 
Total improvement: 127s → 97s (~24% speedup)
- Phase 3: Inline apply path (2% speedup)
- Phase 4: Register pooling (4% speedup)

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock fglock mentioned this pull request Mar 24, 2026
Merged
3 tasks
@fglock fglock closed this Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@fglock