rum icon indicating copy to clipboard operation
rum copied to clipboard

derived-atom: remove watches on source atoms

Open martinklepsch opened this issue 9 years ago • 5 comments

When using derived-atom a watch is added to all source atoms but there's no way to remove these watches potentially causing stale watches when the derived atom is no longer needed.

I have a very rough implementation that extends the derived atom implementation by a IDisposable protocol which provides a method that will remove all relevant watches on source atoms: https://github.com/martinklepsch/derivatives/blob/disposable/src/org/martinklepsch/derived.cljc

Watching a derived atom is broken in this implementation because it's not an atom but that's easily fixable.

Now the question, would you be interested in extending Rum's derived atom to cover this aspect?

martinklepsch avatar Aug 09 '16 21:08 martinklepsch

Yes! A way to dispose derived-atom is surely needed. I think dispose is probably better than going manually to every source atom and unsubscribe the key. I was only thinking about statically-defined derived atoms (via top-level defs), where you don’t really need to dispose anything.

tonsky avatar Aug 16 '16 09:08 tonsky

Finally had some time for this and ended up with a DerivedValue type that implements IDeref & IWatchable (like atoms) in addition to the IDisposable protocol suggested above.

Would you be open to replace derived-atom with this? I guess we could also stick to the derived-atom name if you'd like to maintain compatibility.

https://github.com/martinklepsch/derivatives/blob/master/src/org/martinklepsch/derived.cljc

martinklepsch avatar Sep 07 '16 12:09 martinklepsch

@martinklepsch So essentially we want to propagate remove-watch to source refs, is that correct?

roman01la avatar Apr 16 '20 12:04 roman01la

That sounds about right, yes!

martinklepsch avatar Apr 16 '20 14:04 martinklepsch

Heyo, folks! Been curious about making derived-atoms lazy. I found that it takes on the same problem as been discussed - derived-atoms to be alive and running only when they're being watched. As I've been thinking about a solution, I found these properties desired of, let's call them lazy-derived-atoms:

  1. Compatible with rum/reactive.

  2. Being smart to stay lazy when nobody's watching. When they're watched only by other lazy-derived-atoms (or, rather, transitive dependees are all lazy-derived-atoms) - nobody's intersted in their stuff - we don't want any of them running. E.g., there's a plain atom a, and three lazy-derived-atoms b, c and d

      a
     / \
    b   c
     \ /
      d
    

    Where b depends on a and so on. While a may be getting updates, we don't want any of b, c or d running. But say if there is (rum/react d) - stuff's needed, they are running/eager. And as soon as that component is not rendered anymore - they're back at slacking around, being lazy as before.

  3. A lazy-derived-atom may be dereferenced! Just like we do with plain atoms, via @. In this case I want them to return the correct derived value out of its fresh dependencies. E.g., @d would return it's val over the current state of a. 2.1 Doing that twite, while the deps are the same, don't incur unnecessary re-eval.

  4. When there's a diamond dependency structure, like the one in the example above, using plain add-watch would result in change to a triggering re-eval of b, d and c, d. Note how d evals twice! If these would be plain derived-atoms, then d would first evaluate on fresh b and stale c, and on the second pass eval on fresh vals. But since I wanted @d to re-compute, being fresh (2.), the first pass would actually run d on fresh b and c. So the re-eval on the second pass would do the same, unnecessarily. A solution to that is to re-eval a lazy-derived-atom only when it's deps did change. But this check adds cost, which I'm not happy with. So better suggestions are welcome.

In the end, I managed to get the desired properties with this implementation (some tests down below there, as examples). Having used lazy-derived-atoms here, calculating debug info out of function traces, when the component's rendered (folded by default), I'm happy with the result. But it's just one use-case. Performance cost due to comparison of args is in question. Let me know of a better way.

All-in-all, wanted to share this with you folks, perhaps you may find it to be of use.

@martinklepsch, thanks for this discussion and the link to the IDisposable version, been an inspiration. (And for the derivatives! Concise declaration in data is nice.)

andrewzhurov avatar Mar 29 '24 15:03 andrewzhurov