ConcurrentHashMap put vs putIfAbsent

caching, concurrency, hashmap, java

Solution

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;
    }
}

Problem

Java Docs says that, `putIfAbsent` is equivalent to ``` if (!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); } ```

Original source