enhance(normalizr): Avoid hidden class mutation in normalize() result#3878
enhance(normalizr): Avoid hidden class mutation in normalize() result#3878
Conversation
The normalize() return object was constructed with result: '' as any, then mutated via ret.result = visit(...). This causes a V8 hidden class transition when the property type changes from string to the actual result type (array/object/string), triggering "field type constness changed" invalidations that deoptimize code depending on this object shape. Restructured to compute the result first and construct the final NormalizedSchema in a single step, keeping the object shape stable from creation. Made-with: Cursor
🦋 Changeset detectedLatest commit: 770c6c0 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Size Change: +8 B (+0.01%) Total Size: 80.6 kB 📦 View Changed
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Benchmark React
Details
| Benchmark suite | Current: 770c6c0 | Previous: f5797b4 | Ratio |
|---|---|---|---|
data-client: getlist-100 |
166.67 ops/s (± 5.6%) |
170.95 ops/s (± 4.9%) |
1.03 |
data-client: getlist-500 |
45.25 ops/s (± 6.4%) |
47.17 ops/s (± 9.3%) |
1.04 |
data-client: update-entity |
454.55 ops/s (± 3.9%) |
444.66 ops/s (± 3.5%) |
0.98 |
data-client: update-user |
350.99 ops/s (± 4.1%) |
416.67 ops/s (± 6.6%) |
1.19 |
data-client: getlist-500-sorted |
46.73 ops/s (± 2.6%) |
52.49 ops/s (± 5.9%) |
1.12 |
data-client: update-entity-sorted |
312.5 ops/s (± 6.6%) |
416.67 ops/s (± 5.0%) |
1.33 |
data-client: update-entity-multi-view |
350.99 ops/s (± 8.4%) |
400 ops/s (± 6.6%) |
1.14 |
data-client: list-detail-switch-10 |
11.4 ops/s (± 7.9%) |
11.06 ops/s (± 5.9%) |
0.97 |
data-client: update-user-10000 |
90.91 ops/s (± 3.3%) |
94.34 ops/s (± 0.8%) |
1.04 |
data-client: invalidate-and-resolve |
50 ops/s (± 0.7%) |
51.55 ops/s (± 3.4%) |
1.03 |
data-client: unshift-item |
294.12 ops/s (± 3.8%) |
303.03 ops/s (± 4.3%) |
1.03 |
data-client: delete-item |
434.78 ops/s (± 0.0%) |
400 ops/s (± 8.2%) |
0.92 |
data-client: move-item |
235.33 ops/s (± 2.8%) |
202.04 ops/s (± 6.1%) |
0.86 |
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 #3878 +/- ##
==========================================
- Coverage 98.10% 98.10% -0.01%
==========================================
Files 153 153
Lines 2899 2898 -1
Branches 564 564
==========================================
- Hits 2844 2843 -1
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: 770c6c0 | Previous: 467a5f6 | Ratio |
|---|---|---|---|
normalizeLong |
455 ops/sec (±1.67%) |
454 ops/sec (±1.16%) |
1.00 |
normalizeLong Values |
420 ops/sec (±0.23%) |
413 ops/sec (±0.34%) |
0.98 |
denormalizeLong |
289 ops/sec (±2.59%) |
291 ops/sec (±2.38%) |
1.01 |
denormalizeLong Values |
264 ops/sec (±2.15%) |
263 ops/sec (±2.52%) |
1.00 |
denormalizeLong donotcache |
1037 ops/sec (±0.12%) |
1004 ops/sec (±0.18%) |
0.97 |
denormalizeLong Values donotcache |
757 ops/sec (±0.15%) |
751 ops/sec (±0.14%) |
0.99 |
denormalizeShort donotcache 500x |
1409 ops/sec (±0.73%) |
1599 ops/sec (±0.12%) |
1.13 |
denormalizeShort 500x |
842 ops/sec (±2.04%) |
849 ops/sec (±2.31%) |
1.01 |
denormalizeShort 500x withCache |
6310 ops/sec (±0.11%) |
6326 ops/sec (±0.11%) |
1.00 |
queryShort 500x withCache |
2693 ops/sec (±0.14%) |
2676 ops/sec (±0.44%) |
0.99 |
buildQueryKey All |
55202 ops/sec (±1.10%) |
54520 ops/sec (±0.33%) |
0.99 |
query All withCache |
6378 ops/sec (±0.20%) |
6705 ops/sec (±0.25%) |
1.05 |
denormalizeLong with mixin Entity |
278 ops/sec (±1.85%) |
275 ops/sec (±2.32%) |
0.99 |
denormalizeLong withCache |
7203 ops/sec (±0.17%) |
6826 ops/sec (±0.15%) |
0.95 |
denormalizeLong Values withCache |
4739 ops/sec (±1.27%) |
5043 ops/sec (±0.58%) |
1.06 |
denormalizeLong All withCache |
6069 ops/sec (±0.18%) |
6426 ops/sec (±0.24%) |
1.06 |
denormalizeLong Query-sorted withCache |
6358 ops/sec (±0.20%) |
6738 ops/sec (±0.13%) |
1.06 |
denormalizeLongAndShort withEntityCacheOnly |
1747 ops/sec (±0.18%) |
1677 ops/sec (±0.18%) |
0.96 |
denormalize bidirectional 50 |
5543 ops/sec (±2.25%) |
5868 ops/sec (±1.91%) |
1.06 |
denormalize bidirectional 50 donotcache |
40285 ops/sec (±0.57%) |
41809 ops/sec (±0.77%) |
1.04 |
getResponse |
4610 ops/sec (±1.34%) |
4524 ops/sec (±0.62%) |
0.98 |
getResponse (null) |
9694613 ops/sec (±0.97%) |
10391914 ops/sec (±1.16%) |
1.07 |
getResponse (clear cache) |
264 ops/sec (±2.04%) |
267 ops/sec (±1.99%) |
1.01 |
getSmallResponse |
3351 ops/sec (±0.20%) |
3161 ops/sec (±1.56%) |
0.94 |
getSmallInferredResponse |
2497 ops/sec (±0.28%) |
2443 ops/sec (±0.51%) |
0.98 |
getResponse Collection |
4585 ops/sec (±0.46%) |
4560 ops/sec (±0.38%) |
0.99 |
get Collection |
4561 ops/sec (±0.25%) |
4567 ops/sec (±0.50%) |
1.00 |
get Query-sorted |
5305 ops/sec (±0.14%) |
5219 ops/sec (±0.24%) |
0.98 |
setLong |
458 ops/sec (±0.18%) |
458 ops/sec (±0.30%) |
1 |
setLongWithMerge |
259 ops/sec (±0.20%) |
257 ops/sec (±0.25%) |
0.99 |
setLongWithSimpleMerge |
276 ops/sec (±0.14%) |
273 ops/sec (±0.43%) |
0.99 |
setSmallResponse 500x |
919 ops/sec (±0.78%) |
934 ops/sec (±0.21%) |
1.02 |
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) showed multiple "dependent field type constness changed" invalidations. One source was thenormalize()return object:V8 tracks property types as part of an object's hidden class. When
ret.resultis initialized as''(string) then overwritten with the actual result (which can be an array, object, or string), V8 observes a "field type constness changed" transition. This invalidates any optimized code that depends on the original hidden class shape, forcing recompilation.Solution
Separate the mutable
state(whichNormalizeDelegatewrites to during traversal) from the final return object. Theresultis computed first viavisit(), then theNormalizedSchemais constructed in a single object literal with the final value — no mutation, no hidden class transition.NormalizeDelegatenever accessesresult, onlyentities/indexes/entitiesMeta, so the separation is safe.Deopt trace evidence: The V8 trace showed 4 "field type constness changed" invalidations before the fix. After applying all V8 fixes, those 4 remain — but they're now attributable solely to Entity in-place field mutations (a separate architectural pattern), confirming this fix removed its source of the invalidation.
No measurable ops/sec impact in isolation (within noise band), but eliminates a known V8 hidden class anti-pattern with zero bundle size cost.
Open questions
N/A
Made with Cursor
Note
Low Risk
Low risk performance refactor confined to
normalize()return construction; behavior should be unchanged but touches a core utility used widely, so any subtle typing/shape assumptions could surface.Overview
Refactors
normalize()to avoid mutating the returnedNormalizedSchemaobject after creation (computesresultfirst, then returns a single object literal).The mutable normalization store is now kept in a separate
stateobject passed toNormalizeDelegate, preventing V8 hidden-class transitions caused by initializingresultas a string and later overwriting it.Reviewed by Cursor Bugbot for commit 770c6c0. Bugbot is set up for automated code reviews on this repo. Configure here.