Java Multithreading: threads shall access list

Question

I'm running with five threads, and I have a list of objects (which I initialize independently of the threads). The objects in the list use a boolean as a flag, so I know if they have been handled by another Thread already. Also, my Thread has an Integer for its "ID" (so U know which thread is currently working).

The problem: The first thread that gets a hand on the for-loop will handle all objects in the list, but I want the threads to alternate. What am I doing wrong?

the run() method looks similar to this:

void run() {
    for (int i = 0; i < list.size(); i++) {
        ListObject currentObject = list.get(i);
        synchronized (currentObject) {
            if (currentObject.getHandled == false) {
                currentObject.setHandled(true);
                System.out.println("Object is handled by " + this.getID());
            } else {
                continue;
            }
        }
    }
}

Answer 1

TL;DR Explicitly or implicitly divide the list among the threads; and synchronization if really needed;

The problem: The first thread that gets a hand on the for-loop will handle all objects in the list, but i want the threads to alternate. What am I doing wrong?

That is expectable this entire block of code

  for (int i = 0; i < list.size(); i++) {
        ListObject currentObject = list.get(i);
        synchronized (currentObject) {
            ....
        }
    }

is basically being executed sequentially since each thread synchronizes in every iteration using the Object currentObject implicit lock. All five threads enter the run method, however one of them enters first in the synchronized (currentObject) all the other will wait in turn for the first thread to release the currentObject implicitly lock. When the thread is finished moves on to the next iteration while the remaining threads are still in the previous iteration. Hence, the first thread entering synchronized (currentObject) will have a head start, and will be steps head of the previous threads, and will likely compute all the remains iterations. Consequently:

The first thread that gets a hand on the for-loop will handle all objects in the list,

As it is you would be better off performance-wise and readability-wise executing the code sequentially.

Assumption

I am assuming that

1. the objects stored on the list are not being accessed elsewhere at the same time that those threads are iterating through the list;
2. the list does not contain multiple references to the same object;

I would suggest that instead of every thread iterating over the entire list and synchronizing in every iteration -- which is extremely non perform and actually defeats the point of parallelism -- every thread would compute a different chunk of the list (e.g., dividing the iterations of the for loop among the threads). For instance:

Approach 1: Using Parallel Stream

If you don't have to explicitly parallelize your code then consider using ParallelStream:

list.parallelStream().forEach(this::setHandled);
   
private void setHandled(ListObject currentObject) {
    if (!currentObject.getHandled) {
        currentObject.setHandled(true);
        System.out.println("Object is handled by " + this.getID());
    }
}

Approach 2 : If you have to explicitly parallelized the code using executors

I'm running five threads,

(as first illustrated by ernest_k)

 ExecutorService ex = Executors.newFixedThreadPool(5);
 for (ListObject l : list)
     ex.submit(() -> setHandled(l));
 ...

private void setHandled(ListObject currentObject) {
    if (!currentObject.getHandled) {
        currentObject.setHandled(true);
        System.out.println("Object is handled by " + this.getID());
    }
}

Approach 3: If you have to explicitly use the Threads

void run() {
    for (int i = threadID; i < list.size(); i += total_threads) {
        ListObject currentObject = list.get(i);
        if (currentObject.getHandled == false) {
           currentObject.setHandled(true);
           System.out.println("Object is handled by " + this.getID());
       }
    }
}

In this approach, I am splitting the iterations of the for loop among threads in a round-robin fashion, assuming that total_threads is the number of threads that will compute the run method, and that each thread will have a unique threadID ranging from 0 to total_threads - 1. Other approaches to distribute the iterations among threads would also so be visible, for instance dynamically distribution the iterations among threads:

void run() {
    for (int i = task.getAndIncrement(); i < list.size(); i = task.getAndIncrement();) {
        ListObject currentObject = list.get(i);
        if (currentObject.getHandled == false) {
           currentObject.setHandled(true);
           System.out.println("Object is handled by " + this.getID());
       }
    }
}

where task would be an atomic integer (i.e., AtomicInteger task = new AtomicInteger();).

In all approaches the idea is the same assign different chunks of the list to the threads so that those threads can execute those chunks independently of each other.

---

If the assumptions 1. and 2. cannot be made then you can still apply the aforementioned logic of splitting the iterations among threads but you will need to add synchronization, in my examples to the follow block of code:

  private void setHandled(ListObject currentObject) {
        if (!currentObject.getHandled) {
            currentObject.setHandled(true);
            System.out.println("Object is handled by " + this.getID());
        }
    }

as it is you can just turn the currentObject field into an AtomicBoolean as follows:

private void setHandled(ListObject currentObject) {
    if (currentObject.getHandled.compareAndSet(false, true)) {
        System.out.println("Object is handled by " + this.getID());
    }
}

otherwise use the synchronized clause:

private void setHandled(ListObject currentObject) {
        synchronized (currentObject) {
            if (!currentObject.getHandled) {
                currentObject.setHandled(true);
                System.out.println("Object is handled by " + this.getID());
            }
       }
 }