EHC ❯ Every Cache.put does a containsKey check
-
Bug
-
Status: Closed
-
2 Major
-
Resolution: Fixed
-
ehcache-core
-
-
asingh
-
Reporter: asingh
-
February 11, 2010
-
0
-
Watchers: 2
-
May 21, 2010
-
February 25, 2010
Attachments
Description
Every cache.put() does a check on containsKey() to notify listeners whether it was a put for new mapping or an update of an existing mapping in the cache. When cache is clustered with terracotta, doing a containsKey for each put is costly in serialization mode (Keys are generated in ClusteredStore for each put, containsKey by serialization/deserialization in ClusteredStore.generatePortableKey). Each put results in generating the keys twice for each put (if not using localKeyCache).
Store.put() right now returns void. We can modify Store.put() to return a boolean to indicate whether it was an update or a put of a new mapping and use this return value instead of doing containsKey every time.
Comments
Abhishek Singh 2010-02-11
Steve Harris 2010-02-11
Do we have time to get this in?
Tim Eck 2010-02-11
I’d have to look at the code, but from the way it is described it also sounds like a race condition. Can two thread compete to insert the new element mapping and both call the listeners saying it was a new entry? If so, I’d open another bug and link it
Saravanan Subbiah 2010-02-16
I would do this for both coherent and incoherent mode, since it will benefit both cases. We could just make memoryStore.put() return a boolean instead of void which tells us if its a new entry or an update to an existing entry and use that to call updateUpdateStatistics(). Of course it means changing the interface to memoryStore but this simple change gives us 20-30% improvement, then I think it is worth it.
As for Tim’s question, the current code might still race even with coherent memoryStore because the check and update is done outside the lock, but if we change it to depend on memoryStore.put() then that will also get fixed.
Abhishek Singh 2010-02-18
Committed changes (ehcache-core rev-1892, tim-ehcache rev-20654). Some numbers for multiple runs of same test, non-express mode, 1 node, 64 threads, serialization mode: Cluster TPS: 3571.4 Cluster TPS: 3614.5
So seems the perf improvements is still valid.
See previous comments for numbers before the commit.
Abhishek Singh 2010-02-25
Fixed
Attached patch to avoid containsKey check when in incoherent mode. Running non-express, 1 node, 64 threads, serialization mode, some numbers: Before patch: Cluster TPS: 2752.3
After patch: Cluster TPS: 3370.8