synchronize a method by achieving better performance?

Question

I have a class that is being called by multiple threads on multi core machine. I want to make it thread safe.

add method will be called by multiple threads. And if key exists, just append the current value to new value otherwise just put key and value in the map.

Now to make it thread safe, I was planning to synchronize add method but it will destroy performance. Is there any better way by which we can achieve better performance without synchronizing add method?

class Test {
  private final Map<Integer, Integer> map = new ConcurrentHashMap<>();

  public void add(int key, int value) {
    if (map.containsKey(key)) {
      int val = map.get(key);
      map.put(key, val + value);
      return;
    }
    map.put(key, value);
  }

  public Object getResult() {
    return map.toString();
  }
}

Possible duplicate of [ConcurrentHashMap vs Synchronized HashMap](https://stackoverflow.com/questions/1291836/concurrenthashmap-vs-synchronized-hashmap) — admlz635, May 06 '19 at 21:01

Andreas · Answer 1 · 2019-05-06T21:16:20.057

but it will destroy performance

It likely wouldn't destroy performance. It will reduce it some, with further reduction if there is a high collision rate.

Is there any better way by which we can achieve better performance?

Yes, use merge() (Java 8+). Quoting the javadoc:

If the specified key is not already associated with a value or is associated with null, associates it with the given non-null value. Otherwise, replaces the associated value with the results of the given remapping function, or removes if the result is null.

Example:

public void add(int key, int value) {
    map.merge(key, value, (a, b) -> a + b);
}

Or using a method reference to sum(int a, int b) instead of a lambda expression:

public void add(int key, int value) {
    map.merge(key, value, Integer::sum);
}

@flash In Java 7 you do it with `synchronized` (yet another good reason to upgrade). — Andreas, May 06 '19 at 21:08

Not a JD · Accepted Answer · 2019-05-06T21:10:05.787

4

Use merge:

class Test {
    final Map<Integer, Integer> map = new ConcurrentHashMap<>();

    public void add(int key, int value) {
        map.merge(key, value, Integer::sum);
    }

    public Object getResult() {
        return map.toString();
    }
}

Java 7 solution if you absolutely can't use synchronized (or, you absolutely cannot lock explicitly):

class Test {
    final Map<Integer, AtomicInteger> map = new ConcurrentHashMap<>();

    public void add(int key, int value) {
        get(key).addAndGet(value);
    }

    private AtomicInteger get(int key) {
        AtomicInteger current = map.get(key);

        if (current == null) {
            AtomicInteger ai = new AtomicInteger();

            current = map.putIfAbsent(key, ai);

            if (current == null) {
                current = ai;
            }
        }

        return current;
    }

    public Object getResult() {
        return map.toString();
    }
}

edited May 06 '19 at 21:10

answered May 06 '19 at 20:55

Not a JD

1,864
6
14

what is merge doing behind the scene? And if we don't need to use merge then how can we do in old fashion way? – flash May 06 '19 at 20:56
https://github.com/frohoff/jdk8u-jdk/blob/master/src/share/classes/java/util/concurrent/ConcurrentHashMap.java#L1960 it's using CAS. "don't need to use merge" -> " Is there any better way by which we can achieve better performance without synchronizing add method?". That's how you do it – Not a JD May 06 '19 at 20:57
Yeah agreed but I was asking instead of using merge feature can we do it in Java 7 as well in old fashion way? – flash May 06 '19 at 21:00
If you are dead set on not using synchronized or any explicit look, see answer edit. – Not a JD May 06 '19 at 21:10

score 1 · Answer 3 · answered May 06 '19 at 21:10

synchronized causes a bottleneck only when you run an expensive operation holding a lock.
In your case by adding a synchronized you are doing:
1. check a hashmap for existence of a key
2. get the value mapped to that key
3. do an addition and put the result back to the hashmap.

All these operations are super cheap O(1) and unless you are using some strange pattern for the keys which are integers it should be very unlikely that you can get some degenerate performance due to collisions.

I would suggest if you can't use merge as the other answers point out, to just synchronize. You should be considered so much about performance only in critical hotpaths and after you have actually profiled that there is an issue there

synchronize a method by achieving better performance?

3 Answers3