Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

derived-atom: remove watches on source atoms #94

Open
martinklepsch opened this issue Aug 9, 2016 · 5 comments
Open

derived-atom: remove watches on source atoms #94

martinklepsch opened this issue Aug 9, 2016 · 5 comments

Comments

@martinklepsch
Copy link
Contributor

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?

@tonsky
Copy link
Owner

tonsky commented Aug 16, 2016

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.

@martinklepsch
Copy link
Contributor Author

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

@roman01la
Copy link
Collaborator

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

@martinklepsch
Copy link
Contributor Author

That sounds about right, yes!

@andrewzhurov
Copy link

andrewzhurov commented Mar 29, 2024

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants