Closed xsc closed 3 years ago
@xsc feel free to submit a pull request!
Well I'm not sure spyglass should even contain an implementation for core.cache
since those operations seem to be designed for immutable in-memory caches (no, really!). There is no concept of state in core.cache
and a lot of composability relies on that fact.
More importantly, it should be guaranteed that if I call hit
or miss
on a cache and then perform a lookup
on the result of these functions, I should not have to fear that my key expired in-between. This cannot be made sure using a memcached- or DB-backed cache.
What I'm trying to say: core.cache
seems to be designed for in-memory memoization (e.g. with core.memoize
) and the only time values are really purged from the data store should be when calling miss
. So that's not the kind of protocol/interface/semantics a memcached-backed cache should (or is able to) implement.
All this semantics was not documented by the time our core.cache
implementation landed. Even though our implementation is not perfect, it is certainly not unheard of to use core.cache
as a common interface to the most basic K/V operations. So it makes sense for Spyglass to provide one.
I can see that, sure. And if one implements a cache protocol it's probably best to choose the library prefixed with core.
. There really is no problem with direct usability here, only when trying to integrate spyglass with libraries using core.cache
does one see that it's not really suitable. (core.memoize
stores e.g. values wrapped in delays which are not serializable and thus result in exceptions thrown inside of the memcached client.)
I'll still try to create an appropriate pull request, adressing the most immediate problems. Thanks for your response!
Serializability of delays sounds like a core.memoize
issue to me and not something inherent to caching or core.cache
's protocol. Thank you for considering to contribute!
I meant to address the reasoning behind core.memoize
's concrete behaviour: it expects core.cache
to store keys and values in-memory, thus seeing no problem with non-serializable values. And if someone models their library to rely on the semantics they gather from core.cache
and a specific cache implementation breaks said library, on which side does the issue lie?
So, the main problem is probably the naming of core.cache
since it's not really a suitable abstraction over caching in general but seems to have been built as the basis for core.memoize
, as well as the fact that nowhere on the core.cache
page does it really describe the semantics.
(Additionally, I don't think has?
/hit
/miss
/lookup
is the right access pattern for mutable caches without transaction semantics; atomic operations like lookup-if-cached
seem better to me.)
Edit: I should probably address that over there. Sorry for ranting. ;)
The implementation in
clojurewerkz.spyglass.cache
seems to have some minor problems.clojure.core.cache/defcache
expects the first field of a cache to be a seq/map with its contents (see here), an exception is thrown when e.g. callingseq
/count
/any seq function covered bydefcache
on the cache. This makes sense, although the exception message is not very expressive. Maybe using a constant empty map ornil
would reduce headaches when trying to integrate with other libraries relying on core.cache?clojure.core.cache/has?
throws an exception if the value-to-be-accessed is not seqable (because of this). This breaks the proposedhas?
/hit
/miss
pattern for a lot of cases.