1

Will there be a performance hit if I synchronize a SynchronizedMap?

For eg:

private static Map<Integer, Integer> intMap= Collections.synchronizedMap(new HashMap<Integer, Integer>());

public static int doSomething(int mapId) {
  synchronized(intMap) {
    Integer id = intMap.get(mapId);
    if (id != null) {
       //do something
    }
    if (id == null) {
       intMap.put(mapId);
    }
  }
}

I have to synchronize explicitly in the above method since its a combination of operations on the synchronizedMap. Since I have to synchronize anyways, is it better to use normal hashmap instead of synchronizedMap?

Will there be a performance issue if I synchronize a synchronizedMap?

usha
  • 28,973
  • 5
  • 72
  • 93
  • `int id` cannot be `null`. I think you intended to write `Integer id` – Peter Lawrey Aug 08 '14 at 21:28
  • BTW synchronization can be as cheap as creating a new `Integer` object if the lock is uncontended. – Peter Lawrey Aug 08 '14 at 21:29
  • An interesting question. The anwers so far are basically just saying "Yes", but a proof or evidence would be nice here. I could imagine that a pattern like `sync(x) { sync(x) { foo(); } bar(); }` is actually optimized ("collapsed") to `sync(x) { foo(); bar(); }` because it should be possible to detect that the inner synchronization is superfluous. If nobody else does this, maybe I'll have a look at the bytecode or JITed code on my own later... – Marco13 Aug 08 '14 at 23:12

2 Answers2

0

Yes, there will be a performance hit. It may not be much, because the JVM is capable of optimizing these locks based on usage, but it will always be there.

There's no reason to use a synchronized map if you're already using the synchronized keyword on that object, unless you have code in other parts of your program that needs a synchronized Map.

markspace
  • 10,621
  • 3
  • 25
  • 39
0

Yes. You are doing unnecessary synchronization on an already synchronized collection. It will be just as safe and efficient (in fact more efficient) to remove the extra synchronized block.

Further, I performed a small test small-scale: https://gist.github.com/AgentTroll/02b77680efe3b92e350f

(Ignore the license, I had it on one of my projects on GitHub, it's there so people don't think I steal code)

  • I cannot remove the synchronized block because my method does more than one operation on the map. http://stackoverflow.com/questions/567068/java-synchronized-block-vs-collections-synchronizedmap – usha Aug 08 '14 at 21:42
  • You don't have a synchronization policy clearly documented, so it is difficult to see how your two operations are related to each other. –  Aug 08 '14 at 21:55