Using ConcurrentHashMap, when is synchronizing necessary?

Question

I have a ConcurrentHashMap where I do the following:

sequences = new ConcurrentHashMap<Class<?>, AtomicLong>();

if(!sequences.containsKey(table)) {
    synchronized (sequences) {
        if(!sequences.containsKey(table))
            initializeHashMapKeyValue(table);
    }
}

My question is - is it unnecessary to make the extra

if(!sequences.containsKey(table))

Check inside the synschronized block so other threads wont initialize the same hashmap value?

Maybe the check is necessary and I am doing it wrong? It seems a bit silly what I'm doing, but I think it is necessary.

Answer 1

All operations on a ConcurrentHashMap are thread-safe, but thread-safe operations are not composable. You trying to make atomic a pair of operations: checking for something in the map and, in case it's not there, put something there (I assume). So the answer to your questions is yes, you need to check again, and your code looks ok.

Answer 2

You should be using the putIfAbsent methods of ConcurrentMap.

ConcurrentMap<String, AtomicLong> map = new ConcurrentHashMap<String, AtomicLong> ();

public long addTo(String key, long value) {
  // The final value it became.
  long result = value;
  // Make a new one to put in the map.
  AtomicLong newValue = new AtomicLong(value);
  // Insert my new one or get me the old one.
  AtomicLong oldValue = map.putIfAbsent(key, newValue);
  // Was it already there? Note the deliberate use of '!='.
  if ( oldValue != newValue ) {
    // Update it.
    result = oldValue.addAndGet(value);
  }
  return result;
}

For the functional purists amongst us, the above can be simplified (or perhaps complexified) to:

public long addTo(String key, long value) {
    return map.putIfAbsent(key, new AtomicLong()).addAndGet(value);
}

And in Java 8 we can avoid the unnecessary creation of an AtomicLong:

public long addTo8(String key, long value) {
    return map.computeIfAbsent(key, k -> new AtomicLong()).addAndGet(value);
}

Answer 3

You can't get exclusive lock with ConcurrentHashMap. In such case you should better use Synchronized HashMap.

There is already an atomic method to put inside ConcurrentHashMap if the object is not already there; putIfAbsent

Answer 4

I see what you did there ;-) question is do you see it yourself?

First off all you used something called "Double checked locking pattern". Where you have fast path (first contains) which does not need synchronization if case it is satisfied and slow path which must be synchronized because you do complex operation. Your operation consists of checking if something is inside the map and then putting there something / initializing it. So it does not matter that ConcurrentHashMap is thread safe for single operation because you do two simple operations which must be treated as unit so yes this synchronized block is correct and actually it could be synchronized by anything else for example this.

Answer 5

In Java 8 you should be able to replace the double checked lock with .computeIfAbsent:

sequences.computeIfAbsent(table, k -> initializeHashMapKeyValue(k));

Answer 6

Create a file named dictionary.txt with the following contents:

a
as
an
b
bat
ball

Here we have: Count of words starting with "a": 3

Count of words starting with "b": 3

Total word count: 6

Now execute the following program as: java WordCount test_dictionary.txt 10

public class WordCount {
String fileName;

public WordCount(String fileName) {
    this.fileName = fileName;
}

public void process() throws Exception {
    long start = Instant.now().toEpochMilli();

    LongAdder totalWords = new LongAdder();
    //Map<Character, LongAdder> wordCounts = Collections.synchronizedMap(new HashMap<Character, LongAdder>());
    ConcurrentHashMap<Character, LongAdder> wordCounts = new ConcurrentHashMap<Character, LongAdder>();

    Files.readAllLines(Paths.get(fileName))
        .parallelStream()
        .map(line -> line.split("\\s+"))
        .flatMap(Arrays::stream)
        .parallel()
        .map(String::toLowerCase)
        .forEach(word -> {
            totalWords.increment();
            char c = word.charAt(0);
            if (!wordCounts.containsKey(c)) {
                wordCounts.put(c, new LongAdder());
            }
            wordCounts.get(c).increment();
        });
    System.out.println(wordCounts);
    System.out.println("Total word count: " + totalWords);

    long end = Instant.now().toEpochMilli();
    System.out.println(String.format("Completed in %d milliseconds", (end - start)));
}

public static void main(String[] args) throws Exception {
    for (int r = 0; r < Integer.parseInt(args[1]); r++) {
        new WordCount(args[0]).process();
    }
}

}

You would see counts vary as shown below:

{a=2, b=3}

Total word count: 6

Completed in 77 milliseconds

{a=3, b=3}

Total word count: 6

Now comment out ConcurrentHashMap at line 13, uncomment the line above it and run the program again.

You would see deterministic counts.