I have a usecase where I have to
I've the following code to offer:
public void insertOrReplace(String key, String value) {
boolean updated = false;
do {
String oldValue = concurrentMap.get(key);
if (oldValue == null) {
oldValue = concurrentMap.putIfAbsent(key, value);
if (oldValue == null) {
updated = true;
}
}
if (oldValue != null) {
final String newValue = recalculateNewValue(oldValue, value);
updated = concurrentMap.replace(key, oldValue, newValue);
}
} while (!updated);
}
Do you think it's correct and thread-safe?
Is there a simpler way?
computeIfAbsent returns "the current (existing or computed) value associated with the specified key, or null if the computed value is null". putIfAbsent returns "the previous value associated with the specified key, or null if there was no mapping for the key".
synchronizedMap() requires each thread to acquire a lock on the entire object for both read/write operations. By comparison, the ConcurrentHashMap allows threads to acquire locks on separate segments of the collection, and make modifications at the same time.
ConcurrentHashMap is one of the most frequently used collection classes in our daily work, Its characteristic is high performance and thread-safety. However, there are two bugs that impact the performance of this familiar map. The problem is in computeIfAbsent .
The Java HashMap putIfAbsent() method inserts the specified key/value mapping to the hashmap if the specified key is already not present in the hashmap. Here, hashmap is an object of the HashMap class.
You could make it a little shorter with the code below which is equivalent to yours. I have stress tested it a little with thousands of threads accessing it concurrently: it works as expected, with a number of retries (loops) being performed (obviously, you can never prove correctness with testing in the concurrent world).
public void insertOrReplace(String key, String value) {
for (;;) {
String oldValue = concurrentMap.putIfAbsent(key, value);
if (oldValue == null)
return;
final String newValue = recalculateNewValue(oldValue, value);
if (concurrentMap.replace(key, oldValue, newValue))
return;
}
}
Your method seems thread safe. If you do not require the performance benefits of ConcurrentHashMap, consider using a regular HashMap instead and synchronize all access to it. Your method is similar to AtomicInteger.getAndSet(int), so it should be fine. I doubt there is an easier way to do this unless you're looking for a library call to do the work for you.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With