Using Java ReadWriteLock to synchronise cached data - whether to mark state variables volatile?

Question

The ReadWriteLock javadoc at Oracle and its implementation describe what the lock does and how to use it but doesn't say anything about whether to use the volatile keyword.

This is not the same question as do-all-mutable-variables-need-to-be-volatile-when-using-locks because I'm happy that the lock will synchronise access and visibility properly, but is the use of volatile for the variables still a good idea, e.g. for compiler optimisations or any other reasons?

My cached data consists of a rarely changed List and several Maps mapping the objects in the list using various attributes of the objects.

private void reload() {
    Set<Registration> newBeans = dao.listRegistered();
    beans = Collections.unmodifiableSet(newBeans);
    codeToBeanMap.clear();
    userToBeanMap.clear();
    nameToBeanMap.clear();
    idToBeanMap.clear();
    for (Registration bean : newBeans) {
        codeToBeanMap.put(bean.getCode(), bean);
        userToBeanMap.put(bean.getUser(), bean);
        nameToBeanMap.put(bean.getName(), bean);
        idToBeanMap.put(bean.getId(), bean);
    }
}

What would be the best declarations? I have this:

private Set<Registration> ecns;
private final Map<String, Registration> codeToBeanMap =
        new HashMap<String, Registration>();
private final Map<String, Registration> userToBeanMap =
        new HashMap<String, Registration>();
private final Map<String, Registration> nameToBeanMap =
        new HashMap<String, Registration>();
private final Map<String, Registration> idToBeanMap =
        new HashMap<String, Registration>();

score 1 · Answer 1 · answered Feb 03 '15 at 11:14

1

If you are going to access a variable exclusively from code guarded by a lock, then it would be redundant to specify that variable as volatile and would incur a certain performance hit (it's in the nanosecond range).

answered Feb 03 '15 at 11:14

Marko Topolnik

195,646
29
319
436

score 1 · Answer 2 · answered Feb 03 '15 at 11:19

Your declarations like this are fine for concurrency:

private final Map<String, Registration> idToBeanMap = new HashMap<>();

... because Java guarantees that fields declared final will be initialised before other threads can access them (Java 1.5 onwards, at least). The volatile keyword is not needed (and makes no sense with final anyway).

However, this says nothing about the contents of the HashMap - this would not be thread safe and different threads may see different or inconsistent content unless you use synchronized code blocks. Your simplest solution would be to use the ConcurrentHashMap instead. This would guarantee that all threads would see the expected Map content.

However, this only means operations on the maps would be atomic. In your code above it is possible you might have cleared one map but not the others (for example) at the point another thread tries to read the data which might give inconsistent results.

So, because it looks like your code requires multiple Maps to remain in sync and consistent with each other the only way is to make your reload method synchronized and also make clients reading the data do so through a synchronized method on the same object.

Good. That's in agreement with the `ReentrantReadWriteLock` javadoc, which contains an example which is exactly how I used it. The main difference is that I didn't declare the primary `Set` final, because I wanted to assign it from `Collections.unmodifiableSet(setFromDao)` on each reload. This looks like it might be a big issue - how to declare a set final and yet update it. — Adam, Feb 03 '15 at 15:55

score 0 · Answer 3 · answered Feb 03 '15 at 11:16

but is the use of volatile for the variables still a good idea, e.g. for compiler optimisations or any other reasons?

If anything volatile prevents compiler optimizations instead of enabling them.

The ReadWriteLock javadoc at Oracle and its implementation describe what the lock does and how to use it but doesn't say anything about whether to use the volatile keyword.

It does, in fact if you look at the interfaces that the read and write locks implement, specifically the Lock interface states that implementations must provide the same memory visibility guarantees as the synchronized() block:

All Lock implementations must enforce the same memory synchronization semantics as provided by the built-in monitor lock, as described in section 17.4 of The Java™ Language Specification:

A successful lock operation has the same memory synchronization effects as a successful Lock action.

A successful unlock operation has the same memory synchronization effects as a successful Unlock action.

Using Java ReadWriteLock to synchronise cached data - whether to mark state variables volatile?

3 Answers3