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
fix lru hash map #88
fix lru hash map #88
Conversation
What is this fix about? Also, have you implemented an LRU cache from scratch? any reason why you didn't use other implementations such as http://www.codeproject.com/Articles/23396/A-High-Performance-Multi-Threaded-LRU-Cache ? |
my previous implementation was wrong, this is not implemented from scratch, and its working. |
@eladmarg can you point us to the tests that are being fixed by this PR? |
I see one test in the PR request, but the comment referenced plural tests so want to make sure we look at them all. |
currently only TestHashMap |
To be frank my main concern is with the actual implementation - the Java version is relying on a JVM implementation, and if we are to write our own we should at least try and get other proven ones |
Itamar, this is proved one, the previous was mine implementation, and it was wrong. |
can it be merged? |
I want to review this properly and see what our options are for trusted LRU implementations, not going to merge for now until we get it right for sure. |
No description provided.