perf(engine): cut interpreter hot-path overheads (permit churn, VC-key hashing, futures-map rehash) - #4553
perf(engine): cut interpreter hot-path overheads (permit churn, VC-key hashing, futures-map rehash)#4553aaronvg wants to merge 1 commit into
Conversation
…iling Sampling a release-grade corpus run (3,448 tests) showed the engine averaging ~1 core on a 12-core machine with three concrete taxes: 1. every VM task resume paid a full fair-queue acquire on the global heap permit semaphore (17 of 39 sampled threads sat inside `InactiveHeapPermit::acquire` / `batch_semaphore::poll`), 2. every virtual call on a primitive receiver (string/int methods — the hottest calls in practice) built and deep-hashed a `RealizedTy` under SipHash for the static virtual-call cache key, 3. the `FutureManager`'s `active_futures` map rehashed under the manager's mutex mid-run — sampled as one thread in `reserve_rehash` while every other worker parked. Fixes, all behavior-preserving: - `InactiveHeapPermit::acquire`: `try_acquire_owned` fast path. Fairness only matters once a GC park's `acquire_many(MAX_PERMITS)` is pending — and then zero permits are available, the try fails, and acquisition falls through to the fair wait, so parks cannot be starved. - `StaticVirtualReceiverKey::Primitive(u8)`: bare discriminants for the argument-less primitive receivers; no realized type built or hashed. Receivers whose identity carries type arguments keep the structural keys. - the static virtual-call cache switches SipHash for FxHash (in-process cache keyed by program structure; DoS resistance is irrelevant). - `active_futures` pre-sized to 4096 so corpus-scale future churn does not rehash inside the manager lock. Back-to-back A/B on the corpus (same machine, fasttest profile): wall 69.0s -> 67.1s (timer-bound floor), total CPU 88.2s -> 64.1s (-27%), sys time 17.0s -> 1.9s (-89%, the semaphore kernel traffic). All 580 bex_vm/bex_heap/bex_engine tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_4fd667d0-0eba-4e42-9564-e9307c3d422c) |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe runtime now pre-allocates active futures, adds a fast path for inactive heap permits, and uses compact primitive keys with ChangesRuntime optimizations
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🟡 Moderate · up to This PR reduces interpreter CPU and synchronization overhead, but merge readiness is not yet complete because each changed Rust crate still needs its cargo test --lib run completed after the earlier timeout. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Binary size checks passed✅ 7 passed
Generated by |
1 participant
Issue Reference
Changes
Profiling a release-grade corpus run (3,448 tests,
sample-based, thread-state census + symbolized frame buckets) showed the engine averaging ~1 core on a 12-core machine, with three concrete taxes. This PR removes them; all changes are behavior-preserving.1. Heap-permit fast path (
bex_heap). Every VM task resume paid a full fair-queue acquire on the global heap-permit semaphore — 17 of 39 sampled threads sat insideInactiveHeapPermit::acquire/batch_semaphore::pollmachinery.acquirenow triestry_acquire_ownedfirst. Fairness only matters once a GC park'sacquire_many(MAX_PERMITS)is pending — and in exactly that case zero permits are available, the try fails, and acquisition falls through to the fair wait, so GC parks cannot be starved by the bypass.2. Primitive receiver keys for the virtual-call cache (
bex_vm). Every virtual call on a primitive receiver — string/int methods, the hottest calls in any workload — built aRealizedTyand deep-hashed it under SipHash to key the static virtual-call cache (in an earlier debug-profile these hash/clone frames were ~65% of VM samples).StaticVirtualReceiverKeygains aPrimitive(u8)variant: bare discriminants for the argument-less primitives, nothing built or hashed. Receivers whose identity carries type arguments (instances, containers, enums) keep the structural keys. The cache also switches SipHash → FxHash (in-process cache keyed by program structure; DoS resistance irrelevant).3. Futures-map presizing (
bex_engine). TheFutureManager'sactive_futuresmap rehashed under the manager's single mutex mid-run — sampled as one thread inhashbrown::reserve_rehashwhile every other worker parked. Pre-sized to 4096 so corpus-scale future churn never rehashes inside the lock.Measurements
Back-to-back A/B on the full corpus, same machine,
fasttestprofile (release-grade codegen, debug assertions on):Total CPU −27%; sys time −89% (the semaphore's kernel wake/park traffic — the permit fast path); user time −13% (hashing). Wall moves little because the corpus floor is timer-bound (sleep- and timeout-shaped tests), not CPU-bound.
Testing
cargo nextest run -p bex_vm -p bex_heap -p bex_engine— 580/580baml-cli test— 3,447/3,448 (the one failure is the known contention-flakyvertex_stream_time_to_first_token_timeout_ms, which fails identically on baseline runs)Reviewer notes — what this deliberately does NOT do
The remaining parallelism ceiling is structural, and these are documented for follow-up rather than attempted here:
FutureManagersharding: onetokio::Mutexstill guards the futures map + TLAB; every spawn/settle serializes through it. Sharding or a lock-free map is the next real win.caller_function_addr + call_pc— a per-PC IC comparing receiver tags would skip the map entirely.TaskGroupgating intesting/registry.baml): the corpus spawns all ~3,400 tests at once; bounding admission fixes fd storms, tight-timeout flakiness, and makes per-test durations meaningful. Separate stdlib PR.Raw
sampleprofiles are available on request.🤖 Generated with Claude Code
Note
Medium Risk
Touches GC-permit fairness and virtual-dispatch cache identity. Logic is intended to be equivalent, but a keying or try-acquire mistake would mis-dispatch methods or starve GC parks.
Overview
Behavior-preserving hot-path speedups from corpus profiling: less semaphore churn on every VM resume, cheaper virtual-call cache keys, and no futures-map rehash under the manager mutex.
Heap permits:
InactiveHeapPermit::acquirenowtry_acquire_ownedfirst and only fair-waits when a GC park has drained the semaphore, so parks cannot be starved.Virtual-call cache: primitive receivers (int/string/etc.) key as a
Primitive(u8)discriminant instead of hashing aRealizedTy. The cache usesFxHashMapinstead of SipHash.Futures:
active_futuresis pre-sized to 4096 so corpus-scale churn does not rehash while holding the manager lock.Reviewed by Cursor Bugbot for commit a5f0104. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit