Various cache read and write performance optimizations. #5948
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
When I wrote this code, I thought this array would usually be so short that indexOf would be faster than Set.prototype.has, but of course the pathological cases are what end up mattering, and I've recently seen some result objects that cause thousands of shallow copies to be made over a series of many merges using one DeepMerger instance.
Since shouldInclude gets called for every single field in any read or write operation, it's important that it takes any available shortcuts to handle the common case (no directives) as cheaply as possible.
Since any of the provided variables could be consumed by any of the fields in a selection set that we're reading, all variables are potentially relevant as part of the result object cache key, so we don't make any attempt to stringify just a subset of the variables. However, since we use the same stringified variables in every cache key, there's no need to perform that encoding repeatedly. JSON.stringify may be fast, but the variables object can be arbitrarily large.
Believe it or not, iterating over the values of policies.rootTypenamesById was noticeably expensive according to Chrome devtools profiling. Since this information almost never changes, we might as well maintain it in the format that's most convenient.
Creating a throwaway array just to call JSON.stringify was much more expensive than string concatenation. The exact format of these cache keys is an invisible implementation detail, so I picked something that seemed unlikely ever to be ambiguous, though we can easily change it later.
Since policies.applyMerges doesn't change anything unless there are custom merge functions to process, we can skip calling it if no merge functions were found while processing the current entity.
Instead of recursively calling processSelectionSet to handle fragments, we can simply treat their fields as fields of the current selection set.
This change means fragment results will no longer be cached separately from normal selection set results, which is potentially a loss of caching granularity, but there's also a reduction in caching overhead because we're caching fewer result objects, and we don't have to merge them all together, and (most importantly) the result caching system still tracks dependencies the same way as before. It's as if we transformed the query by inlining fragment selections, except without doing any work!
Although this may seem like a reversion to forEach instead of a for loop, the for loop had an unexpectedly negative impact on minification, and a Set has the ability to deduplicate selection objects, so we never re-process the same field multiple times through different fragments.
benjamn
force-pushed
the
read-and-write-performance-optimizations
branch
from
54d6ed2
to
dc61865
Compare
|
These changes have been released in |
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.
Although we've been building Apollo Client 3.0 with performance in mind, making sure (for example) that you only pay for the features you use, we haven't focused on optimizing raw performance per se, until now.
Using an especially large query and response object provided by a customer/contributor, I was able to reduce initial (cold) execution times for the following write/read round-trip from ~150ms (measured using the latest published beta,
@apollo/client@3.0.0-beta.34) to to ~95ms, a 37% improvement, and average (warm) execution times from ~105ms to ~25ms, a 76% improvement:For this benchmark, I created a new
InMemoryCacheobject for each run, so the results are not skewed by the benefits of result caching (see #3394) though result caching can have huge benefits for actual applications.As always with performance, exact numbers will vary from query to query and machine to machine, but I used a 2014 dual-core 3GHz MacBook Pro, a 36KB query with lots of fragments, and a 500KB JSON-encoded result.
It's worth reviewing each of these commits separately, as they do not really follow a common theme (except for speeeed
).