Why is the UMDM using ConcurrentHashMap?

[jason.greene]

I noticed a change in trunk that switched UMDM from HashMap to ConcurrentHashMap.

Why was this change made?

The design of the UMDM is that writes are confined to single thread on startup, and all access after that point is read-only. Switching from HashMap to ConcurrentHashMap adds unnecessary locking.

-Jason

[heiko.braun]

The design of the UMDM is that writes are confined to single thread on startup, and all access after that point is read-only.

Exactly. That's the reason why i changed it. The ConcurrentHashMap does not entail locking on read operations. I think this is the desired behaviour or did i miss your point?

[jason.greene]

ConcurrentHashMap usually does not lock on reads with a successful result. However, if the entry does not exist, a lock is always held on the corresponding stripe. Along with the extra locking, it also adds more processing and memory overhead. All of this is of course needed to handle multiple concurrent writers (or the possibility of multiple concurrent writers).

However, since the UMD is written to only at the start, and only from one thread, no synchronization is needed, even in that single thread. So, since no locking is needed, there is no need for ConcurrentHashMap.

It could be argued though that ConcurrentHashMap would improve safety, since improper use of the UMD (a writer happens after eager initialization for example) would be slightly protected. However the problem is that this would only be protecting one of the many internal types within the UMD, so the overall access to it still can not allow for multiple writers, unless of course, it was redesigned to allow for the multiple writer scenario.

[heiko.braun]

AFAIK plain HashMap is holding a _map wide_ lock for any retrieval operation or iterator traversal. ConcurrentHashMap doesn't do it (well, at least more fine grained).

I am not looking at write operations. Performance tests showed that concurrent requests to the same service (shared meta data) suffered from synchronisation issues. That's why i replaced the plain HashMap.

[jason.greene]

You are thinking of HashTable. HashMap does not use locks at all.

[thomas.diesler]

Lets revisit this for 1.2.1

http://jira.jboss.org/jira/browse/JBWS-1516

[jason.greene]

The ConcurrentHashMap change was not merged to 1.2.0, and I left it out when i merged the log changes it was bundled with.

Anyway there is no way that ConcurrentHashMap could have reduced synchronization since it *adds* synchronization.

Removing synchronization was one of the big reasons for the Java collection types in 1.2. Only the pre 1.2 collection types synchronized all methods (Hashtable and Vector).

If you still don't believe me, read the javadoc
http://java.sun.com/j2se/1.5.0/docs/api/java/util/HashMap.html

[thomas.diesler]

Jas, please be forgiving with Heiko - he must have been on crack.