HashMultimap concurrent update

[kents...@gmail.com]

Hi,

I was searching any existing implementation of a thread safe Map of Sets : Map<K, Set<V>> and finally I reached HashMultimap.

It looks exactly what I want but one thing I feel very confusing.

In the api doc, it says :

 

Concurrent read operations will work correctly. To allow concurrent update operations, wrap your multimap with a call to Multimaps.synchronizedSetMultimap(com.google.common.collect.SetMultimap<K, V>).

It's great that concurrent read work perfectly and concurrent update need wrap the map with a call to synchronizedSetMultimap() which calls synchronizedMultimap().

In the comment under Multimaps.synchronizedMultimap(com.google.common.collect.SetMultimap<K, V>) :

It is imperative that the user manually synchronize on the returned multimap when accessing any of its collection views:

 Multimap<K, V> multimap = Multimaps.synchronizedMultimap(
 HashMultimap.<K, V>create());
 ...
 Collection<V> values = multimap.get(key); // Needn't be in synchronized block
 ...
 synchronized (multimap) { // Synchronizing on multimap, not values!
 Iterator<V> i = values.iterator(); // Must be in synchronized block
 while (i.hasNext()) {
 foo(i.next());
 }
 }


It is confused that we need to synchronize the map when ACCESSING any of its collection views.

Seems the comment above is a modified version of Collections.synchronizedMap(java.util.Map):

It is imperative that the user manually synchronize on the returned map when iterating over any of its collection views:

 Map m = Collections.synchronizedMap(new HashMap());
 ...
 Set s = m.keySet(); // Needn't be in synchronized block
 ...
 synchronized (m) { // Synchronizing on m, not s!
 Iterator i = s.iterator(); // Must be in synchronized block
 while (i.hasNext())
 foo(i.next());
 }

It says "user manually synchronize the map when ITERATING over any of its collection views".

So is it just a misprint in comment under Multimaps.synchronizedMultimap() (should be "iterating" but not "accessing") or really need to synchronize the map when ACCESSING the backing HashMultimap?

Regards,

Ken

[Luke Sandberg]

I think that this doc : "Concurrent read operations will work correctly. To allow concurrent update operations, wrap your multimap with a call to Multimaps.synchronizedSetMultimap(com.google.common.collect.SetMultimap<K, V>)" is misleading.

what it is really saying is that, it is safe for multiple threads to read the collection assuming that all mutations have been safely published first.  It is _not_ safe to mutate the multimap and read it concurrently.  So if you need to mutate the map at all, then all accesses must be guarded.   These are the same semantics that most jdk collections have (e.g. ArrayList/HashMap) are the same way.

--
guava-...@googlegroups.com
Project site: https://github.com/google/guava
This group: http://groups.google.com/group/guava-discuss
 
This list is for general discussion.
To report an issue: https://github.com/google/guava/issues/new
To get help: http://stackoverflow.com/questions/ask?tags=guava
---
You received this message because you are subscribed to the Google Groups "guava-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to guava-discus...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/guava-discuss/1a259466-f3f2-44e3-8f02-8ecbca5fcbe9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Justin Sampson]

I think that Ken's question is a more subtle one, about "accessing" vs. "iterating". You can call "get" on a synchronized map without wrapping it in a "synchronized" block, because the method itself is synchronized by the synchronizing wrapper class -- but since iteration happens outside an actual method on the map, the entire iteration needs to be explicitly guarded. So, the question is, are there ways of accessing a synchronizedSetMultimap, other than iteration, that also need to be explicitly guarded by a "synchronized" block?

 

Cheers,

Justin

 

P.S. I had to resend this because somehow "reply all" didn't include guava-discuss the first time.

To view this discussion on the web visit https://groups.google.com/d/msgid/guava-discuss/CAEu%3D3Gkn1yN97SqKETfSrmYccQPc4Bi6UTOKwsShYYun8zzxVw%40mail.gmail.com.