Optimization: skip writing still-fresh result objects back into the EntityStore. #6274
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
benjamn
commented
May 13, 2020
Comment on lines
+171
to
+172
| const latest = this.executeSelectionSet.peek( | ||
| store, selectionSet, parent, varString); |
There was a problem hiding this comment.
We don't actually have enough information here to call this.executeSelectionSet the normal way, but we can peek its value using the arguments returned by keyArgs, since that's enough to compute the cache key and look up the latest result.
This was referenced May 13, 2020
If we're about to write a result object into the store, but we happen to know that the exact same (===) result object would be returned if we were to re-read the result with the same inputs right now (store, selection set, parent object, variables), then we can skip the rest of the processSelectionSet work for this object. Note that this trick only works because result objects are frozen (in development), which allows us to assume the structure of the === result object has not changed since it was originally returned. The executeSelectionSet.peek method is also vital here, since we need a way to check the latest cached result without recomputing the result if it's missing or dirty. This optimization makes the readQuery-transform-writeQuery pattern much cheaper when the written data contains a bunch of unmodified result objects that we recently read from the cache.
We were already doing this on the StoreReader side, but now we do it for the StoreWriter context, too.
benjamn
force-pushed
the
skip-writing-fresh-cache-results
branch
from
53a2fd1 to
32b3f42
Compare
benjamn
added a commit
that referenced
this pull request
May 16, 2020
The `cache.modify` API was first introduced in #5909 as a quick way to transform the values of specific existing fields in the cache. At the time, `cache.modify` seemed promising as an alternative to the `readQuery`-transform-`writeQuery` pattern, but feedback has been mixed, most importantly from our developer experience team, who helped me understand why `cache.modify` would be difficult to learn and to teach. While my refactoring in #6221 addressed concerns about broadcasting `cache.modify` updates automatically, the bigger problem with `cache.modify` is simply that it requires knowledge of the internal workings of the cache, making it nearly impossible to explain to developers who are not already Apollo Client 3 experts. As much as I wanted `cache.modify` to be a selling point for Apollo Client 3, it simply wasn't safe to use without a firm understanding of concepts like cache normalization, references, field identity, read/merge functions and their options API, and the `options.toReference(object, true)` helper function (#5970). By contrast, the `readQuery`-transform-`writeQuery` pattern may be a bit more verbose, but it has none of these problems, because these older methods work in terms of GraphQL result objects, rather than exposing the internal data format of the `EntityStore`. Since `cache.modify` was motivated to some extent by vague power-user performance concerns, it's worth noting that we recently made `writeQuery` and `writeFragment` even more efficient when rewriting unchanged results (#6274), so whatever performance gap there might have been between `cache.modify` and `readQuery`/`writeQuery` should now be even less noticeable. One final reason that `cache.modify` seemed like a good idea was that custom `merge` functions can interfere with certain `writeQuery` patterns, such as deleting an item from a paginated list. If you run into trouble with `merge` functions running when you don't want them to, we recommend calling `cache.evict` before `cache.writeQuery` to ensure your `merge` function won't be confused by existing field data when you write your modified data back into the cache.
benjamn
added a commit
that referenced
this pull request
May 18, 2020
The `cache.modify` API was first introduced in #5909 as a quick way to transform the values of specific existing fields in the cache. At the time, `cache.modify` seemed promising as an alternative to the `readQuery`-transform-`writeQuery` pattern, but feedback has been mixed, most importantly from our developer experience team, who helped me understand why `cache.modify` would be difficult to learn and to teach. While my refactoring in #6221 addressed concerns about broadcasting `cache.modify` updates automatically, the bigger problem with `cache.modify` is simply that it requires knowledge of the internal workings of the cache, making it nearly impossible to explain to developers who are not already Apollo Client 3 experts. As much as I wanted `cache.modify` to be a selling point for Apollo Client 3, it simply wasn't safe to use without a firm understanding of concepts like cache normalization, references, field identity, read/merge functions and their options API, and the `options.toReference(object, true)` helper function (#5970). By contrast, the `readQuery`-transform-`writeQuery` pattern may be a bit more verbose, but it has none of these problems, because these older methods work in terms of GraphQL result objects, rather than exposing the internal data format of the `EntityStore`. Since `cache.modify` was motivated to some extent by vague power-user performance concerns, it's worth noting that we recently made `writeQuery` and `writeFragment` even more efficient when rewriting unchanged results (#6274), so whatever performance gap there might have been between `cache.modify` and `readQuery`/`writeQuery` should now be even less noticeable. One final reason that `cache.modify` seemed like a good idea was that custom `merge` functions can interfere with certain `writeQuery` patterns, such as deleting an item from a paginated list. If you run into trouble with `merge` functions running when you don't want them to, we recommend calling `cache.evict` before `cache.writeQuery` to ensure your `merge` function won't be confused by existing field data when you write your modified data back into the cache.
benjamn
added a commit
that referenced
this pull request
May 18, 2020
The `cache.modify` API was first introduced in #5909 as a quick way to transform the values of specific existing fields in the cache. At the time, `cache.modify` seemed promising as an alternative to the `readQuery`-transform-`writeQuery` pattern, but feedback has been mixed, most importantly from our developer experience team, who helped me understand why `cache.modify` would be difficult to learn and to teach. While my refactoring in #6221 addressed concerns about broadcasting `cache.modify` updates automatically, the bigger problem with `cache.modify` is simply that it requires knowledge of the internal workings of the cache, making it nearly impossible to explain to developers who are not already Apollo Client 3 experts. As much as I wanted `cache.modify` to be a selling point for Apollo Client 3, it simply wasn't safe to use without a firm understanding of concepts like cache normalization, references, field identity, read/merge functions and their options API, and the `options.toReference(object, true)` helper function (#5970). By contrast, the `readQuery`-transform-`writeQuery` pattern may be a bit more verbose, but it has none of these problems, because these older methods work in terms of GraphQL result objects, rather than exposing the internal data format of the `EntityStore`. Since cache.modify was motivated to some extent by vague power-user performance concerns, it's worth noting that we recently made writeQuery and writeFragment even more efficient when rewriting unchanged results (#6274), so whatever performance gap there might have been between cache.modify and readQuery/writeQuery should now be even less noticeable. One final reason that `cache.modify` seemed like a good idea was that custom `merge` functions can interfere with certain `writeQuery` patterns, such as deleting an item from a paginated list. If you run into trouble with `merge` functions running when you don't want them to, we recommend calling `cache.evict` before `cache.writeQuery` to ensure your `merge` function won't be confused by existing field data when you write your modified data back into the cache.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
If we're about to write a result object into the store, but we happen to know that the exact same (
===) result object would be returned if we were to reread the result with the same inputs right now, then we can skip the rest of theprocessSelectionSetwork for this object, and immediately return aReferenceto it.Note that this trick only works because result objects are immutable (frozen in development), which allows us to assume the structure of a
===result object has not changed since it was originally returned.The
executeSelectionSet.peekmethod is also vital here, since we need a way to check the latest cached result without recomputing the result if it's missing or dirty. This PR depends on these two recentoptimismPRs: benjamn/optimism#65, benjamn/optimism#66.This optimization promises to make the
readQuery-transform-writeQuerypattern much cheaper when the written data contain lots of unmodified result objects that we recently read from the cache, such as when you append a singleTodoobject to a long list of existingTodoobjects.