Faster server rendering: parse each module once per build, and trim per-render JSON and parsing work - #5
Merged
Conversation
Owner
|
@lahma this looks awesome, I'll try get it in over the next day or so! |
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.
Server rendering does the same work more often than it needs to: every pooled engine re-reads
and re-parses the whole module graph from disk, every render parses two small throwaway scripts,
the model and the context are each serialised twice per response, and the rendered markup is
JSON-escaped inside the engine only to be unescaped again on the way out. This PR removes each
of those, keeps two behaviours a pooled engine should have but didn't (a clean global surface
between renders, a timeout that actually bounds the render), and updates Jint to 4.15.3, which
is what makes the pooling work possible.
Each commit stands alone and keeps the suite green, so any of them can be dropped or cherry-picked
if you'd rather take a subset.
What changed
4.15.2's async/bound-function fixes.
prepared module ASTs (thread-safe and shareable per Jint's documentation) shared by every
engine in the pool. The npm CommonJS→ESM transform now also runs once per build instead of
once per engine. Invalidation rides the existing build id, so a rebuild starts fresh exactly
as it does today.
process; the per-render
({})andJSON.parse(...)evaluations are replaced with directobject construction and Jint's own JSON parser (the same parser
JSON.parseuses, minus ascript parse, an engine entry and two global writes per render).
used to serialise the same model and context independently; now both consume one pair of
strings. In
Server-only mode the second serialisation was pure waste.renderViewhands the markupback as the string it already is; only the small head descriptor still travels as JSON. A
whole page no longer gets JSON-escaped and unescaped per render.
bridge cleanup with a snapshot restore, which also removes anything a view left on
globalThis(previously it stayed in the pooled engine indefinitely) and fences stray asynccontinuations from settling into a later request's render.
engine re-arms its built-in timeout at every entry from the host, and one render enters the
engine several times, so
ServerRendering.Timeoutreally bounded each entry rather than therender; a render could legitimately consume several multiples of it. It is now one budget for
the whole render, and
HttpContext.RequestAbortednow stops running JavaScript instead ofonly cancelling the wait for a pool slot. Behaviour change: a view that only fit inside
the old per-entry budgets can now time out —
ServerRendering.Timeoutis the knob.internalonpurpose; see the question below.
Measurements
BenchmarkDotNet 0.15.8, default job,
[MemoryDiagnoser], .NET 10.0.10, Windows 11, serial runson an idle machine. The harness builds a real view project with the real TypeScript compiler and
renders through
JsxServerRenderer— five rows: a minimal view and a 20-record dashboard view ona warm engine, the head-only path, a cold start (
Reset()+ first render, i.e. one engine builtfrom nothing), and a pool-growth burst (
Reset()+ four concurrent renders, i.e. how the poolfills under load).
Preact (baseline
d128b52→ this PR)React (baseline → this PR)
Reading the cold-start regression honestly
The parse-once cache prepares a module (Jint's
PrepareModule, which runs a static-analysispass) where the engine used to just parse it. For a single engine built alone there is
nobody to share with, so that first build pays the analysis and gets nothing back: +17%
Preact, +20% React on the ColdStart row. Every further engine built for the same compilation
skips parsing entirely — the burst row is four engines, and allocation halves. Where each case
lands in practice:
pool-fill after that is the burst case. Net win.
per edit — ~0.9 ms for Preact, ~20 ms for React, alongside a recompile that costs tens of
milliseconds anyway. Subsequent pool growth for that build shares the one parse.
Steady-state rows are unaffected by the cache either way (a warm engine keeps its module graph,
as it always did).
Two per-commit notes for completeness: commit 4's row-level effect is a couple of microseconds
of extra work at the
JsxServerRendererAPI (two small JSON parses instead of one combined) —its saving is at the response level, where the document writer no longer re-serialises the model
and context; and the clean-globals restore plus the whole-render deadline both measured within
noise of free on every row.
A question on commit 8 (
ImmutableCrossingTypes)Jint 4.15.3 lets a host declare CLR types whose instances are immutable while exposed; declared
types get their member reads memoised, which pays off for registered globals whose methods hand
back records a view walks repeatedly. The wiring and tests are in this PR, but the option is
internal: it is a promise (a wrong declaration is answered with stale reads, not an error),so whether to expose it publicly — and under what name and shape — feels like your call rather
than mine. Happy to make it public in this PR, move it to a follow-up, or drop the commit
entirely.
Testing
NpmBootstrapTests.ResolveProjectDirectory_NoManifestAnywhere_FallsBackToTheContentRoot, isenvironmental — a stray
package.jsonin my user profile that the manifest walk-up finds —and fails identically on the base commit).
globalThisis gone by the next render; a never-returning view isended by the configured timeout (and the exception says which budget fired); a request aborted
mid-render stops running JavaScript; immutable-crossing declarations cut member re-resolution
(asserted by counting CLR getter invocations, 2 → 1).