explanations of what java hash maps concurrency is for, get/set of the key or the value also?

Question

Say I store a object in a hashmap that has a atomiclong as property.

Now I want to put a key in the hashmap if it doesn't exist, and update the value if it does.

When I get the object using the key, I will increment the property which is an atomic long.

What I need to understand is, what operation is thread-safe here?

Psuedo code:

HashMap hm = new HashMap

if(hm.containsKey(key1)) {
  MyCounter counter = (MyCounter)hm.get(key1);
  counter.incrementAndGet();  
}
else {
  MyCounter newCounter = new MyCounter();
  newCounter.incrementAndGet();
  hm.put(key1, newCounter);
}

Please take a look at this: http://stackoverflow.com/questions/8567596/how-to-make-updating-bigdecimal-within-concurrenthashmap-thread-safe — Bhesh Gurung, Dec 19 '11 at 21:43
I don't understand why you say the code is not threadsafe. Since he's got the entire function synchronized, no other threads can get into the function until the current thread has exited, so I don't see why there would be an issue with having to guard the code after the map.get(). What am I missing? — , Jan 31 '12 at 16:28

score 3 · Accepted Answer · answered Dec 19 '11 at 22:03

3

I would use a synchronized block as that is much simpler.

Map<KeyType, AtomicInteger> map = ...

synchronized(map) {
   AtomicInteger count = map.get(key);
   if (count == null)
       map.put(key, count = new AtomicInteger());
   count.incrementAndGet();
}

The cost of synchronized is unlikely to be enough to make it worth complicating your solution.

answered Dec 19 '11 at 22:03

Peter Lawrey

525,659
79
751
1,130

but I have a reference to the object, so that should be thread-safe no? – codecompleting Dec 19 '11 at 22:09
One you have a single reference, its thread safe. But incrementing it is trivial (about 5 ns) Its getting the reference in a thread safe way that's tricky. – Peter Lawrey Dec 20 '11 at 08:01

score 0 · Answer 2 · answered Dec 19 '11 at 21:42

Your code is not thread safe.

There is an issue in the sequence where you check for absense/existence and adding/updating the value.

The action to check for existense of value in the HashMap and the update (or addition) of value should be "guarded".

I.e.

synchronized(hm){
  if(hm.contains(key1)){
   //update
  }
  else{
  //add
  }
}

explanations of what java hash maps concurrency is for, get/set of the key or the value also?

2 Answers2