ReentrantReadWriteLock performing very bad compared to ReentrantLock

Question

I have created 1000 threads to increment,1000 threads to decrement,1000 threads to read the value.

Each increment thread ,increases value by 25000 times.

Each decrement thread ,decreases value by 25000 times.

Each read thread ,reads the value by 50000 times.

so over all the operation is Read dominant.

The ReadLock is put while reading the value

and WriteLock is put for methods incrementing & decrementing the values.

Observed: ReentrantReadWriteLock takes around 13000 ms Lock takes around 3000 ms. Expected : ReentrantReadWriteLock to give much faster performance than ReentrantLock.

BTW: I personally think there is no need to have lock/synchronization while using getCounter method ( just reading the value )

import java.util.ArrayList;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

public class Main {
    public static void main(String[] args) throws InterruptedException {

        ArrayList<Thread> reads = new ArrayList<>();
        ArrayList<Thread> increments = new ArrayList<>();
        ArrayList<Thread> decrements = new ArrayList<>();
        Resources resources = new Resources();
        long start = System.currentTimeMillis();
        for (int i = 0; i < 1000; i++) {
            Thread read = new Read(resources);
            Thread increment = new Increment(resources);
            Thread decrement = new Decrement(resources);
            reads.add(read);
            increments.add(increment);
            decrements.add(decrement);
            read.start();
            increment.start();
            decrement.start();
        }
        for (int i = 0; i < 1000; i++) {
            reads.get(i).join();
            increments.get(i).join();
            decrements.get(i).join();
        }
        System.out.println(resources.getCounter());
        System.out.println(System.currentTimeMillis() - start);
    }

    private static abstract class UserThread extends Thread {
        protected Resources resources;

        public UserThread(Resources resources) {
            this.resources = resources;
        }

    }

    private static class Read extends UserThread {

        public Read(Resources resources) {
            super(resources);
        }

        public void run() {
            for (int i = 0; i < 50000; i++) {
                resources.getCounter();

            }

        }
    }

    private static class Increment extends UserThread {

        public Increment(Resources resources) {
            super(resources);
        }

        public void run() {
            for (int i = 0; i < 25000; i++) {
                resources.increment();

            }

        }
    }

    private static class Decrement extends UserThread {

        public Decrement(Resources resources) {
            super(resources);
        }

        public void run() {
            for (int i = 0; i < 25000; i++) {
                resources.decrement();

            }

        }
    }

    private static class Resources {

        private ReentrantReadWriteLock reentrantReadWriteLock = new ReentrantReadWriteLock();

        private ReentrantReadWriteLock.WriteLock writeLock = reentrantReadWriteLock.writeLock();
        private ReentrantReadWriteLock.ReadLock readLock = reentrantReadWriteLock.readLock();
        private ReentrantLock lock = new ReentrantLock();

        public int getCounter() {
            readLock.lock();
            try {
                return counter;
            } finally {
                readLock.unlock();
            }

        }

        private int counter = 0;

        public void increment() {
            writeLock.lock();
            try {
                counter++;
            } finally {
                writeLock.unlock();
            }
        }

        public void decrement() {
            writeLock.lock();
            try {
                counter--;
            } finally {
                writeLock.unlock();
            }
        }

    }

}

You say the code is "read dominated" but your increment and decrement threads add up to 50,000 operations, which is the same number of operations performed by your read thread. Plus, you have twice as many threads writing as you do reading. A `ReadWriteLock` works best when there's many reads and few writes. — Slaw, Oct 20 '19 at 09:32
Your threads are all 100% CPU-bound, i.e. no I/O. With threads like that, creating more threads than you have virtual cores (cores + hyperthreading) will actually run *slower*, because you're now forcing excessive context switching. Do you have 3000 virtual cores on your machine? — Andreas, Oct 20 '19 at 10:12

score 5 · Answer 1 · answered Oct 20 '19 at 09:28

These kinds of locks - read-write - are commonly optimized and suited for many readers and a single or a few writers. They often spin on ops, expecting the reads to be fast and writes to be very few. Further, they are optimized for fairness, or FIFO handling of requests to avoid threads being stalled.

You do exactly the opposite. You do many writers, which cause excessive spinning and other sophisticated approaches, suited for many-reads-few-writes scenarios.

Simple locks are simple. They just block all threads when ready, and no spinning occurs. Their drawback is that they cause an avalanche effect when they awake multiple threads to let them just sleep again.

score 1 · Answer 2 · answered Oct 20 '19 at 11:36

Thanks Nick and Slaw for pointing out,it is not read dominant. I made sure i have 100 increment,100 decrement and 1000 read threads.

Results cameout as expected. The output with ReentrantReadWriteLock is 300 ms and withLock is 5000 ms.

Here is the modified code

import java.util.ArrayList;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

public class Main {
    public static void main(String[] args) throws InterruptedException {

        ArrayList<Thread> reads = new ArrayList<>();
        ArrayList<Thread> increments = new ArrayList<>();
        ArrayList<Thread> decrements = new ArrayList<>();
        Resources resources = new Resources();
        long start = System.currentTimeMillis();
        for (int i = 0; i < 100; i++) {
            Thread increment = new Increment(resources);
            Thread decrement = new Decrement(resources);
            increments.add(increment);
            decrements.add(decrement);
            increment.start();
            decrement.start();
        }

        for (int i = 0; i < 1000; i++) {
            Thread read = new Read(resources);
            reads.add(read);
            read.start();
        }

        for (int i = 0; i < 100; i++) {
            increments.get(i).join();
            decrements.get(i).join();
        }

        for (int i = 0; i < 1000; i++) {
            reads.get(i).join();
        }
        System.out.println(System.currentTimeMillis() - start);
    }

    private static abstract class UserThread extends Thread {
        protected Resources resources;

        public UserThread(Resources resources) {
            this.resources = resources;
        }

    }

    private static class Read extends UserThread {

        public Read(Resources resources) {
            super(resources);
        }

        public void run() {
                resources.getCounter();


        }
    }

    private static class Increment extends UserThread {

        public Increment(Resources resources) {
            super(resources);
        }

        public void run() {
                resources.increment();



        }
    }

    private static class Decrement extends UserThread {

        public Decrement(Resources resources) {
            super(resources);
        }

        public void run() {
                resources.decrement();



        }
    }

    private static class Resources {

        private ReentrantReadWriteLock reentrantReadWriteLock = new ReentrantReadWriteLock();

        private ReentrantReadWriteLock.WriteLock writeLock = reentrantReadWriteLock.writeLock();
        private ReentrantReadWriteLock.ReadLock readLock = reentrantReadWriteLock.readLock();
        private ReentrantLock lock = new ReentrantLock();

        public int getCounter() {
            readLock.lock();
            try {
                try {
                    Thread.sleep(5);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
                return counter;
            } finally {
                readLock.unlock();
            }

        }

        private int counter = 0;

        public void increment() {
            writeLock.lock();
            try {
                counter++;
            } finally {
                writeLock.unlock();
            }
        }

        public void decrement() {
            writeLock.lock();
            try {
                counter--;
            } finally {
                writeLock.unlock();
            }
        }

    }

}

ReentrantReadWriteLock performing very bad compared to ReentrantLock

2 Answers2