perf(solid-router): proxy-free link props in the spread hot path#7609
perf(solid-router): proxy-free link props in the spread hot path#7609brenelz wants to merge 1 commit into
Conversation
useLinkProps stacked four proxies (merge of defaults, two omit proxies, and a final merge with the resolved-props memo), and Solid's spread() re-enumerated them through V8 proxy traps on every navigation for every Link. Return a plain object with a stable key set instead, with reactivity in property getters backed by fine-grained memos, and add href-based equality to the built-location memo. Client-side navigation benchmark: 7.06ms -> 4.80ms per iteration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit e3e4aaf
☁️ Nx Cloud last updated this comment at |
Merging this PR will improve performance by 17.32%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | ssr request loop (vue) |
420.3 ms | 481.2 ms | -12.67% |
| ⚡ | client-side navigation loop (solid) |
72.6 ms | 44.7 ms | +62.36% |
| ⚡ | ssr request loop (solid) |
174.6 ms | 153.3 ms | +13.89% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing brenelz:perf/solid-router-proxy-free-link-props (e3e4aaf) with solid-router-v2-pre (67a9040)2
Footnotes
-
1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩
-
No successful run was found on
solid-router-v2-pre(65133ed) during the generation of this report, so 67a9040 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
2 participants
Problem
Comparing the CodSpeed flamegraphs for the client-side navigation benchmarks showed Solid ~18% slower than React (66.2 ms vs 55.9 ms), but with an odd shape: Solid's attributable JS time was actually lower than React's. The entire gap sat in an unattributed
NodeJS internalsbucket — 18.3 ms (27.7%) for Solid vs 2.5 ms (4.5%) for React.The cause:
useLinkPropslayered four proxies —merge()for the activeProps/inactiveProps defaults, two stackedsplitProps/omitproxies, and a finalmerge(propsSafeToSpread, resolvedProps). Solid'sspread()re-enumerates the returned object on every update, so every navigation walkedownKeys/getOwnPropertyDescriptor/gettraps through all four layers, for every<Link>on the page. V8 dispatches proxy traps in C++ runtime code, which profilers can't attribute to JS frames — hence the opaque "internals" cost.Change
useLinkPropsnow returns a plain object with a stable key set. Reactivity lives in property getters backed by fine-grained memos (href,data-status,aria-current,data-transitioning,class,style,disabled/role/aria-disabled). Values that no longer apply resolve toundefined, whichspread()/assign()treats as attribute removal — DOM output is unchanged.activeProps/inactivePropsare resolved through accessors at the use sites instead of amerge()proxy; the two stackedomitproxies became a single one-time descriptor copy.next) gained href-based output equality, so downstream memos skip recompute when a navigation doesn't change a link's target.Results
Local client-nav benchmark (same machine,
NODE_ENV=production):In the local CPU profile the spread-effect hotspot dropped from 14.3% cumulative to 1.0% and proxy enumeration (
ownKeys/keys) left the hotspot list entirely. Bundle impact: +0.2 kB gzipped.activeProps/inactivePropsclasses,data-status/aria-current, and inherited-search hrefs all update correctly across navigations.Behavioral note
Keys returned by
activeProps/inactivePropsfunctions are discovered once at setup; a function that later returns brand-new keys (beyond the initial set plusclass/style) won't have those keys applied. Nothing in the test suite relies on that pattern, but it's the one semantic trade for keeping the spread target's key set stable.🤖 Generated with Claude Code