Repository navigation
perf(http): gate the keep-alive stash shrink on an idle window - #381
Merged
Merged
Conversation
The C1M async path shrank an empty rbuf/obuf stash to the 4 KiB floor at every keep-alive re-arm, and the next request above the floor regrew it (http_async_stash_grow doubling realloc): a shrink->regrow realloc pair per request on busy large-request connections -- glibc arena lock, a possible buffer move with memcpy, fresh page touches -- a rare-but-large P99.999 stall source. The two shrink sites also disagreed (http_async_dispatch shrank to the floor; cwist_http_async_rearm freed rbuf only above 64 KiB and never touched obuf). Gate the shrink on capacity demand instead of time-since-activity: conn->stash_grown_sec (stamped at every rbuf/obuf growth, the >=16 KiB zero-copy handover, and connection setup) must be older than CWIST_ASYNC_STASH_IDLE_SEC (default 10 s, cached getter, 0 = legacy immediate shrink) before the re-arm shrinks anything. Busy keep-alive connections keep their capacity and pay zero reallocs; a quiet connection still converges to the small floor one window after its last growth, preserving the C1M idle-RSS goal (delayed by the window; glibc retains freed heap anyway, so RSS convergence is in live allocation). Both re-arm sites now share one helper with a single documented policy. A/B (8 KiB POST keep-alive, the path the contract benchmark does not exercise; 3 interleaved rounds, medians, base-vs-base noise RPS 2-4% / P99.999 19.8-83 ms): c64: RPS 190.8k -> 222.0k (+16.3%), P99.999 32.9 -> 18.5 ms (-44%) c256: RPS 262.3k -> 294.9k (+12.4%), P99.999 70.4 -> 41.7 ms (-41%) Standard small-request legs (c256/c400 cwist_bench): within noise, no regression. Tests: test_idle_reaper (also knob 0/1), test_http_fairness, test_async_defer, test_rx_epoll, test_http_pipeline, test_abi_layout pass; check_env_knobs and check_test_wiring OK.
The hysteresis anchor appends one field to cwist_http_async_conn_t; regenerate the baseline so the CI drift gate passes. The baseline diff is the PR's record of the layout change (pre-v4 guarantee check stays skipped until the v4 tag exists).
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.
Summary
conn->stash_grown_sec(new field, stamped at every rbuf/obuf growth) must be older thanCWIST_ASYNC_STASH_IDLE_SEC(default 10 s,0= legacy immediate shrink) before the re-arm shrinks anything.cwist_http_async_rearmfreed rbuf only >64 KiB and never touched obuf) — both now share one helper with a single documented policy.A/B (8 KiB POST keep-alive — the path the contract benchmark doesn't exercise; 3 interleaved rounds, medians; base-vs-base noise RPS ±2–4%, P99.999 19.8–83 ms)
Both beyond noise by the merge rule. Regression legs (standard small-request cwist_bench, c256/c400): within noise, no regression (c256 slightly favorable).
Path-firing proof: instrumented counters showed baseline = 4 GROW + 3 SHRINK per 3-request session vs candidate = 1 GROW + 0 SHRINK checks passed (instrumentation reverted before commit).
Notes
stash_grown_sec, notlast_active_sec— the latter is stamped immediately before both shrink sites, so a literal check would never shrink (failing the RSS goal).CWIST_ASYNC_STASH_IDLE_SEC=0is byte-identical at the REARM site; the continue-path's legacy asymmetry (no obuf handling, different rbuf policy) is intentionally fixed by the unification.Test plan
test_idle_reaper(also with knob0and1),test_http_fairness,test_async_defer,test_rx_epoll,test_http_pipeline,test_abi_layout— passcheck_env_knobs.py(incl. doc-twin check),check_test_wiring.py— OKtest_idle_reaper/test_http_fairness/test_async_deferpass