perf(api): box the cold entry-point futures so consumers stop re-codegening the graphs - #843
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 6 minutes and 31 seconds. Learn how PR review limits work. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
5d40649 to
2705ebe
Compare
…gening the graphs Coroutines are LocalCopy across crates: any consumer crate that awaits a lib future codegens the whole state machine graph behind it into its own binary, and inline attributes cannot change that (measured: zero effect). The demo bin re-instantiated ~1.8 MiB of the lib this way. Bot::build, Bot::run and Client::connect now delegate to a #[inline(never)] plain function that returns the graph as Pin<Box<dyn Future>>, so callers poll through a vtable and the construction/run/connect graphs are compiled once, in the lib. Public signatures are unchanged and the cost is one allocation per process for each entry point. Measured on the release bin: .text 11.85 MiB to 11.74 MiB (-113 KiB on top of the codec pinning); consumer-crate reinstantiation 1815 KiB to 1484 KiB. The remainder is the send graph reached from user handler closures; erasing it would cost a Box::pin per outgoing message (the same per-message boxing #673 removed), so it stays.
2705ebe to
bb8ac4e
Compare
1 participant
Problem
Coroutines (async fn state machines) are LocalCopy items across crates: any consumer crate that awaits a lib future codegens the entire state machine graph behind it into its own binary, with per-crate symbols that fat LTO cannot merge. Measured on the demo bin, the consumer crate re-instantiated ~1.8 MiB of the lib this way. Inline attributes do not change coroutine instantiation mode; I measured a
#[inline(never)]sweep at exactly zero effect before landing on this design.Change
Bot::build,Bot::runandClient::connectkeep their publicasync fnsignatures but now delegate through a#[inline(never)]plain function that returns the real graph asPin<Box<dyn Future>>. The consumer's copy of the entry point shrinks to a tiny shim coroutine that polls through a vtable, so the construction, run-loop and connect graphs are compiled exactly once, in the lib. Cost: one heap allocation per process for each entry point (they are all once-per-session).Measured
On top of #842:
.text11.85 MiB -> 11.74 MiB (-113 KiB); consumer-crate reinstantiation 1815 KiB -> 1484 KiB.What deliberately stays
The remaining consumer-side duplication is the send graph reached from user handler closures (
send_message_impland the cache coroutines it awaits). Erasing that would cost aBox::pinper outgoing message, the same per-message boxing #673 removed for allocation churn, so it stays static. The only stable-Rust alternative is-Zshare-generics, which is nightly.Tests
No signature or behavior changes; full wacore + lib suites pass (1838 tests).