Fork me on GitHub

I've been looking into the implementation of subscriptions and noticed that subscriptions are being recomputed even when dependent data (the collection of input signals) has not changed. I'm curious why the computation fn is not memoized. Since subscriptions function handlers are pure, I don't see why that isn't the default.


can you be more specific when you're seeing excess computations in practice?


I am trying some different use cases where I want to use subscriptions outside of a reaction - I removed the sub -> reaction cache and just memoized the compute fns and only the dependent subscriptions in the compute graph are recomputed. I don't understand why the sub -> reaction cache is useful ( since any event that changes the db will trigger all downstream computations to run. But I also don't fully grok the use of that cache, so there's likely some point I'm missing.


using subscriptions outside of a reaction is going to end up either with excess computations or memory leaks


or both 😄


the sub->reaction cache is useful so that all reactions that depend on a sub end up depending on a single reaction. you end up creating less reaction objects that way


the memoization of the compute functions happen inside of that reaction


you do not want to memoize the compute fns themselves because there's no cleanup. the memoized results will stick around forever, leading to a memory leak


reactions automatically clean themselves up once no one depends on them anymore. re-frame cleans up its sub->reaction cache using this same mechanism


but that cleanup logic only works it you use them inside of a reaction


so even outside of re-frame, if I create a Reaction js object (or a graph of them) and use that however I like to perform computation, let us say in a js event handler - you're saying there is something inherent in Reactions that will result in the prevention of JS VM from garbage collecting those objects?


I'll have to keep playing around with this and see if I can recreate the re-running of the computation functions, I'm working from a forked version of re-frame so could be something I changed


re-frame subs are global. what I'm saying is that it you were to replace reg-sub with something like

(def kw->subs (atom {}))

(defn reg-sub
  [kw compute-fn]
  (swap! kw->subs assoc kw (memoize compute-fn)))


now you have a memory leak


the memoized fn is never GCd since it's reference is kept in the re-frame registry


and since it's memoized, it will collect results for the lifetime of the program


in your example of using reactions in an event handler, I would expect the opposite to occur - the reactions will recompute anytime you call them, because there's no reactive context


cool, that all makes sense. thanks for the explanation. - I put together a small app to test my understanding - the reactions are being memoized, I was seeing all the layer 2 subs fire whenever the db changed, which makes sense, but without inspecting it closer I thought a lot more subs were recomputing than was necessary. I think there could be something useful to come from a fork where the subsription -> reaction cache is removed and then the compute functions are memoized with a ttl cache or some other policy like a fixed window size of the last nth calls, because the benefits of using subscriptions in event handlers and other async contexts would be awesome


The subscription cache is invaluable in re-frame. Without it, subscriptions would only be usable in form-2 components. They can be used in form-1 components now because the subscription cache always returns the same subscription value when components re-render. Before the subscription cache, if a subcription was created and deref'd in a component's render function, a new reaction would be created and with it a new subscription graph, a new set of computations, every time react would render a component tree. It's gets very expensive. There are a number of things that could be done in re-frame to allow subscriptions to be used in event handlers and other async contexts. One possible solution is to simply not cache a subscription when it's not called in a reactive context. I implemented in a couple lines a while back and left it in a comment on a re-frame issue but didn't get any response That side steps the memory leak problem because subscriptions are only cached when there exists a reagent component to clean it up. I think it's worth merging in, but there may be some trade offs I'm not thinking of.


yes! i saw your branch - this is great 🙂 very perplexing why it's not merged by now, would be able to delete a lot of redundant application code that reimplements subscriptions just for use in event handlers


I agree. really that's something that ought to be done in reagent