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

Remove Cache Implementations #353

Closed
lewisjkl opened this issue Feb 17, 2020 · 5 comments · Fixed by #345 or #362
Closed

Remove Cache Implementations #353

lewisjkl opened this issue Feb 17, 2020 · 5 comments · Fixed by #345 or #362
Labels

Comments

@lewisjkl
Copy link
Collaborator

There has been talk (mainly in #336) about removing specific cache implementations from this project. The thought is that we need to draw a line around what cache implementations do and do not belong in the core repo. There can always be another repo that provides other implementations, but this core repo should probably not have an indefinite number of cache implementations.

In my opinion, the core repo should contain only the few most commonly used cache implementations. In the future, another repo could always be created to contain other cache client implementations for less-commonly used caches.

We should discuss, but I’d recommend removing the following cache implementations:

  1. Guava
  2. ehcache
  3. ohc
  4. cache2k

This would leave:

  1. Caffeine
  2. Redis
  3. Memcached
  4. A proposed “default” implementation built on cats-effect Ref.

I’m totally open to debate on which cache implementations do and do not belong in core. In general I think Redis, memcached, and caffeine belong due to their popularity. I also think a default implementation would be nice for those who just want something the works without caring as much about the backend of the cache.

@ben-manes
Copy link

I also think a default implementation would be nice for those who just want something the works without caring as much about the backend of the cache.

Is this for demoware? What advantage do you gain by making it easiest to do the wrong thing in production? 😉

I'd recommend forcing the user to make a choice with as littleceremonyy as possible.

@lewisjkl
Copy link
Collaborator Author

@ben-manes I forgot to mention more of my rationale there. I was mainly thinking that the Ref implementation could be helpful for Scala JS since none of the other cache implementations will actually work there.

You do bring up a fair point, but I’d also be curious to see in a benchmark if it is really that bad. I’m sure it will be outperformed easily by caffeine, but also wouldn’t be completely unreasonable to use for certain cases either.

If it is concerning enough though, perhaps there is a way to support that cache type for Scala JS only. Or maybe there is another backing cache that would be a good default for Scala JS. What are your thoughts?

Sent with GitHawk

@ben-manes
Copy link

Thanks for the explanation, Scala.js makes total sense.

@cb372
Copy link
Owner

cb372 commented Feb 18, 2020

Thanks @lewisjkl for creating the issue. I am 👍 on your suggested removals.

As for whether we still want to support scala.js, that's a different story (which I will open an issue for now).

@lewisjkl
Copy link
Collaborator Author

Sounds good! With that in mind, let’s just consider the Ref cache implementation to be out of the scope of this issue so that this issue can just be focused on removing existing cache implementations.

Sent with GitHawk

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