refactor: align JIT profiling lifecycle and reporting by siyul-park · Pull Request #145 · siyul-park/minivm

siyul-park · 2026-07-13T11:27:45Z

Summary

remove the public WithLocal profiler option and keep the interpreter's sampling collector internal
make WithProfiler(nil) disable observable profiling
count every JIT compilation request, including unavailable backends and compiler construction failures
preserve compile trigger identity in CLI miss reports
separate raw profiler metric collection from ranked report construction
simplify profile domain names and keep rendering isolated in cli/profile.go

Verification

go test -race ./...
go vet ./...
git diff --check

Summary by CodeRabbit

New Features
- Added richer JIT lifecycle profiling metrics (capture, compile triggers/results, emits/bytes, entries, yields, exits, and misses) with opcode/reason detail.
- Enhanced .profile output with normalized, ranked “hot” tables plus JIT summaries and miss/exit sections.
Bug Fixes
- Improved cache build-state handling with trigger-aware queued side-exit requests and safer readiness transitions.
Documentation
- Expanded ARM64 JIT lifecycle internals and profiling semantics documentation.
Tests
- Updated and extended profiling output, interpreter/JIT metrics, tracer capture behavior, ARM64 JIT lowering/exit assertions, cache closing behavior, and profiler counter/low-allocation tests.

coderabbitai · 2026-07-13T11:28:08Z

📝 Walkthrough

Walkthrough

Adds lifecycle-aware JIT profiling across collection, tracing, cache scheduling, compilation, ARM64 exits, native wrappers, and CLI reporting. The .profile command now renders ranked function, IP, opcode, entry, exit, and miss sections.

Changes

JIT lifecycle profiling

Layer / File(s)	Summary
Profiling model and collector `prof/*`	Adds typed lifecycle metrics, stable counters, deterministic labels, merge/reset handling, and tests.
Cache, compiler, and interpreter wiring `interp/cache.go`, `interp/jit.go`, `interp/interp.go`	Adds trigger-aware cache requests, structured compile outcomes, exit descriptors, native entry/yield/exit accounting, and profiler integration.
Tracer and ARM64 exit attribution `interp/trace.go`, `interp/jit_arm64.go`, `interp/*_test.go`	Records capture outcomes and propagates reason/opcode metadata through ARM64 traps, guards, branches, terminals, and native wrappers.
CLI profile reports `cli/profile.go`, `cli/repl.go`, `cli/repl_test.go`	Normalizes metrics into ranked hot-function, IP, opcode, JIT entry, exit, and miss sections with deterministic limits.
Profiling documentation `docs/profile.md`, `docs/jit-internals.md`	Documents lifecycle metric dimensions, report semantics, cache publication, journal exit identifiers, and native profiling behavior.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

Add lifecycle-aware JIT explain metrics and ranked profile reports #130 — Covers the lifecycle-aware JIT metrics and ranked .profile reporting implemented here.

Possibly related PRs

siyul-park/minivm#116 — Related profiler state and JIT metric plumbing in the interpreter.
siyul-park/minivm#139 — Related plan-based compilation changes in the compiler and ARM64 exit pipeline.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description covers the main changes and verification, but it does not follow the required template headings or include related issues and additional info.	Rewrite it using the required 'Changes Made', 'Related Issues', and 'Additional Information' sections, and place the current summary and verification under them.
Docstring Coverage	⚠️ Warning	Docstring coverage is 5.45% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the JIT profiling and reporting refactor.
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.

✨ 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 feature/jit-profile-metrics

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

codecov · 2026-07-13T11:36:35Z

Codecov Report

❌ Patch coverage is 61.20592% with 341 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.42%. Comparing base (0978e9d) to head (651002e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
prof/jit_metrics.go	56.52%	101 Missing and 9 partials ⚠️
interp/jit.go	0.00%	92 Missing ⚠️
interp/interp.go	14.28%	69 Missing and 3 partials ⚠️
interp/cache.go	7.69%	36 Missing ⚠️
interp/trace.go	66.66%	12 Missing and 1 partial ⚠️
interp/jit_plan.go	0.00%	10 Missing ⚠️
cli/profile.go	98.46%	4 Missing ⚠️
cli/repl.go	93.65%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #145      +/-   ##
==========================================
+ Coverage   26.98%   27.42%   +0.43%     
==========================================
  Files          83       86       +3     
  Lines       59196    59883     +687     
==========================================
+ Hits        15974    16421     +447     
- Misses      41999    42233     +234     
- Partials     1223     1229       +6

☔ 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.

coderabbitai

Actionable comments posted: 3

🧹 Nitpick comments (1)

cli/profile_report.go (1)
107-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move comparison helpers below their caller.

entry.less and missRow.less precede profile.jitRows. Move them below jitRows or inline them.

As per coding guidelines, “Go declarations should follow the repository's declaration-order rule: callers before callees.”
🤖 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 `@cli/profile_report.go` around lines 107 - 115, Move the entry.less and
missRow.less comparison helpers below their caller profile.jitRows, preserving
their existing comparison logic and behavior. Do not change unrelated
declarations or inline the helpers unless necessary.
Source: Coding guidelines

🤖 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.

Inline comments:
In `@interp/interp_test.go`:
- Around line 3587-3604: Update the profiling test around New and the
ErrFrameOverflow assertion to construct a prof.Profiler and pass it through
WithProfiler instead of assigning i.samples and i.profile directly. Close the
interpreter before inspecting the profiler’s flushed metrics, while preserving
the existing metric assertions for JIT emits, native entries, exits, and yields.

In `@interp/jit_arm64.go`:
- Around line 3219-3223: Update the guardRC and guardBoxable calls in arrayGet,
arraySet, structGet, and structSet to use a value-labeled ExitGuardValue failure
path instead of the shape-labeled fail path; preserve the existing bounds and
struct kind failure labels.

In `@interp/pool_test.go`:
- Around line 350-379: Strengthen the metric assertions in the test around the
hotCompiles, sideExitCompiles, guardExits, attempts, and emits checks to
validate the only-winners invariant rather than merely lower bounds. Compare
emitted compile records with the corresponding successful cache publication or
claim count, using exact equality so duplicate accounting by losing contenders
causes the test to fail.

---

Nitpick comments:
In `@cli/profile_report.go`:
- Around line 107-115: Move the entry.less and missRow.less comparison helpers
below their caller profile.jitRows, preserving their existing comparison logic
and behavior. Do not change unrelated declarations or inline the helpers unless
necessary.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b588c153-0325-490f-a844-9c112f4c6e83

📥 Commits

Reviewing files that changed from the base of the PR and between a2cee44 and 7d3ce48.

📒 Files selected for processing (24)

cli/profile.go
cli/profile_collect.go
cli/profile_report.go
cli/profile_report_test.go
cli/repl.go
cli/repl_test.go
docs/jit-internals.md
docs/profile.md
docs/superpowers/plans/2026-07-13-profile-report.md
docs/superpowers/specs/2026-07-13-profile-report-design.md
interp/cache.go
interp/cache_test.go
interp/interp.go
interp/interp_test.go
interp/jit.go
interp/jit_arm64.go
interp/jit_arm64_test.go
interp/pool_test.go
interp/trace.go
interp/trace_test.go
prof/collector.go
prof/jit.go
prof/jit_test.go
prof/prof_test.go

💤 Files with no reviewable changes (1)

interp/cache_test.go

coderabbitai · 2026-07-13T12:03:49Z

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

putComment timed out

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

interp/interp.go (1)
763-820: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Restore caller-before-callee declaration order.

compile/shared are declared before their later callers in safepoint (Line 976) and the side-exit path (Line 1191). Reorder these declarations so every caller precedes its callee, including shared before compile if applicable. As per path instructions, Go declarations must follow the repository’s caller-before-callee rule.
🤖 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/interp.go` around lines 763 - 820, Reorder the Interpreter methods so
each caller appears before its callee according to the repository convention:
move shared before compile if shared calls compile or otherwise ensure the
caller/callee chain from safepoint and the side-exit path is ordered correctly.
Keep the implementations unchanged and only adjust declaration placement.
Source: Path instructions

🧹 Nitpick comments (1)

interp/interp.go (1)
857-864: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use a tagged switch for entry.kind dispatch.

This three-way enum dispatch should use switch entry.kind instead of an if/else if chain with a catch-all branch, making unsupported kinds easier to handle intentionally. This corresponds to staticcheck QF1003.
🤖 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/interp.go` around lines 857 - 864, Replace the entry.kind
if/else-if/catch-all dispatch with a tagged switch in the surrounding
interpreter logic, using explicit cases for entryLoop, entryModule, and the
regular call behavior. Preserve the existing arguments, counter calculation, and
assignment to i.code[a.addr][a.ip], while making the default or unsupported-kind
handling intentional.
Source: Linters/SAST tools

🤖 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.

Outside diff comments:
In `@interp/interp.go`:
- Around line 763-820: Reorder the Interpreter methods so each caller appears
before its callee according to the repository convention: move shared before
compile if shared calls compile or otherwise ensure the caller/callee chain from
safepoint and the side-exit path is ordered correctly. Keep the implementations
unchanged and only adjust declaration placement.

---

Nitpick comments:
In `@interp/interp.go`:
- Around line 857-864: Replace the entry.kind if/else-if/catch-all dispatch with
a tagged switch in the surrounding interpreter logic, using explicit cases for
entryLoop, entryModule, and the regular call behavior. Preserve the existing
arguments, counter calculation, and assignment to i.code[a.addr][a.ip], while
making the default or unsupported-kind handling intentional.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e3738f33-8589-4133-af00-03f701ea48c5

📥 Commits

Reviewing files that changed from the base of the PR and between 651002e and 7569b5b.

📒 Files selected for processing (6)

cli/profile.go
cli/repl.go
interp/interp.go
interp/jit.go
interp/jit_arm64_test.go
prof/jit_metrics.go

🚧 Files skipped from review as they are similar to previous changes (5)

cli/repl.go
interp/jit_arm64_test.go
prof/jit_metrics.go
cli/profile.go
interp/jit.go

siyul-park added 10 commits July 13, 2026 13:38

feat(prof): add detailed JIT lifecycle metrics

ea297bb

fix(prof): bound native exit opcode keys

a7737f7

feat(interp): add detailed jit lifecycle profiling

e4575a8

fix(interp): complete jit profiling attribution

fe17e78

fix(interp): preserve cache recompile requests

c44712f

feat(cli): rank profile report

648eca0

fix(cli): preserve profile lifecycle rows

28b8970

refactor(cli): order profile constructor

3341cd4

fix(cli): align profile report contract

4b229b6

fix(cli): complete profile lifecycle coverage

a210d7e

coderabbitai Bot reviewed Jul 13, 2026

View reviewed changes

Comment thread interp/interp_test.go Outdated

Comment thread interp/jit_arm64.go

Comment thread interp/pool_test.go Outdated

siyul-park added 4 commits July 13, 2026 20:49

chore(ci): remove workflow reports

29214e4

fix(interp): align jit profiling lifecycle

b1aa350

refactor(cli): separate profile collection and reporting

0eb7200

refactor(cli): simplify profile reporting structure

25947b0

siyul-park force-pushed the feature/jit-profile-metrics branch from 7d3ce48 to 25947b0 Compare July 13, 2026 11:49

refactor(profile): remove redundant profiling state

3918f88

siyul-park added 3 commits July 13, 2026 21:08

fix(interp): address jit profiling review

22155aa

refactor(profile): align metric ownership and naming

651002e

refactor(profile): simplify metric naming

7569b5b

siyul-park merged commit b1845d2 into main Jul 13, 2026
2 of 3 checks passed

siyul-park deleted the feature/jit-profile-metrics branch July 13, 2026 12:38

coderabbitai Bot reviewed Jul 13, 2026

View reviewed changes

This was referenced Jul 15, 2026

perf: optimize primitive array mutation loops #147

Merged

perf(interp): hoist loop-invariant container state in JIT loops #154

Merged

perf(interp): expand JIT coverage for container ops and equality #158

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: align JIT profiling lifecycle and reporting#145

refactor: align JIT profiling lifecycle and reporting#145
siyul-park merged 18 commits into
mainfrom
feature/jit-profile-metrics

siyul-park commented Jul 13, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 13, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (2 warnings)

Uh oh!

codecov Bot commented Jul 13, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 13, 2026

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

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

Uh oh!

Summary

Verification

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

❌ Failed checks (2 warnings)

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 13, 2026

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

siyul-park commented Jul 13, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 13, 2026 •

edited

Loading

codecov Bot commented Jul 13, 2026 •

edited

Loading