perf(context): eliminate per-poll context clone overhead - #163
perf(context): eliminate per-poll context clone overhead#163nicolasburtey wants to merge 1 commit into
Conversation
EventContextFuture::poll ran two ContextData clones plus a thread-local stack walk on every poll of every request/job root future. ContextData was an im::HashMap, so each clone walked the persistent chunk tree bumping Arc refcounts. - ContextData is now a copy-on-write Arc<Vec<(key, value)>>: cloning is one atomic refcount bump; mutation clones the tiny entry vector only when shared. The public API and the stored JSON shape (a plain object) are unchanged. - Polls only write the context back into the wrapper when the inner future actually mutated it (dirty flag on the stack entry), which almost never happens - inserts happen once at request setup. - Drops the im dependency.
|
I’d want to see local profiling as proof that this is better. The im HashMap should already be optimized for cloning due to structural sharing. |
|
I'll take this over |
|
Benchmarking this change in isolation (criterion suite in #164, cherry-pickable onto either branch) pointed us to a different fix, now up as #164: an 🤖 Generated with Claude Code |
Motivation
This came out of a 5-hour DB-bottleneck stress test of lana-bank
(sandbox
sb-str3lps,stress-testingpreset at a 3 loans/sec target,lana-bank @
314fbd8, es-entity pinned at 0.11.7). The headline numbersfrom that run:
max (Honeycomb kubelet stats) the entire run, while the database sat
largely idle behind advisory-lock waits.
The direct cap is cala's per-balance
pg_advisory_xact_lock(meanwait ~700ms), but the lock holds are stretched app-side: holders
sit
idle in transactionbetween statements because the app has nospare CPU. Framework CPU overhead therefore converts directly into
lock queueing and lost throughput — every percent of wasted app CPU
is a percent of pipeline throughput.
The 60s
/debug/pprofCPU capture showed no single hot function; theburn is framework-level per-operation overhead, and es-entity's event
context is a measurable part of it:
<EventContextFuture as Future>::pollis an ancestor of 51% ofall samples (it wraps every request/job root future).
std::thread::local::LocalKey::try_withis on the stack of 63%of samples.
sized_chunks::Chunk::clone(theimpersistent-map internalsbehind
ContextData::clone) shows up as its own profile row.malloc/free~15%)and atomic refcount ops (~2%) — exactly the cost shape of the
clone-heavy context machinery.
The problem, mechanically
EventContextFuture::pollruns on every poll of every request/jobfuture — futures are re-polled at every
.awaitresumption, so arequest that suspends 50 times pays this 50 times. Each poll did:
context_data.clone()—ContextDatawasim::HashMap<Cow<'static, str>, serde_json::Value>. Cloning animmap walks the persistent chunk tree bumpingArcrefcounts.EventContext::seed(clone)— TLS push +Rcallocation.ctx.data()— a second TLS walk and a second full clone,unconditionally written back into the wrapper.
Plus
EventContext::drop: third TLS access, reverse stack scan, anda
Vec::remove. All of this to support a write-back that is onlymeaningful when the inner future mutated the context — which in
practice happens once at request setup (the
#[es_event_context]macro's inserts), i.e. in a vanishing fraction of polls.
Separately, every event persist calls
data_for_storing(), which alsopays the
imclone — the stress run wrote on the order of 10M eventrows.
The fix
Two contained changes, no public API or behavior change:
ContextDatais now copy-on-writeArc<Vec<(Cow, Value)>>.Clone = one atomic refcount bump. Mutation clones the entry vector
only when shared (
Arc::make_mut). Context maps hold 0-3 entriesin practice, so a linear-scan vector beats both hashing and a
persistent tree on every operation this type actually performs.
Custom
Serialize/Deserializeimpls preserve the exact storedJSON shape (a plain object — what
#[serde(transparent)]over themap produced), so existing
contextcolumns and event payloadsround-trip identically. This also makes
data(),fork()anddata_for_storing()cheap everywhere, not just in the poll path.StackEntrygains adirtyflag set byEventContext::insert;the wrapper only clones data back out (new crate-private
data_if_dirty()) when an insert actually happened during thatpoll. Semantics are unchanged: unchanged context propagates
unchanged.
Drops the
imdependency (it was used nowhere else).Preserved invariants: the per-poll re-seed itself stays — on
tokio's multi-thread scheduler a task's polls hop worker threads and
TLS doesn't travel with the task, so seeding at each poll is what
guarantees
EventContext::current()inside the future resolvescorrectly. Only the redundant clones around it are gone.
Expected effect
Direct profile attribution for this machinery is ~2-5% of app CPU
(chunk-tree clones, Rc traffic, TLS walks, plus the per-persist clone
across ~10M event writes) — modest in isolation, but on this workload
it sits directly upstream of the throughput-capping lock holds, and it
compounds with the other allocation-rate reducers (tracing span
volume, allocator swap) rather than overlapping them.
Validation plan
stress-testingsandbox at 3 loans/sec with alana-bank build on this es-entity version; compare Honeycomb
container.cpu.usageAVG, calafind_for_updatemean advisory-lockwait, and achieved loans/sec against the baseline from the stress
run (5.2 cores avg / ~700ms lock wait / ~2.2 loans/s).
Testing
replacement, dirty-flag write-back behavior.
cargo clippy --workspace --all-featuresandcargo fmt --checkare clean.
this environment); the change touches no SQL, and
ContextData'ssqlx
Encode/Decodego through the preserved serde shape.Note
Medium Risk
Touches thread-local context propagation on every async poll and persisted event context JSON shape; behavior is intended to be unchanged but regressions would affect audit metadata and cross-task context.
Overview
Reduces hot-path CPU and allocation cost in event context propagation by changing how
ContextDatais stored and when async wrappers write context back after each poll.ContextDatano longer usesim::HashMap(and theimcrate is removed). It is nowArc<Vec<(Cow, Value)>>with copy-on-write on insert, soclone()is a cheap refcount bump instead of walking persistent map chunks. CustomSerialize/Deserializekeep the stored JSON as a plain object for DB/event payloads.len/is_emptyare added onContextData.EventContextFuture::pollstill re-seeds TLS each poll, but write-back uses newdata_if_dirty()instead of unconditionalctx.data().StackEntrytracks adirtyflag set oninsert, so read-only polls skip cloning context back into the wrapper.Unit tests cover serde round-trip, key replacement, and dirty write-back behavior.
Reviewed by Cursor Bugbot for commit 81527d2. Bugbot is set up for automated code reviews on this repo. Configure here.