Sebastian Oleszczuk
Sebastian Oleszczuk

Reputation: 1

Synchronization on getting ConcurrentHashMap reference?

I started building an app: many threads reading same map (a ConcurrentHashMap) mainly to reading from many Threads, as I have read, many Threads can read ConcurentHashMap, with every one of them getting "snapshot" of what the map look like when the Thread acquaierd it. That is clear for me, but they need to get the reference of the map (the Threads). I was planning to do public method something alike:

public ConcurrentHashMap MAP;

public ConcurrentHashMap getMAP(){
    return this.MAP;
}

But should it be made as synchronized? Or should I made just map as public in declaration and get just like class.Map on this class providing the Map. (I've made some simplification to the code because I need more a concept answer. of course it would be instantiated I like constructor or something)

Ok now i know i need taking "snapshot" of the Map to get some computations done, so i will go with kind of synchronization, a lock. But another question came in my mind:

if i have let's say:

ConcurentHashMap<String,ConcurentHashMap<String,String>> OneMap;

Thread is getting one of ConcurentHashMap<>'s (InsideLevelMap) from OneMap by it's KEY, it's getting reference to this map only, reading from InsideLevelMap is still done concurently, read of one KEY/VALUE doesn't block writing to another one, and getting references of InsideLevelMap s is still going by it's own way? hope I didn't tumble me thoughts too much.

and another one: I need the Threads to read values and and only if value at this key has changed or it does not exist, then i need to write to the map. So i'm thinking of making map avaiable (as getter or by public field) for reading (checking existance of A KEY) and synchronized method for modyfying Map. but when synch method would be in use to chance Map, would then it will be locked for reading as whole objcect would get a Lock? am i right?

Thanks for the answers

Upvotes: 0

Views: 669

Answers (2)

Gray
Gray

Reputation: 116878

many Threads can read ConcurrentHashMap, with every one of them getting "snapshot" of what the map look like when the Thread acquired it.

This is at best misleading. ConcurrentHashMap is a reentrant class and any thread accessing it (i.e. making any get/put/etc calls) have to cross read/write memory barriers and locks as necessary. This means that they will not see a "snapshot" but will see the actual contents of the map at that point in time.

One place were you would need to lock on a CHM is when you are making multiple calls to it.

if (concurrentMap.containsKey(key)) {
    // possible race condition here
    concurrentMap.put(key, value);
}

In the above example, you are making 2 calls to the map and someone might have removed the key from the map between the calls. The map will protect itself and won't be corrupted but your code my suffer if it is doing the wrong thing. CHM has a number of putIfAbsent(...) and other methods designed to resolve race conditions with common multiple call patterns.

But should it be made as synchronized? Or should I made just map as public in declaration and get just like class.Map on this class providing the Map.

You certainly do not need to make it synchronized or volatile unless you are changing the instance of the map. I'd make sure to make your map field be final:

private final ConcurrentHashMap<...> map = new ConcurrentHashMap<>();

In terms of public versus a method, that is more about data hiding than anything else. I rarely if ever have a public field in my classes (except for constants) so a method is better than that. I'd make sure you were returning one of the interfaces Map or ConcurrentMap instead of the implementation class. Another idea would be to expose get/put/etc. methods on your own class instead of exposing the whole map to the outside.

reading from InsideLevelMap is still done concurently, read of one KEY/VALUE doesn't block writing to another one, and getting references of InsideLevelMap s is still going by it's own way?

I don't quite follow this but ConcurrentHashMap does the minimal locking and memory barriers from an efficiency standpoint. And the locks and volatile fields of the inner maps have nothing to do with those from the output map.

Threads to read values and and only if value at this key has changed or it does not exist, then i need to write to the map. So i'm thinking of making map available (as getter or by public field) for reading (checking existence of A KEY) and synchronized method for modifying Map. but when synch method would be in use to chance Map, would then it will be locked for reading as whole object would get a Lock? am i right?

Again I don't quite follow. I would add containsKey() method and a modify() or something method on your class to expose just those methods that you need to expose. You don't need the modify method to be synchronized.

Upvotes: 1

Kayaman
Kayaman

Reputation: 73558

Generally speaking no. Provided that you initialize the MAP once (for example in the declaration), and then just retrieve and use it. If you changed it you would need to make sure all threads see the updated value and get the right map.

Also yes, you should use a getter to get the map. Don't use public fields.

Upvotes: 0

Related Questions