perf(store): reduce allocations in hot paths across store package#8227
perf(store): reduce allocations in hot paths across store package#8227steveruizok wants to merge 11 commits intomainfrom
Conversation
… loops Avoid intermediate array allocations from Object.entries(), Object.values(), and objectMapEntries()/objectMapValues() by using for...in to iterate object keys directly. This eliminates per-call array creation in hot paths across RecordType.create, reverseRecordsDiff, squashRecordDiffsMutable, filterHistory, index incremental updates, ids incremental updates, extractMatcherPaths, and objectMatchesQuery. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eturn Avoid allocating a keys array just to check if an object is empty. Use for...in with an immediate return/break instead, which is O(1) for empty objects and avoids the array allocation entirely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace {...defaults, id} spread with direct property assignment on the
defaults object. This avoids creating a temporary object just to merge
in the id property.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace three filterEntries() calls (each allocating an intermediate object and iterating with Object.entries) plus three Object.keys().length checks with a single pass using for...in. Track hasChanges with a boolean flag instead of checking emptiness after the fact. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Find the smallest set up front and iterate only its elements, checking membership in the other sets. This reduces iterations from O(|first|) to O(|smallest|). Also replaces rest.every() closure per element with a labeled loop to avoid per-element closure allocation, and adds early returns for length-1 input and empty smallest set. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Short-circuit when prev === next (same reference), and when either set is empty, to avoid iterating when the result is trivially known. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… path Replace Object.fromEntries() keyed map with a simple array of sets, avoiding the intermediate entries array and object allocation. Add a fast path that returns the single set directly when there is only one query predicate, skipping the intersection entirely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cache the result of Map.get() in a local variable instead of calling has() then get() separately, eliminating one hash lookup per record in the index initialization path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace objectMatchesQuery() calls in the ids() incremental path with
a compiled matcher. The query is compiled once into an array of
{parts, kind, value} descriptors, then matched via a tight loop with
a switch statement and getNestedValue(). This avoids repeated
isQueryValueMatcher() checks and Object.entries() allocations on
every record during incremental diff processing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Remove usage of objectMapValues/objectMapKeys and use explicit for-in loops to build toPut and toRemove arrays from diff.added, diff.updated and diff.removed. Preserve existing ignoreEphemeralKeys logic while switching to index-based access (updated entries use diff.updated[id][1]) and add explicit type annotations for arrays. Clean up imports to remove now-unused helpers and apply minor formatting/indent adjustments. No behavioral changes intended.
|
|
||
| for (const [k, v] of Object.entries(properties)) { | ||
| if (v !== undefined) { | ||
| result[k] = v |
There was a problem hiding this comment.
create() mutates object returned by createDefaultProperties()
Low Severity
The old code used { ...this.createDefaultProperties(), id: ... } which always created a fresh object via spread, making create() safe regardless of what createDefaultProperties() returns. The new code directly mutates the returned object by assigning id, typeName, and all properties onto it. If a consumer's createDefaultProperties function returns a shared, cached, or frozen object, subsequent create() calls would corrupt previously created records or throw in strict mode.
There was a problem hiding this comment.
createDefaultProperties is a factory function that always returns a fresh object literal — there are no shared or cached instances in the codebase. This is safe.
When a nested property path resolves to undefined (e.g. null intermediate), the neq matcher incorrectly treated the record as matching because undefined === value is always false. Match the behavior of objectMatchesQuery which returns false for unresolvable paths. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| break | ||
| case 'neq': | ||
| if (value === undefined || value === c.value) return false | ||
| break |
There was a problem hiding this comment.
Compiled neq matcher rejects undefined values differently
Low Severity
The compiled neq matcher adds a value === undefined guard that the original objectMatchesQuery did not have. For top-level properties that are undefined, objectMatchesQuery would consider undefined !== targetValue as matching, but the compiled matcher returns false. This changes observable incremental query results for records with missing or undefined properties when using neq queries.
1 participant
This one has changes separated into clean commits commits. I ran the bots through the package looking to minimize allocations. This isn't something we've really paid attention to before, however it's easy work for the bots and will make a difference on our hot paths. See #8225 for a similar PR. –steve
In order to reduce GC pressure and improve throughput in the store's hot paths, this PR eliminates unnecessary intermediate allocations across the
@tldraw/storepackage. The changes target iteration patterns, empty-object checks, set operations, and query matching—all areas that run frequently during incremental reactive updates.Summary of changes
Object.entries/Object.values/objectMapValueswithfor...inloops — avoids allocating intermediate arrays on every iterationObject.keys().length === 0withfor...inearly return — O(1) empty check without array allocationRecordType.create— mutate the defaults object directly instead of creating a temporary spreadfilterChangesByScope— replaces threefilterEntries()calls + threeObject.keys().lengthchecks with a singlefor...inpassintersectSets— iterate the smallest set first, use labeled loops instead ofrest.every()closuresdiffSets— short-circuit for same-reference and empty setsexecuteQuery— use an array of sets instead ofObject.fromEntries, skip intersection for single-predicate queriesMap.getlookup — cache the result ofMap.get()instead of callinghas()thenget()objectMatchesQuery()in theids()hot pathChange type
improvementTest plan
API changes
Store.filterChangesByScopereturn type changed from mapped type{ [K in IdOf<R>]: R }to equivalentRecord<IdOf<R>, R>(cosmetic, not a breaking change)Release notes
Code changes
Note
Medium Risk
Touches core store diff application, history filtering, and reactive query/index incremental paths; while changes are intended to be behavior-preserving, subtle iteration/empty-check/matcher differences could affect which records are included or which updates are emitted.
Overview
Reduces GC pressure in
@tldraw/storeby rewriting several hot paths to avoid intermediate allocations (replacingObject.*/objectMap*helpers andObject.keys().lengthchecks withfor...inloops and early-exit emptiness checks).Optimizes diff and history handling by inlining
Store.filterChangesByScope, updatingapplyDiffto buildtoPut/toRemovearrays without helper allocations, and updatingRecordsDiffutilities (reverseRecordsDiff,isRecordsDiffEmpty,squashRecordDiffsMutable) to iterate withoutObject.values/objectMapEntries.Speeds up querying by optimizing
executeQuery(array-of-sets + single-predicate fast path), improvingintersectSets/diffSetsfast paths, and adding a compiled query matcher used inStoreQueries.ids()incremental updates (replacing per-updateobjectMatchesQuerycalls). API report updatesfilterChangesByScopeto returnRecord<IdOf<R>, …>types.Written by Cursor Bugbot for commit 4c85390. This will update automatically on new commits. Configure here.