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

Replace watching-cache with Clojure-style non-watching cache #72

Open
camsaul opened this issue Jun 16, 2021 · 0 comments
Open

Replace watching-cache with Clojure-style non-watching cache #72

camsaul opened this issue Jun 16, 2021 · 0 comments
Labels
clojurescript support Issues that we have to tackle in order to get ClojureScript support working.
Milestone

Comments

@camsaul
Copy link
Owner

camsaul commented Jun 16, 2021

Vanilla Clojure multimethods cache the last known state of their hierarchy when adding methods or during method calculation. They check for changes to the hierarchy on invocation by checking whether the cached hierarchy is the same object as the hierarchy obtained by dereffing the hierarchy IRef.

Methodical multimethods instead put a watch on the hierarchy IRef and wipe the cache when the hierarchy changes. Overall this approach is theoretically slightly faster on method invocation since it avoids the deref and conditional. IRL this probably doesn't make a noticeable difference, and multimethods are always going to be slower than plain functions in tight loops anyway. It would be slower overall however if the hierarchy is changing a lot and the watch is getting triggered all the time when the multimethod is invoked only rarely. Or if you have hundreds of Methodical mulitmethods all watching the global hierarchy and any change to it triggers hundreds watches to reset their caches.

So it seems like which implementation is better depends on access patterns. The watching cache is an overall hairier approach. It also makes ClojureScript support difficult, since the watches use a WeakReference to the multimethod so as to avoid causing them to be retained if the multimethod itself would otherwise be GC'ed (e.g., for a programatically-created multimethod). ClojureScript (AFAIK) doesn't have a direct equivalent to WeakReference -- see #28.

Since the watching cache is more complicated, makes CLJS support hard, and probably not noticeably faster (or possibly slower IRL when you take into account the cost of all those watches), I think we should replace it with a Clojure-style identity-comparison cache.

It might be worth profiling both caches and seeing what performance looks like just be be sure. If the Clojure-style cache performs reasonably similar let's make it the new default and get rid of the watching cache altogether (or at least make it Clojure-only)

@camsaul camsaul added the clojurescript support Issues that we have to tackle in order to get ClojureScript support working. label Jun 16, 2021
@camsaul camsaul added this to the 1.0.0 milestone Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clojurescript support Issues that we have to tackle in order to get ClojureScript support working.
Projects
None yet
Development

No branches or pull requests

1 participant