perf: SSR rendering optimizations#15605
Conversation
Add benchmarks isolating specific rendering hot paths: - many-components: markHTMLString/isHTMLString/validateComponentProps - many-expressions: renderChild dispatch ordering, escapeHTML - many-head-elements: head deduplication - many-slots: eager slot prerendering - large-array: BufferedRenderer per array child - static-heavy: baseline for static HTML overhead
🦋 Changeset detectedLatest commit: 4050a6d The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Merging this PR will improve performance by 72.83%
Performance Changes
Comparing Footnotes |
c7d98f3 to
7325c51
Compare
16c98bd to
b529461
Compare
b529461 to
99e4c80
Compare
| if (elements.length <= 1) return elements; | ||
| const seen = new Set<string>(); | ||
| return elements.filter((item) => { | ||
| const key = JSON.stringify(item.props) + item.children; |
There was a problem hiding this comment.
Not that this was added in this PR so it could be changed in another one, but using JSON.stringify seems dangerous here. Two elements with exactly the same properties but in a different order should not be deduplicated?
> a = JSON.stringify({foo: 1, bar: 2})
'{"foo":1,"bar":2}'
> b = JSON.stringify({bar: 2, foo: 1})
'{"bar":2,"foo":1}'I don't think we are normalizing the head elements to have a consistent property ordering.
We already have a dependency on deterministic-object-hash for that, so it should be just a function swap.
There was a problem hiding this comment.
I'll sort the keys. It's not a particularly hot path, but deterministic-object-hash is still probably overkill.
There was a problem hiding this comment.
deterministic-object-hash
I found out that this package uses node.js APIs, so we can't use it anyway :/ I believe uses node:crypto, last time I checked
| flushers[j] = createBufferedRenderer(destination, (bufferDestination) => { | ||
| return renderChild(bufferDestination, children[i + 1 + j]); | ||
| }); |
There was a problem hiding this comment.
If we add a detection logic for the types of child that are known to never return a promise we could avoid the allocations for them even when there is a Promise before them. Since the most common child (a plain string) is one such case this might make for a big improvement for pages that do have async parts.
(also could be a separate PR)
There was a problem hiding this comment.
We will be able to do that with queued rendering, which enabled batching
| Refactors the internal rendering pipeline to avoid unnecessary object allocations and reduce overhead in hot paths: | ||
|
|
||
| - Replaces `Object.prototype.toString.call()` with `instanceof` for HTML string detection | ||
| - Reorders the `renderChild` type dispatch to check strings first (most common case) | ||
| - Eliminates O(N²) head element deduplication by using a `Set` | ||
| - Renders array children and template expressions directly to the destination without buffering when all children are synchronous, falling back to buffered rendering only when a `Promise` is encountered |
There was a problem hiding this comment.
Not sure this is relevant to our end-users. If we want to talk about performance, perhaps we should say where users will these benefits: e.g. pages with more than 50 components, pages with many strings, etc. Essentially, we should give them something that they can quantify
| if (flusher) { | ||
| const result = flusher.flush(); | ||
| return result.then(() => { | ||
| let k = 0; |
There was a problem hiding this comment.
Why is k defined outside the closure? can't we define it before the while loop, inside the closure?
There was a problem hiding this comment.
It's in the closure, but needs to be outside of iterate so that it still increments when iterate calls itself
| // Strings are the most common child type (text expressions like {title}, {name}) | ||
| // so check them first for the fastest dispatch in the common case. | ||
| if (typeof child === 'string') { | ||
| destination.write(markHTMLString(escapeHTML(child))); | ||
| return; | ||
| } |
There was a problem hiding this comment.
oh yeah, this was a perf suggestion that the AI agent suggested while I worked on the queued rendering. Short-circuit to simple node types first
There was a problem hiding this comment.
yeah, these sort of ones are so obvious once you see them! Cheap/common checks first.
| flushers[j] = createBufferedRenderer(destination, (bufferDestination) => { | ||
| return renderChild(bufferDestination, children[i + 1 + j]); | ||
| }); |
There was a problem hiding this comment.
We will be able to do that with queued rendering, which enabled batching
Changes
Object.prototype.toString.call()withinstanceofforisHTMLStringdetectionrenderChildtype dispatch to check strings first (most common child type)SetBufferedRendereronly when aPromiseis encounteredrendering-perf.bench.js) covering many-components, many-expressions, many-head-elements, many-slots, large-array, and static-heavy page shapesCodspeed results: all benchmarks improve or stay flat, no regressions on
.mdor.mdxroutes. The.astropage benchmark (10K-item list) doubles in throughput.Research done with help from Claude. Benchmarks by Claude.
Testing
New benchmarks in
benchmark/bench/rendering-perf.bench.jsand existingbenchmark/bench/render.bench.js.Results from local tests, compared to main:
rendering-perf.bench.js (non-streaming)
render.bench.js (realistic pages)
Docs
Internal optimizations only — no public API or behavior changes. No docs needed.