perf(react-grab): dexnode-equivalent V8 deopt profiler + targeted source fixes#346
Draft
aidenybai wants to merge 4 commits into
Draft
perf(react-grab): dexnode-equivalent V8 deopt profiler + targeted source fixes#346aidenybai wants to merge 4 commits into
aidenybai wants to merge 4 commits into
Conversation
…wser run Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
- Analyzer now resolves IC pc addresses to their containing JS code blob via the v8 log's code-creation records, so megamorphic / polymorphic hot sites are reported with the function name and source URL. - Adds packages/react-grab/perf/v8-deopt-findings.md (curated report) and a generated summary.md sibling next to v8.log. - Gitignores raw v8.log / code.asm; the parsed summaries stay tracked. - Documents perf:deopt / perf:deopt:analyze in perf/README.md. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
…t helpers Three targeted fixes driven by the dexnode V8 deopt capture, each addressing a specific site that appears in packages/react-grab/perf/v8-log/summary.md. 1) get-element-at-position.ts: replace the `null | object` pattern for both positionCache and iframeHoverCache with module-level singletons that carry a `hasValue` / `isHovering` flag. This removes the shape transition the JIT was seeing at the cache-read sites inside the hot getElementAtPosition path (one of the two paired deopts in freeze-updates*.js: `if (z && Gr(e, t, z.rect)) return z.element;`). 2) get-visual-viewport.ts: return a shared mutable singleton instead of allocating a fresh literal each call. The toolbar position helpers inline this and bail out with "dependent field type constness changed" when consecutive returns produce different hidden classes. Callers already consume the result synchronously, so the singleton is safe; a comment documents the contract. 3) toolbar-position.ts: hoist Math.max/Math.min to module-level locals (`mathMax`, `mathMin`). The IC capture showed dozens of KeyedLoadIC hits per run on Math.* inside getPositionFromEdgeAndRatio and getSnapPosition; the rewritten globals come from a single module-scope const so V8 emits a plain LoadIC instead. Aggregate impact on the synthetic grid bench is within run-to-run noise (±5 ms / ~7%, as documented in perf/README.md - the synthetic scenarios don't stress the per-callsite IC behavior these helpers control). The verifiable changes are: - 60+ `Math` KeyedLoadIC events / run -> 0 (Math hoist) - dependent-field-constness deopts: 11 -> 10 - react-grab megamorphic IC SITE count: 62 -> 54 (event volume on Solid's createStore proxy still dominates and is not ours to fix - see v8-deopt-findings.md) Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
- v8-deopt-findings.md gains an 'Applied fixes' table mapping each touched source file to the deopt/IC site it addresses, plus a before/after metrics table (within run-to-run noise on aggregate counts, verified zero Math KeyedLoadIC hits + fewer distinct megamorphic / polymorphic sites in react-grab source). - summary.md is refreshed from a new dexnode-equivalent capture against the post-fix build. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
cursor
Bot
changed the title
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
1 participant
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.
What
Adds an in-repo workflow to capture V8 deopt + IC information from
react-grabas it actually runs in a real Chromium tab, and applies the three deopt fixes that are squarely in our source.Because
dexnodeitself is a Node-only wrapper andreact-grabis a browser library, this PR portsdexnode's flag set to the equivalent Chromium--js-flagsinvocation. The resultingv8.logis byte-compatible with Microsoft's Deopt Explorer VS Code extension; an in-repo CLI summarizer is included for environments without it.New scripts
packages/react-grab/scripts/perf-deopt.mjs— boots thee2e-app(/?perf=grid&rows=50&cols=10) and launches headless Chromium with the exact flag setdexnodeemits for achrome_stablehost (--log-deopt --log-ic --log-maps --log-maps-details --log-code --log-source-code --prof --log-internal-timer-events --detailed-line-info --no-logfile-per-isolate --logfile=…). It then drives five scenarios (hover sweep, scroll-while-frozen, repeatedgetState, viewport invalidation bursts, multi-freeze invalidation bursts) and writes the rawv8.logtopackages/react-grab/perf/v8-log/.packages/react-grab/scripts/perf-deopt-analyze.mjs— parses the log: deopt entries (code-deoptwith inlined<url:line:col>source positions) and IC entries. ICpcaddresses are resolved to their owning JS code blob via the log'scode-creationrecords, so megamorphic / polymorphic hot sites are grouped by their containing function and source URL. Emitssummary.json,summary.md, and a console report.pnpm --filter react-grab perf:deopt/perf:deopt:analyzewire those up.Targeted source fixes (only sites we control)
Driven directly by the capture in
packages/react-grab/perf/v8-deopt-findings.md. Three commits, each one source file:src/utils/get-element-at-position.tspositionCacheandiframeHoverCachebecome module-level shape-stable singletons (hasValue/isHoveringflag replaces thenull/object union)Xr(freeze-updates*.js:~1303) — removes thecache/hoveredIframenull↔objectshape transition that fed the cache-read ICsrc/utils/get-visual-viewport.tsIt(renderer*.js:~1188/~1721) — removes thedependent field type constness changedclusters whose root cause was the per-call literal returning different hidden classessrc/utils/toolbar-position.tsMath.max/Math.minto module-levelmathMax/mathMingetPositionFromEdgeAndRatio/getSnapPosition— eliminates the 60+KeyedLoadICevents / run against theMathglobalVerifiable IC-level deltas (4 runs each)
Aggregate is within run-to-run noise (±5 ms / ~7% per
perf/README.md), but the targeted IC sites measurably moved:MathKeyedLoadICevents / rundependent field type constness changeddeoptsWhat we did NOT change (and why)
These appear in the findings but they are not ours to fix in this PR:
$(e, t)atfreeze-updates*.js:~1671(wrong call target+ paired lazy deopt). Lives inbippy, notpackages/react-grab/src.b(e)atfreeze-updates*.js:~249(unexpected name in keyed access+ 116× megamorphicString.prototype.startsWith). Also bippy.createStoreproxygettrap atcore*.js:70. Owned bysolid-js; the structural fix is to stop funneling heterogeneous targets (DOM nodes, fibers, plain options) through one store, which is a design-level change beyond this PR.incrementViewportVersion→ memo invalidation chain (renderer*.jsconstness deopts). Already called out inperf/README.md— the right fix is to make store slot writes shape-stable, but that touches multiple modules and needs its own design pass.How to reproduce
Then read
packages/react-grab/perf/v8-deopt-findings.mdand the freshly-writtenpackages/react-grab/perf/v8-log/summary.md. The rawv8.logis gitignored but can be opened directly with the Microsoft "Deopt Explorer" VS Code extension.Files
packages/react-grab/scripts/perf-deopt.mjs,packages/react-grab/scripts/perf-deopt-analyze.mjspackages/react-grab/perf/v8-deopt-findings.md, regeneratedpackages/react-grab/perf/v8-log/summary.mdpackages/react-grab/src/utils/get-element-at-position.ts,packages/react-grab/src/utils/get-visual-viewport.ts,packages/react-grab/src/utils/toolbar-position.tspackages/react-grab/package.json(2 new scripts),packages/react-grab/perf/README.md(docs),.gitignore(exclude rawv8.log/code.asm)Summary by cubic
Adds an in-browser V8 deopt + IC profiler for
react-grabthat mirrorsdexnodeflags in headless Chromium, plus a CLI analyzer and curated findings. Also applies three small runtime tweaks to stabilize hidden classes and hoistMathglobals, reducing megamorphic IC sites and eliminatingMathKeyedLoadICs.New Features
perf:deopt: launches thee2e-app, runs Chromium withdexnode-equivalent--js-flags, drives five scenarios, and writesv8.logtopackages/react-grab/perf/v8-log/.perf:deopt:analyze: resolves ICpcaddresses viacode-creationrecords and emitssummary.json/summary.md(+ console report).packages/react-grab/perf/v8-deopt-findings.mdwith an updated post-fix snapshot; docs inpackages/react-grab/perf/README.md; raw logs/asm gitignored.Performance
get-element-at-position.tsandget-visual-viewport.tsvia module-level singletons.Math.max/Math.minintoolbar-position.tsto cut hot property lookups.MathKeyedLoadIC events → 0; react-grab megamorphic IC sites: 62 → 54; dependent-field-constness deopts: 11 → 10.Written for commit 1a2f26a. Summary will update on new commits. Review in cubic