A way to switch off keepAlive for an existed computed

[geomaster]

Currently, there seems to be no way to "dispose" of a computed value that has been initialized with keepAlive. What I'm looking for is a way to allow the computed to be forgotten if it's no longer being observed.

The closest I've come to is this:

computed.keepAlive_ = false;
computed.suspend_();

However this is obviously not ideal because it relies on methods which are not made available on IComputedValue<T> (for good reason).

My motivation at the moment is caching the latest invocation of a very costly function longer than it's observed, which I do by wrapping it in a computed with keepAlive: true. When the function gets called with different arguments than the cached value however, I would like to forget the old computed and create a new one for the new set of arguments. Without being able to un-keepAlive these reactions, this would cause a memory leak, since the old computeds will stay in memory.

[mweststrate]

You could create an autorun that reads it instead? That gives an dispose hook

…

On Fri, 27 Oct 2023, 01:27 David Davidović, ***@***.***> wrote: Currently, there seems to be no way to "dispose" of a computed value that has been initialized with keepAlive. What I'm looking for is a way to allow the computed to be forgotten if it's no longer being observed. The closest I've come to is this: computed.keepAlive_ = false;computed.suspend_(); However this is obviously not ideal because it relies on methods which are not made available on IComputedValue<T> (for good reason). My motivation at the moment is caching the latest invocation of a very costly function longer than it's observed, which I do by wrapping it in a computed with keepAlive: true. When the function gets called with different arguments than the cached value however, I would like to forget the old computed and create a new one for the new set of arguments. Without being able to un-keepAlive these reactions, this would cause a memory leak, since the old computeds will stay in memory. — Reply to this email directly, view it on GitHub <#3784>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBCC4QUDHN2DTPCBLSDYBLWW3AVCNFSM6AAAAAA6R65XDGVHI2DSMVQWIX3LMV43ERDJONRXK43TNFXW4OZVG44DEMJYHE> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[geomaster]

You could create an autorun that reads it instead? That gives an dispose hook

True, but that would cause the computed to be eagerly re-evaluated whenever the values it's observing change, which is not preferable because we have quite a few of those. I'd much prefer if it worked lazily like a normal computed. For example, in the caching scenario I mentioned, the autorun approach would cause the old cached computed to be reevaluated before we get a chance to see if we'll reuse it or would need to dispose it and replace it with a different computation (if the argument list for the cached function changes). If the latter, we'd effecitvely be throwing away that computation.

[mweststrate]

Sorry, meant Reaction. Track the dependency once, but never reexecute it should do the trick. (autorun with custom scheduler that never runs again probably does the same, not sure from top of my head)

…

On Tue, 31 Oct 2023, 19:21 David Davidović, ***@***.***> wrote: You could create an autorun that reads it instead? That gives an dispose hook True, but that would cause the computed to be eagerly re-evaluated whenever the values it's observing change, which is not preferable because we have quite a few of those. For example, in the caching scenario I mentioned, the autorun approach would cause the old cached computed to be reevaluated before we get a chance to see if we'll reuse it or would need to dispose it. If the latter, we'd effecitvely be throwing away that computation. — Reply to this email directly, view it on GitHub <#3784 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN4NBFAZJTNGW4Q6PXUTLLYCE6SBAVCNFSM6AAAAAA6R65XDGVHI2DSMVQWIX3LMV43SRDJONRXK43TNFXW4Q3PNVWWK3TUHM3TIMZYGQYDC> . You are receiving this because you commented.Message ID: ***@***.***>

[vzhou842]

has the thinking here evolved in the past year? I have a similar issue to @geomaster and am considering using the hacky .suspend_() route because I haven't found anything that seems better yet.

from @mweststrate's most recent comment:

Sorry, meant Reaction. Track the dependency once, but never reexecute it should do the trick

I don't see how this would give a different outcome than the autorun() approach. Maybe I'm misunderstanding what "track the dependency once" means? My tests here were basically just using a dummy reaction(() => computed.get(), () => {})

autorun with custom scheduler that never runs again probably does the same, not sure from top of my head

I also tried this because it sounded promising to me at first, but I'm finding that the computed still gets rerun seemingly because of the shouldCompute() check for the autorun, which I'm guessing goes into case IDerivationState_.POSSIBLY_STALE_ and then invokes .get() on the computed? Which leads us back to the original issue of the computed being eagerly re-evaluated

[StephenHaney]

+1'ing the desire for a way to "wipe the board" of all keepAlive computeds of a given class instance. In Paper we have some expensive computed calcs, like walking the tree to find all descendants, or "am I part of the current selection". Most often these are used imperatively. But they are also sometimes observed, and thus computed is good. keepAlive is ideal for this use case, except the memory leaks.

I'm in this monthly cycle of creating memory leaks, raging, deleting all keepAlive: true in the codebase, finding performance issues, carefully adding some keepAlive back, then finding I wasn't careful enough and created more leaks.

I do have a reliable disposer on my class instances, so I just need a way to imperatively dispose these keepAlive computeds.

I'm going to attempt to either create my own decorator which registers a disposer, or try out the value.keepAlive_ = false; value.suspend() approach. I'll report back on success.