enhance(normalizr): Replace megamorphic dispatch in getDependency#3877
enhance(normalizr): Replace megamorphic dispatch in getDependency#3877
Conversation
…h switch getDependency used delegate[['', ...][path.length]](...path) — a computed property lookup with spread that creates a temporary array on every call and prevents V8 from inlining or type-specializing the dispatch. Replace with an explicit switch on path.length for monomorphic call sites. V8 trace showed getNewEntities deopt changing from "Insufficient type feedback for generic named access" to a different reason after this fix, consistent with removing the megamorphic dispatch from the call path. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
🦋 Changeset detectedLatest commit: 0f9580e The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
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 |
|
Size Change: +15 B (+0.02%) Total Size: 80.6 kB 📦 View Changed
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Benchmark React
Details
| Benchmark suite | Current: 0f9580e | Previous: f5797b4 | Ratio |
|---|---|---|---|
data-client: getlist-100 |
175.44 ops/s (± 3.6%) |
170.95 ops/s (± 4.9%) |
0.97 |
data-client: getlist-500 |
50.51 ops/s (± 4.8%) |
47.17 ops/s (± 9.3%) |
0.93 |
data-client: update-entity |
500 ops/s (± 4.8%) |
444.66 ops/s (± 3.5%) |
0.89 |
data-client: update-user |
488.1 ops/s (± 5.8%) |
416.67 ops/s (± 6.6%) |
0.85 |
data-client: getlist-500-sorted |
52.08 ops/s (± 6.8%) |
52.49 ops/s (± 5.9%) |
1.01 |
data-client: update-entity-sorted |
408.33 ops/s (± 7.4%) |
416.67 ops/s (± 5.0%) |
1.02 |
data-client: update-entity-multi-view |
416.67 ops/s (± 5.4%) |
400 ops/s (± 6.6%) |
0.96 |
data-client: list-detail-switch-10 |
13 ops/s (± 6.2%) |
11.06 ops/s (± 5.9%) |
0.85 |
data-client: update-user-10000 |
104.17 ops/s (± 1.5%) |
94.34 ops/s (± 0.8%) |
0.91 |
data-client: invalidate-and-resolve |
51.55 ops/s (± 2.2%) |
51.55 ops/s (± 3.4%) |
1 |
data-client: unshift-item |
333.33 ops/s (± 3.8%) |
303.03 ops/s (± 4.3%) |
0.91 |
data-client: delete-item |
444.66 ops/s (± 3.5%) |
400 ops/s (± 8.2%) |
0.90 |
data-client: move-item |
263.16 ops/s (± 3.7%) |
202.04 ops/s (± 6.1%) |
0.77 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3877 +/- ##
=======================================
Coverage 98.10% 98.10%
=======================================
Files 153 153
Lines 2899 2902 +3
Branches 564 565 +1
=======================================
+ Hits 2844 2847 +3
Misses 11 11
Partials 44 44 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Benchmark
Details
| Benchmark suite | Current: 0f9580e | Previous: 467a5f6 | Ratio |
|---|---|---|---|
normalizeLong |
453 ops/sec (±0.88%) |
454 ops/sec (±1.16%) |
1.00 |
normalizeLong Values |
411 ops/sec (±0.21%) |
413 ops/sec (±0.34%) |
1.00 |
denormalizeLong |
292 ops/sec (±2.37%) |
291 ops/sec (±2.38%) |
1.00 |
denormalizeLong Values |
269 ops/sec (±1.70%) |
263 ops/sec (±2.52%) |
0.98 |
denormalizeLong donotcache |
997 ops/sec (±0.13%) |
1004 ops/sec (±0.18%) |
1.01 |
denormalizeLong Values donotcache |
753 ops/sec (±0.22%) |
751 ops/sec (±0.14%) |
1.00 |
denormalizeShort donotcache 500x |
1572 ops/sec (±0.12%) |
1599 ops/sec (±0.12%) |
1.02 |
denormalizeShort 500x |
850 ops/sec (±1.92%) |
849 ops/sec (±2.31%) |
1.00 |
denormalizeShort 500x withCache |
6217 ops/sec (±0.11%) |
6326 ops/sec (±0.11%) |
1.02 |
queryShort 500x withCache |
2753 ops/sec (±0.11%) |
2676 ops/sec (±0.44%) |
0.97 |
buildQueryKey All |
55035 ops/sec (±0.33%) |
54520 ops/sec (±0.33%) |
0.99 |
query All withCache |
6350 ops/sec (±0.14%) |
6705 ops/sec (±0.25%) |
1.06 |
denormalizeLong with mixin Entity |
276 ops/sec (±2.31%) |
275 ops/sec (±2.32%) |
1.00 |
denormalizeLong withCache |
7298 ops/sec (±0.12%) |
6826 ops/sec (±0.15%) |
0.94 |
denormalizeLong Values withCache |
5173 ops/sec (±0.36%) |
5043 ops/sec (±0.58%) |
0.97 |
denormalizeLong All withCache |
6215 ops/sec (±0.09%) |
6426 ops/sec (±0.24%) |
1.03 |
denormalizeLong Query-sorted withCache |
6396 ops/sec (±0.09%) |
6738 ops/sec (±0.13%) |
1.05 |
denormalizeLongAndShort withEntityCacheOnly |
1716 ops/sec (±0.21%) |
1677 ops/sec (±0.18%) |
0.98 |
denormalize bidirectional 50 |
5810 ops/sec (±1.82%) |
5868 ops/sec (±1.91%) |
1.01 |
denormalize bidirectional 50 donotcache |
43105 ops/sec (±0.87%) |
41809 ops/sec (±0.77%) |
0.97 |
getResponse |
4684 ops/sec (±0.69%) |
4524 ops/sec (±0.62%) |
0.97 |
getResponse (null) |
10168125 ops/sec (±0.88%) |
10391914 ops/sec (±1.16%) |
1.02 |
getResponse (clear cache) |
266 ops/sec (±2.04%) |
267 ops/sec (±1.99%) |
1.00 |
getSmallResponse |
3323 ops/sec (±0.10%) |
3161 ops/sec (±1.56%) |
0.95 |
getSmallInferredResponse |
2521 ops/sec (±0.15%) |
2443 ops/sec (±0.51%) |
0.97 |
getResponse Collection |
4678 ops/sec (±0.54%) |
4560 ops/sec (±0.38%) |
0.97 |
get Collection |
4647 ops/sec (±0.17%) |
4567 ops/sec (±0.50%) |
0.98 |
get Query-sorted |
5255 ops/sec (±0.39%) |
5219 ops/sec (±0.24%) |
0.99 |
setLong |
455 ops/sec (±0.25%) |
458 ops/sec (±0.30%) |
1.01 |
setLongWithMerge |
258 ops/sec (±0.21%) |
257 ops/sec (±0.25%) |
1.00 |
setLongWithSimpleMerge |
275 ops/sec (±0.15%) |
273 ops/sec (±0.43%) |
0.99 |
setSmallResponse 500x |
935 ops/sec (±0.06%) |
934 ops/sec (±0.21%) |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
Motivation
V8 trace profiling of the React benchmark (
examples/benchmark-reactwithBENCH_V8_TRACE=true) identifiedgetDependencyas using a megamorphic dispatch pattern:This creates a temporary array, performs a computed property lookup, and spreads the arguments on every invocation. V8 cannot inline or type-specialize such a call site — it remains megamorphic (IC miss on every call), preventing optimization of the surrounding hot path in
WeakDependencyMap.get().Solution
Replace the computed dispatch with an explicit
switchonpath.length. Each case directly calls the target method with typed arguments, giving V8 monomorphic call sites it can inline and optimize.Deopt trace evidence: With all V8 optimization fixes applied, the
getNewEntitiesdeopt changed from "Insufficient type feedback for generic named access" to "unexpected name in keyed access" — consistent with the removal of the megamorphic dispatch from that call path. The overall bailout count dropped from 16 → 15, and thegetEntity"Smi" bailout was eliminated.No measurable ops/sec impact in isolation (within noise band), but the change removes a known V8 anti-pattern and is bundle-size neutral.
Open questions
N/A
Made with Cursor
Note
Low Risk
Low risk performance refactor that only changes how delegate methods are dispatched based on
path.length. Behavior should be equivalent but could affect edge cases if unexpectedQueryPathlengths occur.Overview
Improves hot-path dependency lookups by rewriting
getDependencyto use aswitchonpath.lengthand direct method calls instead of computed property + spread invocation.Adds a patch changeset documenting the V8 optimization rationale for
@data-client/normalizr.Reviewed by Cursor Bugbot for commit 0f9580e. Bugbot is set up for automated code reviews on this repo. Configure here.