CODEWITHSUNDEEP
javamultithreadingconcurrencyhashmapjava.util.concurrent
Dongyu Jia
Dongyu Jia

Reputation: 437

How to handle concurrency in this case?

I have a HashMap

ConcurrentHashMap<String, Integer> count =new ConcurrentHashMap<String, Integer>();

I will use like this:

private Integer somefunction(){
    Integer order;
    synchronized (this) {
          if (count.containsKey(key)) {
            order = count.get(key);
            count.put(key, order + 1);
          } else {
            order = 0;
            count.put(key, order + 1);
          }
        }
    return order;
}

But as you can see, this may not be ideal to handle concurrency, since only value under the same key may interfere each other.Different key does't interfere each other so it's not necessary to synchronize all operation. I want to synchronize only when the key is the same.

Can I do something that can achieve better performance on concurrency? (I know ConcurrentHashMap and synchronize is kind of redundant here ,but let's focus on if we can only synchronize when key is same)

Upvotes: 4

Views: 138

Answers (5)

Dongyu Jia
Dongyu Jia

Reputation: 437

Here is how I do this,

private Integer somefunction(){
  Integer order = count.compute(key, (String k, Integer v) -> {
      if (v == null)
        return 1;
      else {
        return v + 1;
      }
    });
  return order－1;
}

This avoid keeps trying use replace(key,oldValue,newValue) Will this be better for concurrency?

The problem is that a lot of environment doesn't support jdk8 yet.

Upvotes: -1

markusk
markusk

Reputation: 6677

The whole point of ConcurrentHashMap is to facilitate concurrent operations. Here's how you can do an atomic update with no need for explicit synchronization:

private Integer somefunction() {
    Integer oldOrder;
    // Insert key if it isn't already present.
    oldOrder = count.putIfAbsent(key, 1);
    if (oldOrder == null) {
        return 0;
    }
    // If we get here, oldOrder holds the previous value.
    // Atomically update it.
    while (!count.replace(key, oldOrder, oldOrder + 1)) {
        oldOrder = count.get(key);
    }
    return oldOrder;
}

See the Javadocs for putIfAbsent() and replace() for details.

As Tagir Valeev points out in his answer, you can use merge() instead if you're on Java 8, which would shorten the code above to:

private Integer somefunction() {
    return count.merge(key, 1, Integer::sum) - 1;
}

Another option would be to let the values be AtomicInteger instead. See hemant1900's answer for how to do so.

Upvotes: 4

Tagir Valeev
Tagir Valeev

Reputation: 100239

It's very easy if you can use Java-8:

return count.merge(key, 1, Integer::sum)-1;

No additional synchronization is necessary. The merge method is guaranteed to be executed atomically.

Upvotes: 1

hemant1900
hemant1900

Reputation: 1226

I think this might be better and simpler -

private final ConcurrentHashMap<String, AtomicInteger> count = new ConcurrentHashMap<String, AtomicInteger>();

private Integer someFunction(String key){        
    AtomicInteger order = count.get(key);
    if (order == null) {
        final AtomicInteger value = new AtomicInteger(0);
        order = count.putIfAbsent(key, value);
        if (order == null) {
            order = value;
        }
    }
    return order.getAndIncrement();
}

Upvotes: 1

Nicholas Robinson
Nicholas Robinson

Reputation: 1419

First of all, where does key even come from?

Secondly, if key will never be the same for two threads running that function at any one time you don't need to synchronize any part of the function.

If, however, two threads could have the same key at the same time then you only need:

synchronized(count) {
   count.put(key, order + 1);
}

The reason for this is that only threaded mutation of an object variables will need to be synchronized. But the fact that you are using a ConcurrentHashMap should eliminate this problem (double check me on this), thus no synchronization is needed.

Upvotes: 0

Related Questions