ConcurrentHashMap put vs putIfAbsent
Solution 1
So it doesnt update a key's value. is this correct?
That is correct. It will return the current value that was already in the Map.
would this be a better impl for adding and updating cache?
A couple things would make your implementation better.
1. You shouldn't use putIfAbsent to test if it exists, you should only use it when you want to ensure if one does not exist then putIfAbsent
. Instead you should use map.get
to test it's existence (or map.contains).
V local = _cache.get(key);
if (local.equals(value) && !local.IsExpired()) {
return;
}
2. Instead of put you will want to replace, this is because a race condition can occur where the if
can be evaluated as false by two or more threads in which one of the two (or more) threads will overwrite the other thread's puts.
What you can do instead is replace
When all is said and done it could look like this
public void AddToCache(T key, V value) {
for (;;) {
V local = _cache.get(key);
if(local == null){
local = _cache.putIfAbsent(key, value);
if(local == null)
return;
}
if (local.equals(value) && !local.IsExpired()) {
return;
}
if (_cache.replace(key, local, value))
return;
}
}
Solution 2
Your code will throw an NPE if the key was not previously in the map.
Other than that, although this is a reasonable idea, it will not work in a "concurrent" environment. The reason the putIfAbsent()
method was added was so that the map could manage the atomicity of the operation using whatever underlying support it is using to make the operations thread-safe. In your implementation, 2 different callers could end of stepping on each other (the first replaces an expired value with a new one, and the second immediately replaces the first new one with a second new one).
DarthVader
Updated on July 09, 2022Comments
-
DarthVader almost 2 years
Java Docs says that,
putIfAbsent
is equivalent toif (!map.containsKey(key)) return map.put(key, value); else return map.get(key);
So if the key exists in the map, it doesn't update its value. Is this correct?
What if i want to update a keys value based on some criteria? Say expiration time etc.
Would this be a better impl for adding and updating cache?
public void AddToCache(T key, V value) { V local = _cache.putifabsent(key, value); if(local.equals(value) && local.IsExpired() == false){ return; } // this is for updating the cache with a new value _cache.put(key, value); }
-
jtahlborn about 12 yearsyou should return if replace() returns true.
-
jtahlborn about 12 yearsalso, you don't need to call get() again if putIfAbsent() returns non-null, you can use what you just got. you only need a second get() call if replace fails.
-
jezg1993 about 12 years@jtahlborn I was thinking about cleaning that up, i'll put something in.
-
jezg1993 about 12 yearsIt could have been swapped out for
while(true){
But what you are trying to do is an atomic update. There is the likelyhood of a race condition in which an update to the current value in the cache (as a result of multiple puts at the same time) can fail. Thefor(;;){
simple says "continue trying until you succeed". for(;;) simply means, loop forever. -
jezg1993 about 12 yearsYou can see if the
_cache.replace(key,local,value)
fails it will try again until a return statement is reached. -
Admin almost 9 yearsThe fact about the concurrent situation cannot be pointed out often enough. :) +1