perf: reduce warm jit coordination overhead by siyul-park · Pull Request #119 · siyul-park/minivm

siyul-park · 2026-07-06T01:04:24Z

Summary

Reduce warm JIT/interpreter coordination overhead without changing public APIs.
Replace the hot warm-entry fallbacks[anchor{addr, 0}] map lookup with a dense per-function entry fallback slice.
Skip the threaded-loop safepoint call when it would be a no-op: no cancellation channel, fuel, hook, profiler, or shared cache coordination is active and the current function already has a native entry installed.
Trim compile/trace setup work by aliasing read-only trace clone state and building the JIT function map once per emit instead of once per root.
Update benchmark docs with current-only BenchmarkJITIssue101 results.

Why

BenchmarkJITIssue101 is a branch-heavy batch workload with many tiny tree-score functions over a mutable f64 feature row. After traces are installed, the remaining cost was dominated by interpreter coordination rather than useful VM work. The prior profile showed safepoint and runtime.mapaccess1 as the main overhead on the JIT path.

This keeps the existing design intact: the threaded interpreter remains the correctness source, JIT behavior stays speculative with fallback, and ARM64-specific lowering remains unchanged.

Results

Measured on darwin/arm64, Apple M4 Pro:

Benchmark	Mode	ns/op	B/op	allocs/op
`BenchmarkJITIssue101`	threaded	1,807	0	0
`BenchmarkJITIssue101`	JIT	1,203	0	0

Additional checks:

BenchmarkJITIssue60 remained within expected noise; focused A/B for closure_counter_loop/jit was 1,076 ns/op on this branch vs 1,132 ns/op on main under the same command.
BenchmarkFib35/minivm_jit remained around 49 ms/op under the planned regression command.
Follow-up pprof showed safepoint at 0.65% cumulative and removed runtime.mapaccess1 as the JIT coordination hotspot.

Validation

make lint
go test -count=1 -race ./interp ./asm/...
go test -count=1 ./...
cd benchmarks && go test -run='^$' -bench='BenchmarkJITIssue101/(interp|jit)$' -benchmem -benchtime=3s ./...
cd benchmarks && go test -run='^$' -bench='BenchmarkJITIssue60|BenchmarkFib35/minivm_(interp|jit)$' -benchmem -benchtime=3s ./...

Summary by CodeRabbit

Documentation
- Added an ARM64 JIT benchmarks subsection with a new workload example and clearer threaded-vs-JIT comparison.
Performance Improvements
- Improved interpreter cancellation handling for lower overhead during long-running execution.
- Reduced JIT/trace startup work by skipping compilation steps when no trace anchors are available.
Bug Fixes
- Enhanced entry fallback behavior during tracing and JIT warmup to improve entry-path resolution and fallback checks.
Refactor
- Improved interpreter state setup during trace cloning to reduce unnecessary allocations.

coderabbitai · 2026-07-06T01:04:45Z

📝 Walkthrough

Walkthrough

The PR caches interpreter cancellation state, replaces entry-fallback anchor lookups with a per-function slice, precomputes JIT function lookups, reuses slices in tracer cloning, and adds benchmark documentation for a JIT coordination workload.

Changes

Interpreter fast-path optimizations

Layer / File(s)	Summary
Interpreter state and cancellation `interp/interp.go`	Adds `done <-chan struct{}` and `entryFallbacks []func(*Interpreter)` to `Interpreter`, initializes them in `New`, and switches `Run`, `dispatch`, and `safepoint` to use the cached cancellation channel.
Entry fallback table `interp/interp.go`	Stores root-threaded handlers in `entryFallbacks`, uses `entryFallback(addr)` for fallback and warmup checks, and grows/clears the slice in `bind` and `remove`.
JIT emit and tracer clone `interp/jit.go`, `interp/trace.go`	`emit` precomputes anchors and a shared function map for `emitRoot`; `Tracer.clone` reuses `constants` and `instrs` and initializes `entryFallbacks` for the cloned code.
Benchmark documentation `docs/benchmarks.md`	Adds the ARM64 JIT coordination workload subsection with `BenchmarkJITIssue101`, a comparison table, and a note on the JIT path.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately reflects the main performance-focused warm JIT coordination change.
Description check	✅ Passed	It covers the changes, rationale, results, and validation; only the optional Related Issues section is missing.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch jit-resource-reduction

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (2)

docs/benchmarks.md (1)
174-183: 📐 Maintainability & Code Quality | 🔵 Trivial

Keep this subsection factual and reproducible.

Before/Current does not identify the baseline, and the final sentence attributes the win to internal mechanisms that the benchmark table itself does not establish. Rename the columns to the actual modes (threaded / JIT) and either cite the profiling evidence or move the mechanism explanation to docs/jit-internals.md.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/benchmarks.md` around lines 174 - 183, The benchmark subsection mixes an
unclear baseline label and an unsupported mechanism claim, so make it factual
and reproducible. Update the table headers in the `BenchmarkJITIssue101` section
to use the actual modes (`threaded` and `JIT`) instead of `Before`/`Current`,
and remove or relocate the final sentence about warm-entry fallback checks and
no-op safepoints unless you can cite profiling evidence for it. If the
explanation is needed, move it to `docs/jit-internals.md` and keep
`docs/benchmarks.md` limited to the measured results.
interp/jit.go (1)
221-244: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Cache the address→function map
emit() still rebuilds funcs by scanning every live i.instrs slot on each compile. Cache that lookup on the interpreter/compiler and invalidate it from bind/remove; rename the inner addr/fn variables to avoid shadowing the parameters.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@interp/jit.go` around lines 221 - 244, The emit method is rebuilding the
address-to-function lookup on every call by scanning all live i.instrs entries,
so move that map into a cached field on the Interpreter or compiler and reuse it
across emits. Update the cache from bind and remove so it stays correct when
instructions change, and adjust emit plus its inner loop to stop shadowing the
addr and fn parameters by using distinct local names.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@docs/benchmarks.md`:
- Around line 174-183: The benchmark subsection mixes an unclear baseline label
and an unsupported mechanism claim, so make it factual and reproducible. Update
the table headers in the `BenchmarkJITIssue101` section to use the actual modes
(`threaded` and `JIT`) instead of `Before`/`Current`, and remove or relocate the
final sentence about warm-entry fallback checks and no-op safepoints unless you
can cite profiling evidence for it. If the explanation is needed, move it to
`docs/jit-internals.md` and keep `docs/benchmarks.md` limited to the measured
results.

In `@interp/jit.go`:
- Around line 221-244: The emit method is rebuilding the address-to-function
lookup on every call by scanning all live i.instrs entries, so move that map
into a cached field on the Interpreter or compiler and reuse it across emits.
Update the cache from bind and remove so it stays correct when instructions
change, and adjust emit plus its inner loop to stop shadowing the addr and fn
parameters by using distinct local names.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a5558543-b79f-408a-a6c3-e12cb45d80ea

📥 Commits

Reviewing files that changed from the base of the PR and between b52bc09 and 45e74ea.

📒 Files selected for processing (4)

docs/benchmarks.md
interp/interp.go
interp/jit.go
interp/trace.go

codecov · 2026-07-06T01:11:05Z

Codecov Report

❌ Patch coverage is 63.63636% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.79%. Comparing base (b52bc09) to head (1e6ba16).

Files with missing lines	Patch %	Lines
interp/interp.go	73.58%	11 Missing and 3 partials ⚠️
interp/jit.go	0.00%	10 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #119      +/-   ##
==========================================
- Coverage   46.80%   46.79%   -0.02%     
==========================================
  Files          77       77              
  Lines       17319    17340      +21     
==========================================
+ Hits         8107     8114       +7     
- Misses       8297     8307      +10     
- Partials      915      919       +4

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

perf: reduce warm jit coordination overhead

45e74ea

coderabbitai Bot reviewed Jul 6, 2026

View reviewed changes

docs: keep jit benchmark results current-only

1e6ba16

siyul-park merged commit 2295478 into main Jul 6, 2026
6 checks passed

siyul-park mentioned this pull request Jul 9, 2026

Improve JIT throughput for LightGBM-style batch tree evaluation #101

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: reduce warm jit coordination overhead - #119

perf: reduce warm jit coordination overhead#119
siyul-park merged 2 commits into
mainfrom
jit-resource-reduction

siyul-park commented Jul 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

codecov Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

siyul-park commented Jul 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Results

Validation

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

siyul-park commented Jul 6, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading

codecov Bot commented Jul 6, 2026 •

edited

Loading