perf(context): lazy event-context seeding — skip stack work on untouched polls - #164
Merged
Merged
Conversation
Criterion benches (structured after cala-perf) isolating the paths PR #163 touches, using only public API shared by the im and Arc<Vec> implementations so the commit can be cherry-picked onto either side for an A/B comparison: 1. per_poll_overhead - EventContextFuture wrapper cost per poll (no-op waker) 2. tokio_yield - same on a 1-worker tokio runtime for realistic scale 3. context_data_clone - the per-poll ContextData clone 4. context_lifecycle - seed/drop (kept by the PR) and seed/insert/drop (CoW) 5. persist_path - EntityEvents::push (real data_for_storing) + to_value Run with --features tracing-context to include the per-persist tracing insert. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hed polls EventContextFuture::poll previously did, on every poll of every wrapped request/job future: clone the context data, push a stack entry (TLS access + Rc allocation), and on exit clone the data back out and remove the entry (two more TLS accesses). Criterion measurement of that wrapper cost: ~21ns/poll, ~18ns of which is the seed/drop machinery — independent of the ContextData container (an im map clone is a flat ~3.4ns regardless of size). Almost no poll actually observes the context: EventContext is touched once at request setup (macro inserts) and once per event persist. So the wrapper now parks its ContextData on a thread-local pending-seed stack (a move — no clone, no allocation) and only materializes a real CONTEXT_STACK entry if the inner future calls EventContext::current() or ::seed() during that poll. On exit the data is either taken back unchanged (fast path) or harvested from the materialized entry exactly as before. Ordering invariant: pending seeds always sit logically above every entry that existed when they were parked; current() and seed() materialize all pending seeds (in push order) before touching the stack, so nested wrappers, fork(), and direct seeds interleave identically to eager seeding. A guard pops the pending record on panic-unwind, dropping any materialized handle through the normal EventContext drop path. Measured (criterion, vs main): - wrapper cost per poll: 21.4ns -> 12.6ns (-41%) - wrapped 65-yield tokio task: 1.91us -> 0.89us (-54%); ~5.8ns/poll added - untouched polls now perform zero heap allocations and zero clones - explicit EventContext::seed(): +10% (~2ns, materialize check) - EntityEvents::push persist path: unchanged (p > 0.05) Public API and semantics are unchanged. Two cfg(test) stack-depth assertions were updated: the wrapper's entry now appears only after the context is first observed within the poll. Polling an EventContextFuture again after a poll panicked now panics with an explicit message (futures are contractually dead after a panic). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The pending-seed stack, materialization, and poll guard exist solely to support EventContextFuture's lazy seeding — they are wrapper plumbing, not part of the EventContext/ContextData core, so they live with the future code. mod.rs keeps only the two materialize calls at its stack entry points (current/seed). SeedGuard drops from pub(crate) to module-private. No behavior change; poll-path benchmarks unchanged (-40% vs main). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 task
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
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.
Motivation
Follow-up to the review discussion on #163. Benchmarking that PR's two changes in isolation (criterion suite included here; A/B protocol below) showed:
im::HashMapclone is a flat ~3.4 ns regardless of size — clone cost was never the problem, structural sharing already makes it O(1)Rc::new(())heap allocation, and a stack push/pop — which is independent of the container and survives perf(context): eliminate per-poll context clone overhead #163 unchanged (perf(context): eliminate per-poll context clone overhead #163 lands at ~18.6 ns/poll)So this PR keeps
imand instead removes the machinery cost for the polls that don't need it — which is almost all of them:EventContextis observed once at request setup (the#[es_event_context]macro's inserts) and once per event persist, while the wrapper'spollruns at every.awaitresumption.Design
EventContextFuture::pollno longer eagerly creates a context-stack entry. Instead it parks itsContextDataon a thread-local pending-seed stack (a move — no clone, no allocation) and pops it on exit:CONTEXT_STACKtraffic — just two thread-localVecpush/pops.EventContext::current()/seed()first materialize all pending seeds (in push order) into real stack entries, then proceed exactly as before. On poll exit the wrapper harvestsctx.data()from the materialized entry — identical write-back semantics, including the entry surviving if the inner future kept a handle across the poll.Ordering invariant: pending seeds always sit logically above every entry that existed when they were parked; since every push path (
current/seed/fork) materializes first, nested wrappers, forks, and direct seeds interleave identically to eager seeding.Panic safety: a guard pops the pending record on unwind and drops any materialized handle through the normal
EventContextdrop path — write-back skipped, matching eager behavior. (Re-polling anEventContextFutureafter a panicked poll now panics with an explicit message; futures are contractually dead after a panic.)Measured effect
Criterion, Apple Silicon,
--baseline im= currentmain(reproduce:cargo bench --bench context -- --save-baseline imonmain+ the bench commit, then-- --baseline imhere):main(im, eager)Arc<Vec>)Rc::new)Rc::new)EntityEvents::pushpersist pathEventContext::seed()Trade-offs, honestly: direct
seed()and first-observation polls pay a ~2 ns materialize check; the persist path is unchanged (this PR deliberately doesn't touchContextData— container/serialization changes can be argued separately on hygiene grounds).Scale disclaimer (why this is framed as hygiene, not throughput): even at 1M poll resumptions/sec the eager machinery costs ~2% of one core. What this PR does eliminate at any rate is the per-poll allocation and clone traffic — the
malloc/freeandLocalKey::try_withsignals visible in the lana-bank stress-run profile.Testing
nix run .#nextestgreen (full DB-backed integration suite against live process-compose Postgres); all 37 lib tests + doctests pass;cargo clippy --workspace --all-featuresclean;cargo fmt --checkclean.benches/context.rs, structured after cala-perf) is included so the numbers are reproducible on any branch — it only uses public API, so it cherry-picks ontomainand perf(context): eliminate per-poll context clone overhead #163 alike.🤖 Generated with Claude Code
Note
Medium Risk
Changes async context propagation semantics (lazy vs eager stack depth) across spawned tasks and panics; behavior is heavily tested but ordering bugs could affect event context on persist.
Overview
with_event_contextno longer eagerly seeds the context stack on every poll. Each poll parksContextDataon a thread-local pending-seed stack; a real stack entry is created only when the inner future callsEventContext::current()orseed()(viamaterialize_pending_seeds). Untouched polls avoidRcallocation,ContextDataclone, andCONTEXT_STACKpush/pop.EventContext::seed/currentmaterialize pending seeds before proceeding; stack pushes go through a new privatepush_entry.SeedGuardhandles normal write-back and panic unwind so stacks stay balanced.Adds
benches/context(Criterion) for per-poll overhead, tokio yield, clone, lifecycle, and persist paths, plus tests for lazy depth, untouched polls, fork ordering, and panic cleanup.Reviewed by Cursor Bugbot for commit 03274df. Bugbot is set up for automated code reviews on this repo. Configure here.