apollo-client icon indicating copy to clipboard operation
apollo-client copied to clipboard

Allow the removal of multiple optimistics at once

Open jackstudd opened this issue 1 year ago • 4 comments

In implementing offline mode in an app, we patched Apollo to be able to delete multiple optimistics at once, so that the chain of Layers are recreated only once. Deleting optimistics one by one would result in a spike in RAM & CPU consumption and would end up with a crash of the app, due to the chain of Layers being recreated entirely for each optimistic deletion.

We use things like persisted queue & persisted cache, to provide offline capabilities to the app. To speed up the syncing process, we also batch requests that comes from the queue, and to speed up even further, we don't notify the original operations observables like src/link/batch/batching.ts does, instead we just remove the optimistics. It has proven to work so far! I would love to have your feedbacks on this approach.

jackstudd avatar Jul 17 '24 10:07 jackstudd

Deploy request for apollo-client-docs pending review.

Visit the deploys page to approve it

Name Link
Latest commit 699ab4c4b508ad50e6977e33b6f7369692630587

netlify[bot] avatar Jul 17 '24 10:07 netlify[bot]

🦋 Changeset detected

Latest commit: 699ab4c4b508ad50e6977e33b6f7369692630587

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@apollo/client Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Jul 17 '24 10:07 changeset-bot[bot]

Hi @jackstudd 👋

Thanks for the PR and the background you've shared here. Our team has reviewed this and one question that came up is whether the broadcastWatches call of removeOptimistic was part of the bottleneck you're seeing, or if the goal is purely being able to remove several optimistics while creating only a single Layer? We're considering a few different ways of achieving this and any more information you can share will help here :) thanks!

alessbell avatar Jul 22 '24 15:07 alessbell

Hi @alessbell 🙋‍♂️

Thanks for your review.

I've tried with and without the broadcastWatches call, and didn't notice any behavioural/performance differences (I didn't measure the performances tho). The main issue was really in in the removeLayer function of Layer, replaying all the layers and recreating the entire chain at each optimistic removal, whether removing all the layers we want and recreating the chain once hugely improved performances.

The goal is purely to be able to remove multiple optimistics without having to replay all the layers. We could also say that the goal is to remove multiple optimistics without affecting performances!

I can also say that, notifying the original observables like src/link/batch/batching.ts does with many observables to notify ended up with the same performances issues as to removing multiple optimistics, but the cause is most likely the same.

jackstudd avatar Jul 24 '24 08:07 jackstudd