Calling get() and put() method of Hashmap correctly.

Question

for the code below:

public ReentrantReadWriteLock getLock(String tableName) {
    ReentrantReadWriteLock lock = locksMap.get(tableName);
    if (lock == null) {
        lock = new ReentrantReadWriteLock();
        locksMap.put(tableName, lock);
    }
}

//where locksMap is a HashMap with key String (tableName) and value ReentrantReadWriteLock (Lock).

My question is, if threads are accessing this method simultaneously, they will get different Lock objects with the same "tableName", because the get and put methods of the Map are called separately.

Any solutions with explanation will be appreciated? Thanks in advance.

Perhaps use ConcurrentHashMap? https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ConcurrentHashMap.html — Matt Coubrough, Nov 24 '14 at 19:55
ConcurrentHashMap has a putIfAbsent() atomic method. Or even better since Java 8: an atomic computeIfAbsent() method. — JB Nizet, Nov 24 '14 at 19:57
@njzk2 thanks, thought of using synchronized block, but not sure what is more efficient, using a ConcurrentHashMap or synchronized block ? — Script_Junkie, Nov 24 '14 at 19:59
You may want a [Guava `Striped`](http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/util/concurrent/Striped.html), which can be more efficient here... — Louis Wasserman, Nov 24 '14 at 20:24
Just a general comment, you might be better off using a "table" object instance which holds the lock and other stuff. Then you can pass that around and only need to map.get() it once (and not for every operation including table locks). — eckes, Nov 24 '14 at 20:42

William Price · Accepted Answer · 2014-11-24T20:19:02.597

Using a ConcurrentMap will usually generate better performance than a synchronized block.

Java 5-7:

ConcurrentMap<String, ReadWriteLock> lockMap = new ConcurrentHashMap<>();

ReadWriteLock getLock(String key) {
    ReadWriteLock lock = lockMap.get(key);
    if (lock == null) {
        lock = new ReentrantReadWriteLock();
        ReadWriteLock other = lockMap.putIfAbsent(key, lock);
        if (other != null) {
            // another thread's putIfAbsent won
            lock = other;
        }
    }
    return lock;
}

Java 8+:

ConcurrentMap<String, ReadWriteLock> lockMap = new ConcurrentHashMap<>();

ReadWriteLock getLock(String key) {
    return lockMap.computeIfAbsent(key, ReentrantReadWriteLock::new);
}

For one, an implementation like ConcurrentHashMap is documented to not use locks on read operations. So in this case, where it appears that you intend to get the lock for a single key many more times than you intend to create new locks, that will reduce thread contention. If you used synchronized, even if the lock was already created, you're forcing each and every thread to go single file through the critical section.

Also, implementations can do more advanced forms of locking, or even shard locks so that two writers don't necessarily block each other (if writing to different partitions of the underlying data structure). Again, synchronized uses a single monitor object and can't benefit from knowing the details of the underlying data structure.

The Java 8 version becomes a one-liner thanks to lambdas and function references. The ::new syntax refers to the public, no-arg constructor of the adjoining ReentrantReadWriteLock class. The computeIfAbsent method will only invoke that constructor if necessary and basically does all of the boilerplate work in the Java 7 version above for you. This is particularly useful if the cost of creating the new object is expensive or has unfortunate side effects. Note that the Java 7 version has to create a new lock instance under certain circumstances and that new object might not ever be used/returned.

Agreed. If using Java 8, using computeIfAbsent() is muuuuch simpler: `ReadWriteLock lock = map.computeIfAbsent(key, ReentrantReadWriteLock::new)` — JB Nizet, Nov 24 '14 at 20:10

score 0 · Answer 2 · answered Nov 24 '14 at 20:00

0

Typically, you would use synchronization to accomplish this. The simplest form of this would be to synchronize the method itself.

public synchronized ReentrantReadWriteLock getLock(String tableName) {

However, if you are concerned about performance I would consider the following approach which uses a synchronized block, but only if the initial lock isn't found.

public ReentrantReadWriteLock getLock(String tableName) {
    ReentrantReadWriteLock lock = locksMap.get(tableName);

    if (lock != null) {
        return lock;
    }

    synchronized(locksMap) {
        lock = locksMap.get(tableName);
        if (lock == null) {
            lock = new ReentrantReadWriteLock();
            locksMap.put(tableName, lock);
        }
    }
    return lock;
}

answered Nov 24 '14 at 20:00

skimbleton

239
1
2
11

The second method is not thread safe, because the JVM can optimize the two `lock = locksMap.get(tableName)` instructions. – gaborsch Nov 24 '14 at 20:02
You're reading and writing cocurrently to a HashMap, that is not thread-safe. – JB Nizet Nov 24 '14 at 20:05
I agree that the first read of the Map object is not thread safe. However, the second read, which is inside the synchronized block, is ensured to happen for only one thread at a time. I'm unaware of the JVM performing any optimizations that would ever conflict with this type of programming, but would love to hear more about it if that's the case :). – skimbleton Nov 24 '14 at 20:10
@skimbleton Have a look at this question, it will give you some insight: http://stackoverflow.com/questions/14624365/immutability-and-reordering – gaborsch Nov 24 '14 at 20:11

score 0 · Answer 3 · answered Nov 24 '14 at 20:01

You can either modify the method to be synchronized or add a synchronization block inside the method, encompassing both the get() and put() calls. Note there is a pseudo-pattern (I prefer to call it an idiom) about this, called Double-checked Locking.

Another option is to use a ConcurrentMap, which offers the putIfAbsent() method.

Note that you will run into lots of discussions/debates about performance of the various options. I encourage you to read them with a healthy grain of salt. Micro-optimizations and performance analysis is dangerous territory, and often the readability and maintainability of code far outweighs a few microseconds.

Calling get() and put() method of Hashmap correctly.

3 Answers3

Java 5-7:

Java 8+: