Replaced synchronize's with ReentrantReadWriteLock & added contains() method #56
Conversation
This is a much faster way of checking if a given key exists in the cache
Travis CI was failing because it's very pedantic
This should be functionally the same as all of the synchronize statements, except read operations will not block each other. This gives us huge performance benefits for things like contains() and get() when under heavy load.
|
This is a 2nd attempt at addressing #54 |
|
Just read your code, read the other posts related to the motivation and I like the initiaive. Everything seems good but honestly I think we can do better without any locks at all as you're still at the mercy of deadlocks. Why not fix all the way by making it a Lock-Free data structure using Atomic instructions instead (java.util.concurrent.atomic.*), let's use the hardware instructions made to solve these issues and achieve a greater level of parallelism and performance. just saying ;) |
|
You're probably right. This is currently failing one of the unit testing on However, the read write lock implementation is still better that the
|
|
can you write a simple benchmark test to prove this performs better? |
|
Yes, I will also write a unit test for the new contains() method.
|
|
Other people who have forked from this master have also seen this test failure after not changing anything of substance. The method that is causing the assert also states that it "Returns the approximate total number of tasks that have ever been scheduled for execution" It looks to me like the test is meant to prove that only 1 evict was caused by the shrink, is there a better way we can test this? |
|
Closing this, there is another PR #59 that is from it's own branch, and contains unit tests for the new |
This PR removes all synchronized statements in favor of a
ReentrantReadWriteLock. This allows read operations to not block each other, but write operations to block everything (the way synchronized was working previously).With the addition of a simple
contains()method, it is now super cheap to check if an image exists in the cache.