-
Type: Improvement
-
Resolution: Done
-
Priority: Major - P3
-
None
-
Affects Version/s: 2.6.1
-
Component/s: Performance
-
Labels:None
It seems that the synchronized block has been put in place in order to guarantee that a particular DBCollection is uniquely created and returned to the caller.
The synchronization is needed because the operation as a whole must be atomic. However, it adds an extra lock/unlock beyond that provided by synchronizedMap on _collections, and this can significantly negatively affect performance in highly concurrent situations which call this essential function.
One solution could be to not use synchronizedMap, and instead handle locking ourselves by always wrapping uses of _collections with synchronized blocks. This would reduce the number of blocking lock acquisitions.
However, a much better solution is for _collection to be ConcurrentHashMap. We would then never have to use synchronized blocks, and in this case could check the result of a putIfAbsent call:
MyCollection existing = _collections.putIfAbsent(name , c);
if (existing != null) c = existing;
This code would guarantee uniqueness of the collection, and offer much better throughput in high concurrency.