graphitation icon indicating copy to clipboard operation
graphitation copied to clipboard

[ARRDT] look into optimism thrashing

Open alloy opened this issue 2 years ago • 4 comments

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

alloy avatar Jan 28 '22 11:01 alloy

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

MicrosoftTeams-image (1)

vladar avatar Feb 03 '22 16:02 vladar

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 avatar Feb 03 '22 16:02 vladar

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.

vladar avatar Feb 11 '22 14:02 vladar

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.

alloy avatar Apr 05 '22 13:04 alloy