-
Notifications
You must be signed in to change notification settings - Fork 4
Added invalidation helpers + ::cache meta to defcached and cached
#10
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
base: master
Are you sure you want to change the base?
Changes from all commits
2d0adce
9f77eb5
57fd5aa
1defc80
01adbc2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,6 +86,11 @@ | |
| (.build cache-builder (cache-loader cache-fn)) | ||
| (.build cache-builder)))) | ||
|
|
||
| (defn cache-keys | ||
| "Returns a list of cache keys" | ||
| [^Cache cache] | ||
| (.keySet (.asMap cache))) | ||
|
|
||
| (defn put | ||
| "Insert/update a cache entry." | ||
| [^Cache cache key val] | ||
|
|
@@ -102,6 +107,11 @@ | |
| [^Cache cache key] | ||
| (.invalidate cache key)) | ||
|
|
||
| (defn invalidate-keys | ||
| "Invalidate a cache entries." | ||
| [^Cache cache keys] | ||
| (.invalidateAll cache keys)) | ||
|
|
||
| (defn lookup | ||
| "Performs cache lookup. Accepts optional function which uses cache key to calculate missing value" | ||
| ([^Cache cache key] | ||
|
|
@@ -139,12 +149,16 @@ | |
| (let [options (meta f) | ||
| condition-fn (-> f meta :when) | ||
| cache (build-cache options (when-not condition-fn f))] | ||
| ^{::cache cache} | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like it's pretty much an implementation detail, so maybe we could add a helper for getting cache from the function using the meta? 🤔
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a bit tricky, but I did it=) Question is how to name it... too many
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, there is a
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. now I feel like maybe storing cache in function meta/var meta is a bit messy way 🤔
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally, I prefer meta because:
Yes, integrating this meta into That said, if you have a clear idea for a different approach, I’d be really interested to see how it turns out =)
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. meta is convenient, but it's way more complicated than it seems, as it could be attached to the object, but only of certain types, and passed along with it, and also to refs (vars, namespaces etc) which has completely different semantics and can contain objects of any type including nil 🤷🏽♂️ |
||
| (fn [& args] | ||
| (let [key (or args [])] | ||
| (if condition-fn | ||
| (cond-lookup cache key condition-fn f args) | ||
| (lookup cache key)))))) | ||
|
|
||
| (defn cached-cache [cached-fn] | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds like |
||
| (-> (meta cached-fn) ::cache)) | ||
|
|
||
| (defmacro defcached | ||
| "Creates a function which uses Caffeine Loading Cache under the hood. | ||
| Function declaration is similar to defn: | ||
|
|
@@ -181,11 +195,14 @@ | |
| :else 3)) | ||
| fdefn (filter some? (list name docstring args prepost))] | ||
| `(do | ||
| (def ^:private ~fname | ||
| (cached | ||
| (with-meta | ||
| (fn [~@cache-key] | ||
| ~@body) | ||
| ~cache-options))) | ||
| (defn ~@fdefn (~fname ~@cache-key)) | ||
| (vary-meta ~name merge ~(meta name))))) | ||
| (def ^:private ~fname (cached | ||
| (with-meta | ||
| (fn [~@cache-key] | ||
| ~@body) | ||
| ~cache-options))) | ||
| (defn ~@fdefn (~fname ~@cache-key)) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe should apply cache metadata taken from something like (def ~name
(with-meta
(fn ~@(when docstring [docstring])
~@(when prepost [prepost])
[~@args]
(~fname ~@cache-key))
(meta ~fname)))))my idea here is to always put cache meta on function object, so when we need to get cache, we can use only function instead of 2 approaches: function and var
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks interesting!🤔 I didn't want to fight with all those variables, but it doesn't look as bad as I imagined. I'm busy with other stuff this week, but on the next week I should be able to return into this pr. |
||
| (alter-meta! (var ~name) merge (meta ~fname) ~(meta name))))) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. interesting, does
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I mentioned in PM, looks like
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes,
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also I can't remember which kind of meta did we pass on that function and for the sake of what 😅
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't remember whether I tested all possibilities, but as I remember, in this case, I could have used |
||
|
|
||
| (defmacro defcached-cache [defcache-fn] | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a bit weird name, and I don't fully understand what's the use-case |
||
| `(cached-cache (var ~defcache-fn))) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking if
invalidate-allcould be better name to mimic native call, but feels likeallis a bit misleading because it's not "all" but listed ones 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that is why I named it this way -
invalidate-alllooks too bad for my liking😅🙈 Of course, we can have, for example,invalidate-allwith 2 arities -cache+cache keys. Then it will be a bit more logical, but still...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm now thinking maybe we should allow passing caching function as an argument 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean we're passing cache here, but what if we'll pass function, and this functions will extract cache using the implementation details