fix: bound idle streaming buffer retention - #532
Open
Mohamed Mansour (mohamedmansour) wants to merge 1 commit into
Open
fix: bound idle streaming buffer retention#532Mohamed Mansour (mohamedmansour) wants to merge 1 commit into
Mohamed Mansour (mohamedmansour) wants to merge 1 commit into
Conversation
Drop oversized pool returns and reserve smaller acquired buffers from their cleared length. Preserve existing coalescing targets, headroom, caller sizing, transport behavior, and final-consumer ownership. Document the idle-only memory bound and add focused regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Mohamed Mansour (mohamedmansour)
marked this pull request as ready for review
September 12, 2026 00:53
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.
Measured retention/CPU tradeoff
The isolated pool comparison is complete. This is a bounded idle-retention
fix, not a blanket speed or process-RSS improvement. Recurring 1 MiB
quoted attributes incurred approximately 50% higher latency and CPU in
the matched comparison below because their oversized allocations are no
longer retained. Large writes still remain whole; the hard-chunking
regression from #527 is not included.
The complete transport/pool report
includes every measured cell, repeated-run ranges, allocation accounting,
output hashes, and limitations. The reproduction harness and instructions
are published on the held experimental PR, not added to this focused fix.
Summary
Extract the idle-pool capacity fix from #527 without its hard-chunking change. This independent branch is based on
2dab6fc6b7c6f4cdd005fed00f18e28c840248fd.Only two operations change executable behavior:
chunk_size. Do not retain or shrink it.reserve_exact(chunk_size). The oldreserve(chunk_size - capacity)can return an undersized buffer becausereserveis relative to length.Preserve the existing writer's coalescing target, 1 KiB headroom, caller pool sizes, channel behavior, flush behavior, timeouts, and consumer-owned buffer lifetime. Large writes still remain whole; this PR does not impose a hard 4 KiB limit or claim a 16 KiB queue-byte ceiling.
Update the specification and public memory/sizing guidance, including corrections to the old all-per-flush-allocation and total-memory claims. Add seven focused regressions. No production dependency or public API is added.
Resource bound and tradeoff
Before, the pool bounded only the number of retained buffers; each could retain a capacity grown by a large write. After, summed idle recorded
Veccapacity is bounded by:These are source-derived bounds, not measured RSS:
Allocator, queue, pool, and
Bytes::from_ownermetadata remain additional. So do active producer buffers, pending sends, queued chunks, and consumer-held references. The bound is not a claim about total server memory or allocator RSS retention.Default caller pools remain at
StreamingWriter::CHUNK_TARGET + 1024(5 KiB). Custom targets need the same headroom. Recurring oversized writes can incur additional allocation/deallocation instead of retaining their peak buffer. This fix makes retention predictable; it does not promise universally faster pooled writes.The old CLI, watcher, and hard-4-KiB measurements from #527 are not isolated performance evidence for this pool-only change.
Isolated pool-only measurements
Both variants use a 4096-byte soft coalescing target, 5120-byte pool buffers,
16 pool slots, and four channel slots. This preserves the original
recommended headroom. The pool-only streaming source matches this PR's
a7cba0abaf0f3beccf5dda3e4fc4de2104f5fcb4byte-for-byte. Other source anddependencies are the same umbrella snapshot
e274c8b8; the original writeris restored from
f43db650(unchanged on this PR's main base2dab6fc6).Release Rust 1.98.0, thin LTO, one codegen unit, x86-64 Linux/WSL2. Three
separate process runs per cell, rotated variant order, five warmups, and a
persistent concurrent consumer. Fast mixed cells use 15 batches of 100
requests; other fast cells use 15 batches of 20; paced cells use nine
individual requests per process. One response is active at a time; this is
not multi-request contention or whole-server throughput.
Latency is the median of three process-run request medians. CPU is the
median of three CPU/request means, including producer and consumer; it
is not a per-request CPU median or producer-only CPU.
The small apparent improvements are not claimed as consistent wins. For the
large attribute, the three process-P50 ranges were 65.496-78.149 us before
and 86.257-101.057 us after. These ranges are not confidence intervals.
The paced raw case maintains approximately 125.3 ms completion under the
same byte rate, but its reported combined producer/consumer CPU is about
38% higher. Its three-run CPU-mean ranges were 177.8-196.0 us before
and 244.2-263.2 us after. Equal paced completion time is not proof of equal
CPU efficiency.
Separate warmed allocation accounting observed 1 to 3 allocation calls
per raw response and 1 to 4 per quoted-attribute response. For the latter,
requested bytes rose from 40 to 2,097,218 per response. The counters
include allocation/zeroed-allocation calls and growing realloc calls, with
realloc bytes counted as growth deltas; they also include harness allocations.
They measure allocation traffic, not simultaneously live heap or RSS.
Mixed and unpooled raw allocation controls were unchanged.
All output bytes and semantic flush offsets were checked separately, and the
large raw/attribute responses still use one transport chunk. WSL scheduling
and frequency noise remain. Five warmups are not a proof that every pool's
inventory is fully steady. No throughput or request-RSS claim is inferred
from these numbers.
Regression coverage
Seven regressions cover normal reuse and clearing; capacity-based rejection with a small payload; exact-size admission; idle-count limits including zero
max_pool; reserve-from-length behavior; summed idle capacities; and final clone/slice lifetime across threads.The large-write regression additionally proves that output still arrives as one oversized transport chunk, remains byte-identical while cloned, is not pooled after the last clone drops, and does not prevent later normal-sized reuse. Four of the new regressions fail against the original pool implementation.
Validation and independence
cargo test -p microsoft-webui streaming::passed all 23 selected tests. Completecargo xtask checkpassed in 75.1 seconds, including docs, workspace/WASM/example builds, and benchmark smoke.This is ready for review as an idle-retention fix with the measured CPU/latency
tradeoffs above, not as an unqualified performance win. It can merge independently
of CLI #529 and watcher #530; all three branch pairs merge without textual
conflicts. The combined #527 remains draft because the completed experiments do
not justify any tested hard chunk limit as a general default.