BookStore demo improvement

[hrstoyanov]

Hello,
Currently the Book Store demo uses Guava just for one dependency - striped locks. I wonder if we can get rid of it and replace it with some better/modern clean Java implementation, for example:

import java.lang.ref.Reference;
import java.lang.ref.ReferenceQueue;
import java.lang.ref.WeakReference;
import java.util.Objects;
import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

public class StripedLocks {

    private final int stripes;
    private final transient ReferenceQueue<LockHolder> queue;
    private final transient ConcurrentHashMap<Integer, WeakReference<LockHolder>> locks;

    public StripedLocks(int stripes) {
        this.stripes = stripes;
        queue = new ReferenceQueue<>();
        locks = new ConcurrentHashMap<>();
    }

    public StripedLocks() {
        this(4);
    }

    public final <T> T read(Object keyObject, Callable<T> readOperation) throws Exception {
        return execute(getLock(keyObject).readLock(), readOperation);
    }

    public final <T> T write(Object keyObject, Callable<T> writeOperation) throws Exception {
        return execute(getLock(keyObject).writeLock(), writeOperation);
    }

    private ReentrantReadWriteLock getLock(Object keyObject) {
        Objects.requireNonNull(keyObject,"keyObject can't be null");
        var key = keyObject.hashCode() % stripes;
        return locks.compute(key, (_, currentReference) -> {
            if(currentReference==null)
                return new WeakReference<>(new LockHolder(key, new ReentrantReadWriteLock()), queue);
            var lockHolder = currentReference.get();
            if (lockHolder == null)
                return new WeakReference<>(new LockHolder(key, new ReentrantReadWriteLock()), queue);
            return currentReference;
        }).get().lock();
    }

    private <T> T execute(Lock theLock, Callable<T> operation) throws Exception {
        theLock.lock();
        try {
            return operation.call();
        } finally {
            theLock.unlock();
            purgeStaleLocks();
        }
    }

    @SuppressWarnings("unchecked")
    void purgeStaleLocks() {
        Reference<LockHolder> reference;
        while ((reference = (Reference<LockHolder>) queue.poll()) != null) {
            synchronized (queue) {
                locks.remove(reference.get().key());
            }
        }
    }

    private record LockHolder(int key, ReentrantReadWriteLock lock) {}

}

[fh-ms]

Hi, that's a good idea.
Fewer dependencies are always good.

[fh-ms]

Range was also used. But it's easy to replace.

main...remove-guava

[hrstoyanov]

@fh-ms thanks for the fast turnaround! One caveat though - i really have not tested my suggestion above that well (will do it shortly though).

A few questions:

1. Any reason why Range is still a class and not a record?
2. You are sure the ``ensureInitialized``` with double-locking check is worth the headaches? This is executed only once and creates a couple of empty objects with minimal memory footprint you can easily put in a contrutor? Or maybe I am missing something about how ES deserializes objects and executes constructors?
3. I also realized you will need Runnable and Supplier<T> for the operations, but also added Callable<T> because:

• it can return stuff like the Supplier you added, but allows checked Exceptions to be thrown.
• Callable<Void> can be somewhat like Runnable, but allows checked exceptions.

4. Not sure if that is worth the effort, but the execute(...) can be reused:

   private <T> T execute(Lock theLock, Supplier<T> operation) {
       var results = ReadWriteLocked.execute(theLock, operation);
       purgeStaleLocks();
       return results;
   }

[hrstoyanov]

@fh-ms I reviewed your code and I have a question - are the total stripes always 4? Should stripeCount be transient at all?
Also, is this just your personal style or a requirement of ES Persistence I should be aware of:

public ReadWriteLockedStriped()
    {
        super();
    }

Attached is my latest edition of the two files - they reuse code between each other and is even less code because of that.
locks.zip

[fh-ms]

Yes, the stripe count should be configurable.
This code style is just an old habit of mine.
I'll check out your files.