-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update optimism to use lighter-weight dependency API in EntityCache. #5325
Conversation
// the StoreObject ensures the ID stays dirty unless its StoreObject | ||
// value is actually === the same as before. | ||
return wrap((dataId: string) => this.data[dataId], { | ||
disposable: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's how we used to abuse the wrap
API to achieve what dep
now gives us immediately.
Background explanation: benjamn/optimism#50
a0d3ce5
to
c98cdcb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome @benjamn - thanks!
Anecdotally, I saw a ~5% improvement on a benchmark involving a large query that accesses many cache IDs. Your mileage will vary depending on your queries, but I'm confident this PR does not accidentally regress performance. |
Ever since #3394, we have used the http://npmjs.org/package/optimism to speed up repeated cache reads, by recording dependencies on specific cache IDs while computing functions like
StoreReader#executeSelectionSet
, and later invalidating those cached results if/when the underlying data changed in the cache.While this system has proven to be logically correct and net-beneficial for many use cases, it wasn't as fast as it could be, because we handled the underlying ID-specific dependencies as if they were cached functions that could be recomputed, and paid the price of allowing them to have child dependencies of their own, even though they never did.
By taking advantage of the new
dep
API introduced by benjamn/optimism#50, we can record and dirty these leaf dependencies much more cheaply, which should reduce the overhead associated with leavingresultCaching
enabled in theInMemoryCache
.This performance diet is especially appealing because leaf dependencies (cache IDs) outnumber cache reads in most applications, and ID values can change in the
EntityCache
many times in between cache reads, so speeding up all thoseget
/set
operations is worthwhile.