bridge-cpp: run host callables inline instead of one thread per crossing#4097
Open
codeshaunted wants to merge 1 commit into
Open
bridge-cpp: run host callables inline instead of one thread per crossing#4097codeshaunted wants to merge 1 commit into
codeshaunted wants to merge 1 commit into
Conversation
The C++ trampoline spawned a detached std::thread for every BAML -> host callable crossing, mirroring bridge_python's spawned tokio task. A tokio task is ~free; an OS thread is not. On an M-series Mac the spawn alone measures 7.7us, and a crossing that does no work costs ~29us -- all of it overhead, on a path hot enough to matter (an embedded game loop issues thousands of these per second). Run the dispatcher inline on the engine's worker instead. This is safe because host_impls installs the in-flight entry *before* firing the dispatch, so completing from inside the callback resolves an already-registered oneshot. Inline dispatch does occupy the worker for the duration of the callable, which would strand the runtime if the callable synchronously re-enters the engine. So fire_dispatch now announces the blocking section with block_in_place on a multi-thread runtime, migrating the remaining tasks off this worker. Re-entrancy keeps working -- it is not part of the supported contract, but it worked before this change and still does. block_in_place panics on a current-thread runtime and is meaningless outside a runtime context, so both fall back to a direct call. Measured on aarch64-apple-darwin, 6 crossings per call, 400 iterations: before 29.08 us/crossing after 8.47 us/crossing (3.4x) Re-entrant callable (host callable that calls back into BAML) verified working before and after.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ 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):
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughHost dispatch now runs inline instead of spawning detached threads. The native Rust path uses ChangesInline host dispatch
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant EngineWorker
participant HostTrampoline
participant HostDispatchFn
participant TokioRuntime
participant InFlightCall
EngineWorker->>HostTrampoline: invoke host-callable trampoline
HostTrampoline->>HostDispatchFn: dispatch inline
HostDispatchFn->>TokioRuntime: inspect current runtime
TokioRuntime->>HostDispatchFn: use block_in_place or direct invocation
HostDispatchFn->>InFlightCall: complete call
HostTrampoline->>InFlightCall: complete bridge failure on dispatcher exception
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The C++ trampoline spawned a detached
std::threadfor every BAML → host callable crossing:This mirrors
bridge_python::host_value, which spawns a tokio task. A tokio task is ~free; an OS thread is not. On an M-series Macstd::threadspawn+detach alone measures 7.7us, and a crossing that does no work costs ~29us — all overhead, on a path hot enough to matter (an embedded game loop issues thousands of these per second).This PR runs the dispatcher inline on the engine's worker instead.
Why it's safe
host_implsinstalls the in-flight entry (InflightGuard::new(call_id)) before callingfire_dispatch, so completing from inside the dispatch callback resolves an already-registered oneshot. The awaiting future simply polls ready.Inline dispatch does occupy the worker for the duration of the callable, which would strand the runtime if the callable synchronously re-enters the engine. So
fire_dispatchnow announces the blocking section withblock_in_placeon a multi-thread runtime, migrating the remaining tasks off this worker.Sync re-entrancy is not part of the supported contract (
host_dispatch.rs: "No synchronous re-entrancy"), but it did work before this change by accident of the thread spawn — so it is preserved rather than regressed.block_in_placepanics on a current-thread runtime and is meaningless outside a runtime context, so both fall back to a direct call.Measurements
aarch64-apple-darwin, a BAML function making 6 crossings, 400 iterations:block_in_place)Verified separately, before and after: a host callable that calls back into BAML synchronously returns the correct result. With inline dispatch but without the
block_in_placehalf, that same test deadlocks — which is what the second half of this change buys.Test notes
cargo test -p sys_native host_dispatchpasses; workspace clippy (--all-targets --all-features -D warnings) is clean.Note that
sdks/cpp/bridge_cpp/tests/run.shdoes not build oncanaryat the moment, independent of this change:runtime_smoke.cc:42readsstarted.envelope, butenvelopeis a member ofcall_state, notcall_registry::started. Untouched here.Summary by CodeRabbit
Performance
Reliability