[Geotools-devel] Contesting the utility of ObjectCacheEntry

[Martin D. <mar...@ge...>]

ObjectCacheEntry holds a ReadWriteLock. ReadWriteLock are used for example with 
HashMap, in order to allows multiple concurrent read operations (on different 
keys) while allowing only one write operation at time.

But with ObjectCacheEntry, ReadWriteLock are used on a entry-by-entry level, 
which seems useless to me. I understand that we want to lock on an 
entry-by-entry level (allowing concurrent operations while we are creating a 
CRS), but ObjectCacheEntry need only a write lock for this task.

ObjectCacheEntry is not in the "many read operations with few write operations" 
case like HashMap, since it can returns only one value. Every use cases I found 
in the code is of the form (directly or indirectly):

     Object foo = cache.get(key);  // Read lock here
     if (foo == null) {
         cache.writeLock(key);     // Write lock here
         foo = cache.peek(key);    // An other write lock here
         if (foo == null) {
             // Do some stuff
         }
     }

So:

1) Either the "foo" value was already cached (in which case
    the 'get' method perform a very short locking since it
    executes nothing more than "return value" - we could even
    use no lock at all for such a trivial task, since returning
    a single reference is already an atomic operation on JVM.

2) Either the "foo" value is already in process of being
    computed, in which case "get" will block anyway (or return
    "null" is not synchronized) and "peek" will block too.


I believe there is no need for ObjectCacheEntry holding a ReadWriteLock. This 
class could probably be completly removed in favor of (maybe) AtomicReference 
and ConcurrentHashMap.

	Martin