2

So i'm planning on having multiple threads accessing a shared resource which is a 2D array. Each Object will access one index at a time to set the contents of the object as shown below.

Cell is what each Object of the array is made up of.

RandomObject is just a temporary name for the class that will be inside the Cell.

So the purpose of this code is to check if the matrix is occupied, hence the null.

public class Cell
{
    private ReentrantLock lock;
    private RandomObject currentObject;
    
    public Cell()
    {
        lock = new ReentrantLock();
    }
    
    public boolean hasRandomObject()
    {
        boolean successfulLock = lock.tryLock();
        boolean randomObjectStatus = false;
        
        if (successfulLock)
        {
            if (currentObject != null)
            {
                randomObjectStatus = true;
            }
            System.out.println("Lock released.");
        }
        return randomObjectStatus;
    }
    
    public boolean setRandomObject(RandomObject inRandomObject)
    {
        boolean successfulLock = lock.tryLock();
        
        if (successfulLock)
        {
            if (currentObject != null)
            {
                successfulLock = false;
            }
            else
            {
                currentObject = inRandomObject;
            }
            //Inner lock release
            lock.unlock();
        }
        return successfulLock;
    }

    public boolean remove()
    {
        boolean successfulLock = lock.tryLock();
        
        if (successfulLock)
        {
            if (currentObject != null)
            { 
                currentObject.endThread();
                currentObject = null;
            }
            lock.unlock();
        }
        
        return successfulLock;
    }
}
public class Map
{
    private Cell[][] map;
    private int width, height;
    
    public Map(int inRowSize, int inColumnSize)
    {
        map = new Cell[inRowSize][inColumnSize];
        width = map.length;
        height = map[0].length;
        
        for (int i = 0; i < map.length; i++)
        {
            System.out.println(map[i].length);
            for (int j = 0; j < map[i].length; j++)
            {
                map[i][j] = new Cell();
            }
        }
    }
}

So a little context.

  1. Only one object can be in a Cell at one time, so I returned false if the object is not null when setting. So that I could know that it is already occupied.

  2. endThread is just a method inside RandomObject to end the thread as each RandomObject has their own thread.

  3. Included the Map class just to add context as to what the Array is.

Just want to check that i'm using Reentrant Locks correctly and making this class Threadsafe as there can be 1..* threads trying to occupy these Cells.

Also would love to discuss opinions about any better options, optimizations or improvements.

I normally use monitors and wanted to use ConcurrentLocks for the added functionality so I could have access to tryLock to instantly know if it was successful.

Cheers.

Helium
  • 31
  • 1
  • What you have is [lock striping](https://www.baeldung.com/java-lock-stripping), but if you intend to have one lock per cell, you're most likely striping things too fine. This will give you poor performance. – Kayaman Aug 31 '20 at 13:27
  • So what could I do to improve this performance? – Helium Aug 31 '20 at 13:43
  • Or rather not poor performance, but an unnecessarily finely striped data structure. In addition, if your `Cell` class really doesn't have other functionality, it seems practically replaceable by `AtomicReference`. You don't actually seem to be needing mutual exclusion anywhere, so a lock is completely overkill and you could use compare-and-set instructions instead. – Kayaman Aug 31 '20 at 14:00
  • This is what [AtomicReference](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/atomic/AtomicReference.html) would turn one of your methods into: `public boolean setRandomObject(RandomObject inRandomObject) { return reference.compareAndSet(null, inRandomObject); }` – Kayaman Aug 31 '20 at 14:05

0 Answers0