microsoft / graphitation

GraphQL tooling & runtime support needed for MS Teams and beyond
https://microsoft.github.io/graphitation/
MIT License
141 stars 36 forks source link

[ARRDT] look into optimism thrashing #113

Open alloy opened 2 years ago

alloy commented 2 years ago

According to @vladar any read from the cache adds entries to optimism. We need to verify this and try to avoid it, perhaps as described here https://github.com/apollographql/apollo-client/issues/9306#issuecomment-1015634246

vladar commented 2 years ago

Yes, I see optimism cache growing on simple clicks on todo items:

MicrosoftTeams-image (1)

vladar commented 2 years ago

One offender is this call where we create a new fragment document object every time (note: fragment: { is always a new object):

https://github.com/microsoft/graphitation/blob/cc843406967da7c964bd5c68e79e1cc3f1b234f5/packages/apollo-react-relay-duct-tape/src/storeObservation/nodeFromCacheFieldPolicy.ts#L42-L52

A simple caching of this document in a WeakMap (using options.query as a key) allowed to lower growth rate to 2 new entries per click vs 4.

Those 2 remaining items are still to be investigate - I don't think they should appear at all.

vladar commented 2 years ago

One thing that popped up during a discussion: graphitation compiler may produce multiple instances of fragment documents (one in queries, and another for fragment hooks) which may end up in even larger number of optimism entries.

We need to investigate if it actually affects optimism size.

alloy commented 2 years ago

One offender is this call where we create a new fragment document object every time (note: fragment: { is always a new object):

https://github.com/microsoft/graphitation/blob/cc843406967da7c964bd5c68e79e1cc3f1b234f5/packages/apollo-react-relay-duct-tape/src/storeObservation/nodeFromCacheFieldPolicy.ts#L42-L52

This one is resolved by #127.