7

I am declaring a Java Map as 

Map<String, String> map = Collections.synchronizedMap(new HashMap<String, String>());

to deal with the concurrency issues, and synchronizing on the map for all the operations on it. However, I read that synchronization isn't necessary on a synchronizedMap when the operations are atomic. I checked the Java API and the documentation of HashMap doesn't seem to mention which are atomic, so I'm not sure which are.

I'm synchronizing on the following calls to the map:

map.size()

map.put()

map.remove()

map.get()

But if some are atomic, it seems synchronization isn't necessary for these. Which are atomic?

La-comadreja
  • 5,627
  • 11
  • 36
  • 64

3 Answers3

11

A synchronized map as the name suggests is synchronized. Every operation on it is atomic in respect to any other operation on it.

You can think of it as if every method of your synchronized map is declared with a synchronized keyword.

Please bear in mind that although individual operations are atomic, if you combine them they're no longer atomic, for instance:

String value = map.get("key");
map.put("key", value+"2");

is not equivalent to your custom synchronized code:

synchronized (map) {
    String value = map.get("key");
    map.put("key", value+"2");
}

but rather:

synchronized (map) {
    String value = map.get("key");
}
synchronized (map) {
    map.put("key", value+"2");
}
ciamej
  • 6,918
  • 2
  • 29
  • 39
  • I guess the difference between this and http://stackoverflow.com/questions/567068/java-synchronized-block-vs-collections-synchronizedmap is that in the other post, the other person who asked the question wanted to synchronize multiple methods. – La-comadreja Jul 02 '14 at 20:45
  • Note also that the operations are properly synchronized only when ANY access to the map is made via the synchronized wrapper. If you called any method of the inner HashMap (the one passed to the `Collections.synchronizedMap`) directly, it would not only ceased to be atomic but could result in undefined behavior. – Marwin Jul 02 '14 at 20:51
  • @Marwin Fortunately the OP is not saving a reference to the inner map, though even if he would I'd argue that using it directly would result in undefined behavior. That would be true only if two concurrent accesses to the map were issued through the inner map and the synchronized wrapper. – ciamej Jul 02 '14 at 20:55
  • @ciamej Thanks for your clarification. That's what I meant but did not explicitely say. I somehow supposed there is some concurrent access when we talk about atomicity of operations. – Marwin Jul 02 '14 at 20:59
  • What you are saying is that "thread safety" (whatever that actually means) is not _composeable_. You can't make a program thread safe merely by insuring that every method within it is thread safe. Thread safety (whatever that actually means) is a holistic property. – Solomon Slow Jul 02 '14 at 21:59
  • @jameslarge I didn't use the term "thread safe" at all. I said "atomic". Btw: atomicity *is* composable, but not simply by guaranteeing that every element is atomic. – ciamej Jul 03 '14 at 00:25
  • 1
    Perhaps I was using the wrong word. Maurice Herlihy and Nir Shavit use the word "compositional" in their book, "The Art of Multiprocessor Programming." They say, "A correctness property P is _compositional_ if, whenever each object in the system satisfies P, the system as a whole satisfies P." – Solomon Slow Jul 03 '14 at 13:33
6

A HashMap is not guaranteed to have atomic operations. Calling any of its methods from different threads (even size()) may corrupt the map. However, a map obtained using Collections.synchronizedMap will have each call synchronized (and hence thread-safe).

However, you may need higher-level synchronization. For instance, if you test whether a key is present, read the size, or otherwise access something from the map and then do something else with the map based on the result, the map may have changed between the two calls. In that case, you need a synchronized block to make the entire transaction atomic, rather than a synchronized map (that just makes each call atomic).

Ted Hopp
  • 232,168
  • 48
  • 399
  • 521
  • I'm only making one call to a map function per method invocation, with each map function call independent of the previous ones. So I'm in the clear, right? – La-comadreja Jul 02 '14 at 21:01
  • 1
    @La-comadreja - From what you describe, you should be. But it all depends on whether you need to treat a sequence of method invocations as a unit. If so, then for those you need to use `synchronized` blocks instead of (or in addition to) synchronized map functions. – Ted Hopp Jul 02 '14 at 21:04
  • thanks for your thoughtful answer. Each of the methods the Map is in is a distinct unit, rather than multiple methods belonging to a sequential unit. – La-comadreja Jul 02 '14 at 21:07
4

The map itself is synchronized, not some internal locks. Running more than one operation on the map does require a synchronized block. In any event, if you are using a JDK 1.6 or greater, you should consider using ConcurrentHashMap

ConcurrentHashMap is optimal when you need to ensure data consistency, and each of your threads need a current view of the map. If performance is critical, and each thread only inserts data to the map, with reads happening less frequently, then use the path you've outlined. That said, performance may only be poorer when only a single thread accesses a ConcurrentHashMap at a time, but significantly better when multiple threads access the map concurrently.

Keith
  • 3,079
  • 2
  • 17
  • 26