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

Switch from concurrentlinkedhashmap-lru to caffeine #6725

Closed
hakanai opened this issue Sep 20, 2016 · 5 comments
Closed

Switch from concurrentlinkedhashmap-lru to caffeine #6725

hakanai opened this issue Sep 20, 2016 · 5 comments

Comments

@hakanai
Copy link

hakanai commented Sep 20, 2016

I noticed we have a copy of concurrentlinkedhashmap-lru in our project, pulled in by orient-core:

com.googlecode.concurrentlinkedhashmap:concurrentlinkedhashmap-lru:1.4.1
\--- com.orientechnologies:orientdb-core:2.2.8

The project site itself says that Caffeine is the successor library and a ticket I just raised with concurrentlinkedhashmap got a reply saying that switching is recommended.

I would also prefer not to have more than one library for the exact same purpose in our project when possible, and in this case, since it seems to be a direct successor, it seems like it should be possible.

Would it be possible for orientdb to switch caching libraries to caffeine?

@lvca
Copy link
Member

lvca commented Sep 22, 2016

Makes sense for OrientDB 3 that will require Java8, but now we can't substitute it because OrientDB 2.2.x is Java7 compatible.

@andrii0lomakin
Copy link
Member

@trejkaz @lvca why not will do

@andrii0lomakin andrii0lomakin added this to the 3.0 milestone Sep 22, 2016
@taburet taburet assigned taburet and unassigned andrii0lomakin Jan 23, 2017
@taburet
Copy link
Contributor

taburet commented Jan 24, 2017

Most of our usages of ConcurrentLinkedHashMap are more like of a concurrent pool-like acquire/release pattern: (1) no size limit to avoid automatic eviction of acquired entries to let all pool clients share the same entry instance, (2) manual, sometimes batched, eviction management, (3) usage of ConcurrentMap#remove to handle the concurrent entry loading/unloading, (4) entries in some pools are relatively cheap to construct, so the pool/map/cache acts more like an interning instance-sharing pool.

Unfortunately, Caffeine has about x5 overhead on such usage pattern comparing to ConcurrentLinkedHashMap. Some of our pools are on really hot paths, for example OOneEntryPerKeyLockManager, which is used by OAtomicOperationsManager, which is touched by almost any database operation.

At this moment, I don't see real benefits in switching from ConcurrentLinkedHashMap to Caffeine on our pools. What we really need here is a flexible performant concurrent pool implementation, not a cache. Maybe one day we will do a second try on Caffeine and pools, if it receives a feature like this.

@ben-manes
Copy link

Perhaps stormpot is a better fit. It's an object pool that utilizes thread locals to improve acquisition time, with a concurrent queue for management.

If you have a benchmark (e.g. jmh) then I can investigate the slowdown.

@taburet
Copy link
Contributor

taburet commented Jan 25, 2017

Thanks for Stormpot, seems like that is a great thing. But it doesn't have mapping/interning semantics and seems like there is no cheap and easy way of introducing it. It will be a great fit for pure pools maybe with some kind of instance attach-detach semantics on top of it.

Regarding JMH benchmark, I will create one on spare time and let you know.

@robfrank robfrank modified the milestones: 3.0, 3.0-M1 Apr 12, 2017
@lvca lvca modified the milestone: 3.0-M1 Aug 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants