fix(interp): stop host allocation scanning the heap and leaking frame slots - #171
Conversation
… slots Two defects landed together with the heap-ownership rework and made allocation-heavy guest programs regress badly (#169, #170). owner() answered "is this pointer already in a heap slot?" by scanning every live slot with reflect.ValueOf().Pointer(). It runs on Alloc, Store, and Push, so an embedder that allocates once per guest operation paid O(heap) per allocation and ran quadratically: a fannkuch-redux program that took 6.1s stopped finishing inside 45s. Replace the scan with an index keyed by the value itself. Only the host boundary records hints, so the interpreter's own allocation path is untouched, and lookup validates against the heap, so a hint left by a freed or reused slot self-heals instead of needing removal bookkeeping on release and sweep. A prune pass bounds the index and doubles the budget it survives, which keeps upkeep amortized constant. RETURN moved the returned values down and truncated the stack without releasing the params, locals, and operands it discarded, so every call that received a reference leaked one count. land already releases those slots when an exception unwinds a frame; RETURN now does the same. Reference counting had leaked this way before, but the old root-based mark and sweep reclaimed it; trial deletion reads an inflated count as ownership outside the heap and cannot, so the leak became unbounded. Frames whose every slot is a plain scalar skip the sweep when their operand stack is balanced, which keeps scalar recursion close to its former cost. Measured with the reporter's program corpus on linux/amd64 (min of three): fannkuch 6.9s where it previously did not finish, binarytrees 1.60s against 1.80s before the regression window, and the unrelated kernels unchanged. RecursiveFib(20)/threaded costs 604 -> 625 ns/op for the exact counts. Adds vm_gc_cycles_total and vm_gc_slots_total so collector pressure is observable, a heap-exhaustion regression test that fails without the RETURN fix, and two builder-based benchmarks covering the reported shapes with no external dependency. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ZsuY2FrrwQ7S3PGw98RQh
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe interpreter adds indexed heap ownership tracking, return-time frame-slot cleanup, garbage-collection metrics, regression tests, documentation, and recursive memory benchmarks. ChangesMemory runtime behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant HostBoundary
participant Interpreter
participant owners
participant Frame
participant GC
HostBoundary->>Interpreter: Alloc, Store, Push, Load, Retain, Pop
Interpreter->>owners: Record and validate heap-slot ownership
Frame->>Interpreter: returnOp
Interpreter->>Frame: retire discarded frame slots
Interpreter->>GC: Record collection cycles and heap slots
Possibly related PRs
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.45.0)interp/threaded.goast-grep timed out on this file Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Two defects landed together with the heap-ownership rework and made
allocation-heavy guest programs regress badly (#169, #170).
owner() answered "is this pointer already in a heap slot?" by scanning
every live slot with reflect.ValueOf().Pointer(). It runs on Alloc,
Store, and Push, so an embedder that allocates once per guest operation
paid O(heap) per allocation and ran quadratically: a fannkuch-redux
program that took 6.1s stopped finishing inside 45s. Replace the scan
with an index keyed by the value itself. Only the host boundary records
hints, so the interpreter's own allocation path is untouched, and lookup
validates against the heap, so a hint left by a freed or reused slot
self-heals instead of needing removal bookkeeping on release and sweep.
A prune pass bounds the index and doubles the budget it survives, which
keeps upkeep amortized constant.
RETURN moved the returned values down and truncated the stack without
releasing the params, locals, and operands it discarded, so every call
that received a reference leaked one count. land already releases those
slots when an exception unwinds a frame; RETURN now does the same.
Reference counting had leaked this way before, but the old root-based
mark and sweep reclaimed it; trial deletion reads an inflated count as
ownership outside the heap and cannot, so the leak became unbounded.
Frames whose every slot is a plain scalar skip the sweep when their
operand stack is balanced, which keeps scalar recursion close to its
former cost.
Measured with the reporter's program corpus on linux/amd64 (min of
three): fannkuch 6.9s where it previously did not finish, binarytrees
1.60s against 1.80s before the regression window, and the unrelated
kernels unchanged. RecursiveFib(20)/threaded costs 604 -> 625 ns/op for
the exact counts.
Adds vm_gc_cycles_total and vm_gc_slots_total so collector pressure is
observable, a heap-exhaustion regression test that fails without the
RETURN fix, and two builder-based benchmarks covering the reported
shapes with no external dependency.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_017ZsuY2FrrwQ7S3PGw98RQh
Summary by CodeRabbit
Performance
Bug Fixes
Documentation
Monitoring