Skip to content

perf: cut per-render handler allocations - #511

Merged
Mohamed Mansour (mohamedmansour) merged 2 commits into
mainfrom
mohamedmansour-reduce-handler-render-allocations
Sep 3, 2026
Merged

perf: cut per-render handler allocations#511
Mohamed Mansour (mohamedmansour) merged 2 commits into
mainfrom
mohamedmansour-reduce-handler-render-allocations

Conversation

@mohamedmansour

@mohamedmansour Mohamed Mansour (mohamedmansour) commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Why

Server memory is not cheap, and the handler's render path still paid a steady stream of avoidable heap allocations on every request. Following up on #510, this pass targets the remaining per-request allocation sources in webui-handler without giving up any throughput.

The largest one was hiding in the fragment-graph walks. Every walk (reachability, inventory, chain, nested params) queued a QueuedFragment { id: String, route_base: String } per graph edge and keyed its visited set on (String, String), so traversing a component graph cost roughly four String allocations per edge. These are not startup-only paths: collect_reachable_component_order_for_request fires at the body_end boundary on every full render that has a hydration plugin attached, which is the standard configuration.

What changed

Allocation-free graph traversal. QueuedFragment now borrows the protocol's fragment name (&'protocol str) and stores its route base as a u32 index into a small interning arena, making the struct Copy. Route nesting is shallow, so the arena's linear dedup scan beats hashing and stops allocating after each level is seen once. The visited key becomes (&'protocol str, Option<u32>), where None collapses every base into one bucket for route-insensitive walks, matching the previous empty-string sentinel. This required threading a 'protocol lifetime through collect_inventoryable_components_from_stack, walk_route_children, collect_route_boundary_components, collect_inventory_and_chain, and walk_children_for_inventory_and_chain.

Stack-rendered JSON scalars. Interpolating a non-string signal called Value::to_string() and then ran the result through encode_safe. Numbers, booleans, and null can never contain HTML-significant bytes, so they now render through a fixed-capacity ScalarBuffer and skip escaping entirely. Applied to write_signal_value, dynamic attribute emission, and template attribute assembly.

Residual per-route clones. route_base is saved with mem::replace instead of a deep Cow clone (inside is_matched, best_route is always Some, so the slot is unconditionally overwritten). process_component and emit_css_module take &str, so generated route hosts no longer build a throwaway WebUIFragmentComponent just to pass a name. Nested route params merge entry-wise instead of cloning an entire HashMap only to move its entries out.

Performance

Measured with cargo xtask bench streaming-resource (exact counts from a custom GlobalAlloc), release build, 2000 iterations per row.

Metric Before After Change
string allocs/run 156 141 -9.6%
streaming allocs/run 170 155 -8.8%
streaming POOLED allocs/run 163 148 -9.2%
streaming bytes/run 43.7 KiB 43.6 KiB -0.2%
POOLED wall us/run 1000 (median, n=5) 13.92 13.65 -1.9%, within noise
Output size 24152 B 24152 B unchanged

Output bytes are byte-identical at all three scales, which is the correctness proof. Wall time improved alongside the allocation count, so nothing was traded away for the memory win.

Correction. An earlier revision of this description listed process RSS @1000 | 14.12 MiB | 12.83 MiB | -9.1% as a headline metric. That row was sampling noise and has been removed. It was a single sample per build, and the benchmark's process RSS column turns out to be multimodal. Re-running the identical binary 8 times gives 12.66 / 12.73 / 12.94 / 13.20 / 13.70 / 13.97 / 13.98 / 14.02 MiB - a 1.36 MiB spread, which is wider than the 1.29 MiB "improvement" originally claimed. The column is dominated by where the allocator places the ~6.8 MiB scale-1000 fixture state, which is built before any render runs, so it is largely blind to the render path regardless. allocs/run and bytes/run are exact counts from a custom GlobalAlloc and are the numbers to judge this PR on. Tracked in #516.

Second correction, same cause. The wall-time row originally read 15.46 -> 13.74 us, -11.1%, also a single sample per build. A proper A/B - baseline built from 78b7617f in a separate worktree, the two binaries run alternately five times each - gives medians of 13.92 vs 13.65 us (POOLED/1000) and 13.28 vs 13.05 us (string/1000), about -1.9% and -1.7%, with the two ranges overlapping. The original 15.46 baseline was a cold first-run outlier. The honest statement is that this PR shows no measurable wall-time regression and possibly a small improvement that n=5 cannot separate from noise - not the double-digit speedup first claimed.

The same A/B re-confirms the allocation counts are exact and deterministic: 156 and 163 in all five baseline runs, 141 and 148 in all five patched runs, zero variance. That is the result this PR rests on.

Notes for reviewers

These numbers understate the real-world gain. The streaming-resource fixture renders the contact-book dashboard, whose output is ~24 KB whether the state carries 10, 100, or 1000 contacts, because the scaled contacts array is not rendered at /. That makes it a good zero-copy proof (state tree size does not affect render allocations) but it exercises a shallow route graph. Savings here scale with route depth and component-graph size, so apps with deeper route trees benefit proportionally more.

ScalarBuffer overflow is reported, not truncated. write_str returns fmt::Error rather than silently clipping, so format_plain_json_scalar declines and the caller falls back to the allocating path. The 48-byte capacity is comfortably above the 24-byte worst case for an f64 (serde_json is built without arbitrary_precision), so the fallback is unreachable in practice while staying correct if that ever changes. Added scalar_buffer_tests asserting byte-identical output versus Value::to_string() across i64::MIN, i64::MAX, u64::MAX, f64 extremes, and MIN_POSITIVE, plus coverage that strings/arrays/objects correctly decline so nothing escapes unescaped.

Deliberately left out of scope. context.route_children is still a Vec<WebUiFragmentRoute> deep-cloned from the protocol per matched route, and rendered_components / the local_vars and component_attrs keys are still String despite always originating from &'protocol str. All three want the same fix, but it requires threading a lifetime through SessionCore and ContinuationVm, which are currently lifetime-free because they hold this data across streaming suspension. That is a real payoff but a much riskier refactor, so it belongs in its own PR rather than bundled here.

Validation

  • cargo xtask check (all phases pass)
  • cargo xtask bench streaming-resource
  • 489 handler tests pass, including the 3 new scalar-rendering tests

No public API, protocol schema, FFI surface, or user-facing behavior changed, so DESIGN.md and docs/ need no updates. No new dependencies.

Reduce steady-state heap pressure in the render hot path without
changing a single output byte.

Fragment-graph walks (reachability, inventory, chain, nested params)
queued one `QueuedFragment { id: String, route_base: String }` per graph
edge and keyed their visited set on `(String, String)`, so every edge
cost two heap allocations plus two more to record the visit. These walks
run per request - `collect_reachable_component_order_for_request` fires
at `body_end` on every full render with a hydration plugin. Queue entries
now borrow the protocol's fragment name and index route bases through a
small interning arena, making `QueuedFragment` `Copy` and the traversal
allocation-free.

Interpolating a non-string signal called `Value::to_string()` and then
ran the result through `encode_safe`. Numbers, booleans, and null can
never contain HTML-significant bytes, so they now render through a
fixed-capacity stack buffer and skip escaping entirely. Overflow is
reported rather than truncated, so the allocating fallback still covers
any value the buffer cannot hold.

Also drop the residual per-route clones: `route_base` is saved with
`mem::replace` instead of a deep `Cow` clone, `process_component` and
`emit_css_module` take `&str` so generated route hosts no longer build a
throwaway `WebUIFragmentComponent`, and nested route params are merged
entry-wise instead of cloning a whole `HashMap` to move out of.

| Path | Before | After | Change |
|---|---:|---:|---:|
| string allocs/run | 156 | 141 | -9.6% |
| streaming allocs/run | 170 | 155 | -8.8% |
| streaming POOLED allocs/run | 163 | 148 | -9.2% |
| streaming bytes/run | 43.7 KiB | 43.6 KiB | -0.2% |
| POOLED wall us/run @1000 | 15.46 | 13.74 | -11.1% |
| process RSS @1000 | 14.12 MiB | 12.83 MiB | -9.1% |
| Output size | 24152 B | 24152 B | unchanged |

Savings scale with route and component-graph size, so apps with deeper
route trees than the contact-book fixture gain proportionally more.

Validation: `cargo xtask check`, `cargo xtask bench streaming-resource`.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2edb7e98-5706-46bc-b191-51620cd290e5

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It changes core routing/rendering hot paths and threads new lifetimes through traversal logic, so it warrants final human review despite added tests and benchmark validation.

Pull request overview

This PR continues the allocation-reduction work in webui-handler by eliminating several per-request heap allocations in fragment-graph walks and scalar JSON rendering on the hot render path.

Changes:

  • Reworked fragment-graph traversal to queue fragments using borrowed IDs and route-base interning (RouteBaseArena), reducing per-edge String allocations.
  • Added stack-based JSON scalar formatting (ScalarBuffer) to avoid Value::to_string() allocations and unnecessary escaping for numbers/bools/null.
  • Reduced route/render clones by switching some APIs to take &str and using mem::replace/entry-wise merges instead of deep clones.
File summaries
File Description
crates/webui-handler/src/streaming/vm.rs Updates VM-to-handler calls to pass borrowed fragment IDs, avoiding temporary component wrappers.
crates/webui-handler/src/route_handler.rs Introduces route-base interning + borrowed queued fragments across multiple graph walks, threading a protocol lifetime through traversal helpers.
crates/webui-handler/src/lib.rs Adds ScalarBuffer + scalar fast-path rendering and refactors component/CSS emission APIs to accept &str, reducing clones/allocations in render paths.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread crates/webui-handler/src/route_handler.rs Outdated
`RouteBaseArena::intern` took `&str` and stored misses via `to_string()`,
but five of its six call sites pass a temporary produced by
`route_matcher::compute_route_base`, which already returns an owned
`String`. Discovering a new route level therefore allocated twice: once
to build the path and once to copy it into the arena.

Split the dedup scan into `find` and add `intern_owned`, which moves the
caller's `String` in on a miss. The borrowed `intern` stays for the one
site that genuinely holds a `&str`.

Allocation counts and output bytes are unchanged on the contact-book
fixture (141 / 155 / 148 allocs per run, 24152 B at scale 1000) because
its route graph is shallow enough that no nested base is ever interned.
The saving applies per newly discovered route level, so it shows up on
deeper route trees.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2edb7e98-5706-46bc-b191-51620cd290e5
@mohamedmansour
Mohamed Mansour (mohamedmansour) merged commit f405b7f into main Sep 3, 2026
35 checks passed
@mohamedmansour
Mohamed Mansour (mohamedmansour) deleted the mohamedmansour-reduce-handler-render-allocations branch September 3, 2026 05:58
Mohamed Mansour (mohamedmansour) added a commit that referenced this pull request Sep 3, 2026
Matched route descent deep-cloned the entire remaining
`WebUiFragmentRoute` subtree. Because each route recursively owns
strings, child vectors, cache tags, and invalidation data, the cost
grows with route depth and sibling count on every request.

This change keeps protocol-owned route levels borrowed during ordinary
rendering and materializes them only when a streaming continuation must
outlive that borrow.

## Approach

- Represent the active route level as `Cow<'protocol,
[WebUiFragmentRoute]>`.
- Borrow child levels directly while descending protocol-owned routes.
- Keep parked `SessionCore` and `ContinuationVm` route levels owned
across host calls.
- Move children out of an already-owned route with `mem::take` instead
of recursively cloning them.
- Preserve the existing second-`<outlet />` behavior exactly; its latent
bug remains isolated in #515.

The scope maps and name sets deliberately remain `String`-keyed, and the
scope-map pool remains live across suspension steps. An earlier broad
`Cow<str>` prototype improved one-step rendering but a real
three-boundary session exposed a deterministic regression: 243 to 249
allocations and 25,081 to 29,539 allocated bytes per run. That design
was rejected rather than accepting a streaming trade-off.

## Exact allocation results

Baseline is the current stacked base, #511 at `e908d327`. Counts come
from the benchmark counting allocator and were deterministic across
repeated runs.

`streaming_resource_bench`, scale 1000:

| Path | Before allocs / bytes | After allocs / bytes | Output |
|---|---:|---:|---:|
| string | 141 / 31,237 | 127 / 29,488 | 24,152 B |
| streaming one-step | 155 / 44,637 | 141 / 42,888 | 24,152 B |
| streaming one-step POOLED | 148 / 8,893 | 134 / 7,144 | 24,152 B |

The same 14-allocation and 1,749-byte reduction holds at scales 10 and
100. Output remains byte-identical at all scales: 24,114 / 24,134 /
24,152 B.

Nested routes:

| Request path | Before allocs / bytes | After allocs / bytes | Output |
|---|---:|---:|---:|
| `/` | 58 / 8,418 | 49 / 7,662 | 3,366 B |
| `/sections/a` | 86 / 9,944 | 77 / 9,188 | 3,902 B |
| `/sections/a/topics/b` | 107 / 11,388 | 98 / 10,632 | 4,455 B |
| `/sections/a/topics/b/lessons/c` | 138 / 18,121 | 129 / 17,365 | 4,740
B |

Each route output length and checksum is identical before and after.

## Suspension regression checks

The existing benchmark's streaming rows do not encounter a runtime
boundary, so they cannot validate state parked across host calls. #517
tracks adding a durable suspending case. This PR was additionally
measured with two real `StreamingSession` probes:

| Session path | Before allocs / bytes | After allocs / bytes | Output |
|---|---:|---:|---:|
| Three boundaries with populated scopes | 243 / 25,081 | 243 / 25,081 |
4,940 B |
| VM-owned nested routes delegated to ordinary outlets | 233 / 24,687 |
233 / 24,687 | 2,144 B |

Both paths exactly match the base for allocations, allocated bytes, and
output. The nested-route probe specifically covers the borrowed-to-owned
transition and caught an intermediate recursive clone before the final
move-based implementation restored exact parity.

## Wall time

No speedup is claimed. Separately built release binaries were run in
alternating order, n=9 per build and path, with warm-ups. Every
distribution overlaps, so the defensible result is **no measurable
wall-time regression**.

| Path, wall us/run | Before median [range] | After median [range] |
|---|---:|---:|
| resource string/1000 | 13.02 [12.87-13.76] | 12.85 [12.52-13.15] |
| resource POOLED/1000 | 13.64 [13.56-13.87] | 13.49 [13.08-13.80] |
| deepest ordinary route | 7.300 [7.237-7.391] | 7.179 [7.096-7.350] |
| VM-owned nested streaming route | 11.013 [10.906-11.092] | 11.056
[10.990-11.623] |

RSS is intentionally omitted from the comparison because #516
demonstrates that the current column is multimodal and dominated by
fixture construction. Exact allocator counts are unaffected by that
issue.

## Validation

- `cargo test -p microsoft-webui-handler`: 457 unit and 36 integration
tests passed.
- `cargo xtask check`: full workspace gate passed.
- Added tests for borrowed pointer identity, move-based owned descent,
owned session parking, scope-pool retention across suspension, and the
`ContinuationVm` to ordinary-render nested outlet path.
- Independent performance review found no remaining correctness,
lifetime, or allocation regression.

No public API or behavioral contract changed, so no `DESIGN.md` or
developer-doc update is required.

---

Stacked on #511 and targeting
`mohamedmansour-reduce-handler-render-allocations`. Review #511 first.

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2edb7e98-5706-46bc-b191-51620cd290e5
Copilot-Session: 07ffb414-b15a-4409-a020-b2e406287de0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants