Performance: Schema Benchmark Fixes, Improvements, & Headless Harness - #92
Merged
Conversation
…t it in the AD4M host
`DataSource` / `ModelClass` / `DatasetHandle` / `RendererDataBindings` were declared but nothing
implemented or checked them. `stores` entered the renderer as `unknown`, so every binding was read
through a hand-written cast (26 of them), and the AD4M host pushed its native types straight through
the hole. Being unenforced, the contract had already drifted: `$useQueryIR` was read by the renderer
but never declared, and `Stores` described `$getModel` as returning AD4M's `ModelClass` — whose
`query` takes a `PerspectiveProxy`, not the neutral shape.
**The dataset handle is now genuinely opaque.** `DatasetHandle` was `{ id, uri }`, which sounds
principled but nothing reads those fields: the renderer obtains a handle, checks it is present, and
hands it back. Its only inspection was `uuid ?? id`, purely to key `getModelForPerspective`. A
structural handle would therefore force every backend to destroy its native handle and reconstruct
it — AD4M flattening a `PerspectiveProxy` to an id and re-resolving it by lookup on every query — to
satisfy fields nobody uses. `$getModelForPerspective` now takes the handle itself and the host
derives its own key, so both peeks are gone along with the `uuid ?? id` TODO.
**The model shape is adapted, because that the renderer does depend on.** `toRendererModel` maps
AD4M's statics onto neutral `query`/`findAll`. It is a pure signature map with no data conversion —
precisely because the handle round-trips untouched. The two decisions pay for each other: adapt what
the renderer must understand, keep opaque what it doesn't.
Also:
- `stores` is typed `RendererStores` throughout; all 26 casts deleted. `Stores` now *extends* it, so
drift surfaces at the host's own declaration rather than at runtime.
- `$agent` resolves through a neutral `$identities` binding instead of reaching for `adamStore`,
removing the last backend reference from the agnostic renderer.
- The AD4M adapter is one artifact: `ad4mCapabilities`, `createAd4mQueryAdapter`, `toRendererModel`
and `createAd4mDataBindings` together, with `TemplateProvider` composing rather than implementing.
Its deps are declared structurally (four accessors), so `shared/` stays framework-agnostic and the
adapter is stubbable — `adamStore` satisfies them and can still be passed whole.
- `$onError` / `$useQueryIR` deliberately stay with the app: any backend wires those the same way.
Landed as one commit because the pieces are atomically coupled — changing the contract without
updating the host does not build, and git cannot split a single file's changes non-interactively.
Known follow-up surfaced by the typing: `@coasys/ad4m` types `findAll` as `(perspective, query?)`
with no third argument, yet the renderer passes an abort-signal options object that both its own
comment and CLAUDE.md describe as forwarded to the executor's cancel machinery. The blanket cast hid
the disagreement. Kept the argument behind a documented widened call — dropping it would silently
disable cancellation — but if the runtime never supported it, the AbortController buys nothing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…TODO The playground's vite.config carried a TODO, since the very first harness commit, claiming esbuild-plugin-solid's dist "breaks scheduled effects downstream" — leaving WE's published artifact documented as suspect for reactivity. That matters for adoption: it is what a consumer without a Solid toolchain gets. It does not reproduce. Checked and ruled out a second Solid runtime in the bundle (all three solid entry points are externalized), malformed JSX output (the dist emits the normal template/insert/effect calls), and a dual-package hazard on REACTIVE_ACCESSOR — a module-local Symbol(), so two copies of schema-shared would silently make every reactive prop read as static, except schema-shared exposes no source condition, so only one instance exists. Confirmed in the browser by aliasing the playground onto dist/index.js; the dist-mode and source-mode bundles hash differently, so the alias was verifiably in effect. Initial paint and live reactivity both worked. The likeliest history is that the real cause was a duplicate solid-js instance, fixed by the dedupe line in that same config, and that the comment blaming the compiler was a misdiagnosis nobody revisited. distReactivity.test.tsx runs the portable-slice feed cases against both the src and dist entry points, so the published artifact can't silently regress. The live-mutation case is the load-bearing one — it exercises the scheduled $query effect, which is what breaks first when two compilers or two runtimes meet. It skips itself when dist/ is absent (gitignored), so a fresh clone isn't failed for the wrong reason. The vite.config comment now records the durable why: dedupe is load-bearing, because two owner graphs stop scheduled effects while the initial paint still looks correct. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The dedupe added in a105164 called scheduleDismiss on every repeat of a still-visible toast, which cleared and restarted its 4s timer. A condition that reports faster than the duration — a reactive query effect re-running per node on every dataset change — therefore reset the countdown indefinitely and pinned the toast on screen until whatever was reporting it unmounted. A repeat still collapses into the live toast, but keeps its original deadline, so one transient failure reads as transient however many times it is reported. scheduleDismiss is now called once per toast; the timers map remains so a manual dismiss can cancel the pending auto-dismiss. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tten updateCustomVars walks a fixed list of ~59 custom properties on every update and calls setProperty for each, whether or not the corresponding design-system prop is set. A bare we-text with no DS props was therefore issuing ~59 removeProperty calls per update, almost all of them clearing properties that had never been written. setProperty now tracks which properties an element has actually written and skips the clear path for the rest. Removing a property that was never set is a no-op by definition, so rendered output cannot change. Measured by ablation on a 3006-element tree: updateAllCustomVars accounted for 83% of the flush phase (~564ms of ~681ms). This recovers 87% of that. headless (4000 nodes) flush 681ms -> 192ms (-72%) browser, Static Extreme flush 128ms -> 93ms (-27%), total -7% browser, Web Components flush 23ms -> 14ms (-42%), total -22% Build is unchanged in both, confirming the change is isolated to the DS prop pipeline. The browser gains less than headless because happy-dom's CSSOM is a JS implementation where a no-op removeProperty is real work; the in-app suite is the figure that counts. Two primitives write a custom property directly that helpers.ts also generates (--we-spinner-color, --we-markdown-gap). Both are covered by tests; this likely fixes a latent ordering bug where helpers.ts could clear a var the component had just set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DesignSystemMixin calls this once per class, but ~20 primitives (button, text, input, badge, checkbox, ...) override getInstanceProps() and call it again on every invocation — once per instance per update. Each uncached call allocated a Set and spread it into a fresh array of up to 82 keys. layerKeyMap is a module constant, so the result is a pure function of the layer set and there are only a handful of distinct combinations across the whole design system. The returned array is shared rather than copied; callers treat it as read-only (filterProps takes readonly string[] and only filters/maps). Kept for correctness rather than speed: measured against a 3006-element tree it made no detectable difference, so this removes redundant work rather than delivering a win. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The shell overlay container is the scroll container for every shell view. Any view whose content crosses the viewport height gains and loses its scrollbar as content changes, and each transition reflows the whole page horizontally. Most visible in the benchmark runner, which navigates between routes of wildly different heights and so jitters continuously, but it affects any shell view with variable-length content. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The suite reported a single number that excluded most of the render. The timer sits last in each route, so its own body cannot run until every preceding sibling has been built — but it started the clock there, which put the entire schema walk outside the measured window. It also stopped at a single rAF, which fires before paint. The visible symptom was Tokens Heavy scoring faster than Tokens Light despite doing strictly more token work. Timing is now four phases derived from raw marks the timer hands back: Build navigation -> timer constructed schema walk, tokens, DOM creation Mount timer constructed -> onMount insertion + custom-element upgrade Flush onMount -> microtask Lit first render + DS prop pipeline Paint microtask -> 2nd rAF style, layout, paint Also: - One queue-driven runner. A single route's Run is a queue of one and Run All a queue of twelve, so both obey the same sampling rules; previously they were separate paths that would have drifted. - Median of 5 warm samples, first discarded. Samples bounce through a new /idle route because navigating to the current path is a no-op, so repeats would otherwise never remount. - Element and custom-element counts, so results normalise to us/element. Total ms alone is not comparable between a 137-element route and an 8015-element one — the previous absolute thresholds left the largest route permanently red while it was in fact the best performer per unit of work. - Per-route baselines for us/element (green +20%, amber +50%), and absolute bands for spread (green <10%, amber <20%). Both calibrated from measured run-to-run variation rather than guessed; earlier guessed thresholds coloured everything amber, which carries no information. - Spread reported as a trimmed range, dropping the slowest sample, so one GC pause doesn't dominate the error bar. - A reactive-update route, since update cost is paid during interaction and is a different question from mount cost. - Heap sampled on /idle before each route, to expose accumulation across a run. - A static progress overlay replaces the per-route status block, which used an animating we-spinner. That spinner was inflating every measurement in proportion to DOM size — removing it cut Static Extreme roughly in half. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Iterating against the in-app suite means edit, rebuild, reload, run twelve
routes three times, read results. That loop is slow enough to encourage
guessing, and guessing already put a 2.5x Build regression into the app before
it was caught.
Renders the same fixtures through two registries and reports both:
stub the schema walk in isolation
real the walk plus what it causes downstream (buildLayoutStyles, Lit
reactive-property setters, the CSSOM writes)
The gap between them is the point, and is why this is a package rather than a
file in @we/schema-solid. Measuring real cost needs the real design system, and
the renderer must not depend on it — it is a thin adapter over an injected
registry, and knowing nothing about the DS is what keeps it portable. Nothing
depends on this package, so it is free to depend on both. A stub-only harness
was tried first and proved structurally blind to the class of change most
renderer optimisations fall into: the regression above measured +6% against
stubs and +160% in the app.
Scope is Build and Flush. happy-dom has no layout engine, so Paint (~30% of
browser total) is unreachable, and its JS CSSOM overstates flush by roughly
2.7x. Treat a result as a filter — a regression means stop, a win is a
hypothesis to confirm in the app on a settled run 3.
Includes an environment guard asserting Lit actually upgrades and the DS prop
pipeline actually writes styles here. Without it, that pipeline silently
failing under happy-dom would collapse flush and read as a spectacular
optimisation.
setProperty.probe.ts covers the write-tracking change in @we/primitives,
including the two names a primitive writes directly that helpers.ts also
generates. It belongs in @we/primitives; that package has no test
infrastructure at all, and this is the only package already wired for
happy-dom plus primitives.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for coasys-we ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
12 tasks
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.
Render measurement, and a fix for design-system custom-property writes
Summary
The schema benchmark suite was measuring the wrong window.
BenchmarkTimersits last in eachroute, so its own body cannot run until every preceding sibling has been built — but it started the
clock at that point, which put the entire schema walk outside the measurement. It then stopped at a
single
requestAnimationFrame, which fires before paint. The visible symptom was Tokens Heavyscoring faster than Tokens Light despite doing strictly more token work. Every number the suite had
produced was unreliable, and in a way that hid exactly the phase most renderer work targets.
This branch rebuilds that measurement, adds a headless harness so renderer changes can be evaluated
in seconds rather than a browser round-trip, and — using the two together — finds and fixes the
single largest cost in the render pipeline:
updateCustomVarsissuing ~59 CSSOM calls perdesign-system element regardless of how many props are actually set, most of them clearing
properties that had never been written.
The fix cuts the flush phase by 27–42% in the browser depending on how custom-element-dense the
route is, with no change to rendered output. It applies to every Lit primitive everywhere in the
app, not only schema-rendered pages.
Honest framing on impact: at realistic template sizes this is around 1% of total render time and
nobody will feel it. It is worth having because it is free — no structural change, no behavioural
trade-off — and because the measurement work that found it is what makes future performance claims
checkable. Several changes attempted along the way were measured, found to do nothing or to make
things worse, and dropped; that filtering is the main deliverable.
Changes
packages/design-system/3-primitives/src/shared/helpers.tssetPropertynow tracks which custom properties an element has actually written (aWeakMapofSet<string>) and skipsremovePropertyfor ones it never wrote.updateCustomVarswalks a fixed list of ~59 properties on every update and callssetPropertyforeach, whether or not the corresponding design-system prop is set. A bare
we-textwas thereforeissuing ~59
removePropertycalls per update, almost all clearing properties that had neverexisted. Removing a property that was never set is a no-op by definition, so rendered output cannot
change.
Two primitives write a custom property directly that
helpers.tsalso generates(
--we-spinner-color,--we-markdown-gap). Both are covered by new tests. This most likely fixesa latent ordering bug, where
helpers.tscould clear a variable the component had just set.packages/design-system/utils/src/index.tsgetKeysForLayersis memoised.DesignSystemMixincalls it once per class, but ~20 primitivesoverride
getInstanceProps()and call it again per instance per update, each time allocating a Setand spreading it into a fresh array of up to 82 keys.
Kept for correctness, not speed — it measured no detectable difference. Included so the redundancy
is gone and so the commit message records that it isn't a performance win, rather than leaving a
plausible-looking optimisation for someone to cite later.
packages/app-framework/src/frameworks/solid/components/BenchmarkTimer.tsxReduced to a probe that stamps four raw timestamps and hands them back;
testStorederives thedurations. It deliberately does not know when navigation started, because it cannot — that boundary
is what makes the schema walk measurable at all.
Two timing corrections: a
queueMicrotaskstep isolates Lit's async first render from browserpaint, and a second
rAFis used because a single one fires before paint and excluded it entirely.packages/app-framework/src/shared/schemas/shell/tests/testStore.tsOne queue-driven runner replaces two separate paths. A single route's Run is a queue of one and Run
All a queue of twelve, so both obey the same sampling rules — previously they were independent code
paths that would have drifted.
Sampling is median-of-5 with the first sample discarded. Samples bounce through a new
/idleroutebecause navigating to the path you are already on is a no-op, so repeats would never remount.
Element and custom-element counts are captured so results normalise to µs/element, and JS heap is
sampled on
/idlebefore each route to expose accumulation across a run.Spread is reported as a trimmed range — the slowest sample dropped — because plain min–max over
five samples is dominated by a single GC pause. Measured across three consecutive runs, Static Small
reported 6% / 25% / 45% spread while its median moved less than 5%.
packages/app-framework/src/shared/schemas/shell/tests/SchemaBenchmark.schema.tsResults are shown per phase (Build / Mount / Flush / Paint) with each phase's share of total, plus
element counts, heap and spread.
Colour coding is calibrated from measurement rather than guessed:
because fixed overhead dominates small routes (~165µs/el at 139 elements) while large ones
amortise it (64µs/el at 8015), so no global threshold can work. The +20% green band is sized to
absorb the ~10–14% cross-session drift that a reboot introduces.
answers "can I trust this median", which is an absolute question, and its own run-to-run variance
exceeds the quantity itself (Static Large has measured 7–26%), so a baseline would encode a
lottery result.
Two earlier threshold schemes are recorded in comments as failures: absolute-ms left the largest
route permanently red while it was in fact the best performer per unit of work, and the first
µs/element attempt left nearly everything amber. A threshold that is always red carries no
information.
Also adds a reactive-update route (update cost is paid during interaction and is a different
question from mount cost), and replaces the per-route status block with a static progress overlay.
That block used an animating
we-spinner, which was inflating every measurement in proportion toDOM size — removing it cut Static Extreme roughly in half.
packages/app-framework/src/frameworks/solid/layouts/TemplateLayout.tsxscrollbarGutter="stable"on the shell overlay's scroll container. Any shell view whose contentcrosses the viewport height gains and loses its scrollbar as content changes, and each transition
reflows the page horizontally. Most visible in the benchmark runner, but it affects any shell view
with variable-length content — this is the one change here that touches non-benchmark UI.
packages/schema-system/benchmarks/(new package, private)A headless harness that renders identical fixtures through two registries — stub components
(the schema walk in isolation) and the real design system (the walk plus everything it causes
downstream) — and reports both. The gap between them is the point.
It is a package rather than a file inside
@we/schema-solidbecause measuring real cost needs thereal design system, and the renderer must not depend on it: it is a thin adapter over an injected
registry, and knowing nothing about the DS is what keeps it portable. Nothing depends on this
package, so it is free to depend on both. Adding the DS as devDependencies of
@we/schema-solidwastried and reverted for that reason.
A stub-only harness was tried first and proved structurally blind to the class of change most
renderer optimisations fall into — one change measured +6% against stubs and +160% in the real app,
because the cost lived entirely in what the per-prop effects then did.
environment.probe.tsasserts that Lit genuinely upgrades under happy-dom and that the DS proppipeline actually writes styles. Without that guard, the pipeline silently failing would collapse
flush and read as a spectacular optimisation.
setProperty.probe.tscovers the write-tracking change, including both direct-write collisions. Itbelongs in
@we/primitives; that package has no test infrastructure at all, and this is the onlypackage already wired for happy-dom plus primitives.
Results
Browser, comparing settled run 3 before and after, on the same machine after a reboot:
$eachBuild is unchanged across all routes, confirming the change is isolated to the design-system prop
pipeline. Gains scale with custom-element density, which is the expected shape.
Known follow-ups
@we/primitiveshas no test infrastructure. The six behaviour tests covering this change sitin
@we/schema-benchwith a note saying where they belong. Deferred to a broader tests PR.read red without anything having regressed; the fix is to re-record, not to assume a regression.
Noted in the source comment.
number that looks off for Solid. Unlike mount it is paid during interaction, where 33ms is two
dropped frames — the first place to look if the app ever feels laggy.
gave −27%. It gets direction and phase right, magnitude wrong by roughly 2.7×, because happy-dom's
CSSOM is a JS implementation. Use it as a filter, never as a prediction.
divper schema node was investigated and rejected. It doubles DOM element count(8015 elements for 4000 schema nodes), but the wrappers are
display: contentsand so generate nolayout box — ablation confirmed Build −17% with Paint and Flush unmoved, which is ~3–6% of total
at realistic scale. Not worth the structural risk or the remount-on-entering-visual-edit-mode
behaviour it would force.
Test plan
Verified:
pnpm --filter @we/schema-solid test— 38 passpnpm --filter @we/schema-shared test— 448 passpnpm --filter @we/schema-bench probe— 11 pass, including both direct-write collisions andthe happy-dom capability guard
pnpm --filter @we/schema-shared validate— 14 schemas, no issuestsc --noEmitclean for@we/app-framework,@we/schema-solidand the new packagethe fix; figures above are from settled run 3 in each case
updateAllCustomVarsdrops flush to ~117ms(the floor), and the shipped fix reaches ~192ms — 87% of the available win
scrollbarGutterchange — itapplies to all of them, and only the benchmark view was exercised
we-spinnerandwe-markdownin the running app; both are covered by headlesstests but were not visually confirmed