Cache query documents transformed by InMemoryCache. #3553
Merged
+14
−1
Conversation
This file contains hidden or 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
After #3444 removed `Map`-based caching for `addTypenameToDocument` (in order to fix memory leaks), the `InMemoryCache#transformDocument` method now creates a completely new `DocumentNode` every time it's called (assuming this.addTypename is true, which it is by default). This commit uses a `WeakMap` to cache calls to `addTypenameToDocument` in `InMemoryCache#transformDocument`, so that repeated cache reads will no longer create an unbounded number of new `DocumentNode` objects. The benefit of the `WeakMap` is that it does not prevent its keys (the original `DocumentNode` objects) from being garbage collected, which is another way of preventing memory leaks. Note that `WeakMap` may have to be polyfilled in older browsers, but there are many options for that. This optimization will be important for #3394, since the query document is involved in cache keys used to store cache partial query results. cc @hwillson @jbaxleyiii @brunorzn
benjamn
force-pushed
the
cache-transformed-query-documents
branch
from
9073936 to
c3382d8
Compare
Generated by 🚫 dangerJS |
ghost
mentioned this pull request
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.
After #3444 removed
Map-based caching foraddTypenameToDocument(in order to fix memory leaks), theInMemoryCache#transformDocumentmethod now creates a completely newDocumentNodeevery time it's called (assumingthis.addTypenameis true, which it is by default).This commit uses a
WeakMapto cache calls toaddTypenameToDocumentinInMemoryCache#transformDocument, so that repeated cache reads will no longer create an unbounded number of newDocumentNodeobjects. The benefit of theWeakMapis that it does not prevent its keys (the originalDocumentNodeobjects) from being garbage collected, which is another way of preventing memory leaks. Note thatWeakMapmay have to be polyfilled in older browsers, but there are many options for that.This optimization will be important for #3394, since the query document is involved in cache keys used to store cache partial query results.
cc @hwillson @jbaxleyiii @brunorzn