Java readwrite concurrency: readwrite lock vs lock + volatile

Question

Assuming that readers = 10 x writers, which of the following solutions is better in terms of throughput. Which solution is better to use in production code?

Using single lock for set operation and volatile variable for get operation

import java.util.List;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

public final class InstanceProvider {

    private final Lock wLock = new ReentrantLock();
    private volatile List<Instance> instances;

    public void setInstances(List<Instance> newInstances) {
        wLock.lock();
        try {
            doSmthWith(newInstances);
            instances = newInstances;
        } finally {
            wLock.unlock();
        }
    }

    public List<Instance> getInstances() {
        return instances;
    }

    @Getter
    public static final class Instance {

        private final String name;

        public Instance(String name) {
            this.name = name;
        }

    }
}

Using single read-write lock for both set and get operations

import java.util.List;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

public final class InstanceProvider {

    private final ReadWriteLock lock = new ReentrantReadWriteLock();
    private List<Instance> instances;

    public void setInstances(List<Instance> newInstances) {
        lock.writeLock().lock();
        try {
            doSmthWith(newInstances);
            instances = newInstances;
        } finally {
            lock.writeLock().unlock();
        }
    }

    public List<Instance> getInstances() {
        lock.readLock().lock();
        try {
            return instances;
        } finally {
            lock.readLock().unlock();
        }
    }

    @Getter
    public static final class Instance {

        private final String name;

        public Instance(String name) {
            this.name = name;
        }

    }
}

My assumption is that reading volatile is cheaper(basically it will be cached most of the time) and locks require additional instructions to be executed

The lock accomplishes nothing; volatile is all you need here. — shmosel
– shmosel, Commented Jun 4, 2024 at 20:35
What does doSmthWith() do? Does it modify the list? Why does it need to be synchronized? — shmosel
– shmosel, Commented Jun 4, 2024 at 20:55
There’s the fundamental problem with this code that nothing guarantees that the list is immutable. The caller of setInstances has a reference to the list and so will every caller of getInstances(). Everyone could modify the list and hence, break everything. It’s also misleading to call this class InstanceProvider; it does not provide instances, it merely maintains a list of instances provided by someone else. — Holger
– Holger, Commented Jun 12, 2024 at 10:11

shmosel · Accepted Answer · 2024-07-09 09:21:43Z

Locking is useful for a few things:

Keeping operations atomic. E.g., an increment operation is technically composed of read, update, write. If another thread can access your field between read and write, you have a race condition that can result in data corruption.
Ensuring updates are visible. When you modify a field, there's no guarantee another thread will see those changes unless it synchronizes with yours.
Safe publication. When you assign an object to a field, unless the object is immutable, it may be seen by another thread in a partially-constructed state. As with the visibility concern, this can be prevented by synchronization.

In your scenario, none of these concerns are pertinent. The write operation (setInstances()) only updates a reference field, which is inherently an atomic operation. Visibility and safe publication are ensured by making the field volatile, which guarantees that writes are immediately and fully visible to subsequent reads.

So there's no need to lock at all. Just stick with volatile.

rzwitserloot · Accepted Answer · 2024-06-04 20:43:19Z

The write lock isn't needed either. If this is literally it and you don't care about ordering, then just make that field volatile and don't have any lock at all. This works because java guarantees that reference assignments are atomic.

You'd use locks when there are multiple steps (or a single step that does not guarantee atomicity), and you don't want anybody to ever read anything that is half-baked (in between steps).

That's why you 'get away' with no locking at all here. You just want to ensure that happens-before doesn't show up and imply that one thread can write to this field and the other threads take a day to 'see' that update, which the JMM allows a JVM to do if you also remove the volatile. So keep that around. Locking may still happen - the point is, the JVM decides, not you. The volatile ensures that writes to that field and reads to that field establish happens-before so that you 'see' it.

The point of a read/write lock is that multiple entities can read simultaneously without locking each other out, but, a write lock locks out all other writes and all other reads (and is locked by any open reads). You don't need any of that here.

Collectives™ on Stack Overflow

Java readwrite concurrency: readwrite lock vs lock + volatile

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related