perf(query-core): Improve mutationCache implementation performance #8451
Conversation
This file contains 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
The mutationCache implementation models all mutations as scoped even when the scope is implicit. With implicit scopes there can only ever be one mutation per scope so the overhead of tracking the scope is unecessary. Additionally because scopes need a unique value the current implementation bootstraps this value using a random source (Date.now()). While this is practically likely to be fine it is not impossible that there is a scope collision with mutation hydration. Modeling the internal state of the cache without deriving virtual scopes simplifies the implementation in memory and code and avoids the possibily of scope collision by design. This should provide a modest perf bump for mutations that do not use scopes and be neutral for mutations that do use scopes.
…rformance by modeling mutations in a single set Scopes are more like scheduling metadata than an intrinsic structure to the set of mutations. In this change I updated the storage format of mutations so that they can be cleared more efficiently. Using a Set allows us to have O(1) removals. We can also construct the Array for getAll() more efficiently by not having to flatten the inner scope arrays. Additionally the scope array is now mutated both on add and remove to avoid allocating extra arrays and doing additional copies. I rewrote clear to not use this.remove which avoids many intermediate operations against the underlyign data structures. I considered using an Array again for the primary storage format and this may actually be better if the expected number of mutations isn't excessively large but since I don't know enough about mutation usage I stuck with a Set for the better deletion performance at larger mutation size.
bai
reviewed
Dec 27, 2024
| if (typeof scope === 'string') { | ||
| const scopedMutations = this.#scopes.get(scope); | ||
| if (scopedMutations) { | ||
| scopedMutations.push(mutation);; |
There was a problem hiding this comment.
Suggested change
| scopedMutations.push(mutation);; | |
| scopedMutations.push(mutation); |
chore: format
|
I don’t know why our CI doesn’t run on this PR :/ I’ll try to open a new one |
found out why that happened. That’s what you get for trying out beta features I guess 😂 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #8277
This work is stacked on #8450
In the prior change I modeled unscoped mutations as a separate data store in a way that would speed up methods operating on the internal state of the cache when using unscoped mutations. In this change I further refatored the internal implementation to make removals faster (O(1) generally) and to make the aggregate APIs like
getAllandfindfaster by avoiding multiple concatenations and flattening.Implementation Details
Scopes are more like scheduling metadata than an intrinsic structure to the set of mutations. In this change I updated the storage format of mutations so that they can be cleared more efficiently. Using a Set allows us to have O(1) removals. We can also construct the Array for getAll() more efficiently by not having to flatten the inner scope arrays. Additionally the scope array is now mutated both on add and remove to avoid allocating extra arrays and doing additional copies.
I rewrote clear to not use this.remove which avoids many intermediate operations against the underlyign data structures.
I considered using an Array again for the primary storage format and this may actually be better if the expected number of mutations isn't excessively large but since I don't know enough about mutation usage I stuck with a Set for the better deletion performance at larger mutation size.