0

I am trying to populate a static array using multiple threads. The suggested usage of join() in https://stackoverflow.com/a/1492077/10418317 seems to be imposing a specific order on the threads. I want them to run in parallel if they can so as to speed up the whole process
Assigment

    public class Assigment {

    public static int minPrime;
    public static int maxPrime;
    public static int[] masterMin = new int[10];
    public static int[] masterMax = new int[10];
    public static void main(String[] args) {
    // TODO code application logic here

    for(int i = 1; i < 11;i++){
     Thread thread = new Thread(new NumGenerator(i));
     thread.start();
     System.out.println("Thread " + i + " generating nums");

    }

    Arrays.sort(masterMin);
     Arrays.sort(masterMax);
     minPrime = masterMin[0];
     maxPrime = masterMax[9];
     System.out.println("<------------------------------>");
     System.out.println("Max Prime is " + maxPrime);
     System.out.println("Min Prime is " + minPrime);
 }

}


NumGenerator

public class NumGenerator implements Runnable {
int[] numbers;
int thrnum;
public NumGenerator(int thrnum){
    this.thrnum = thrnum;
}
@Override
public void run(){
Random rand = new Random();
 numbers = new int[3000];

for(int i = 0; i<3000; i++){
    numbers[i] = rand.nextInt(899999)+1;

     }
Searcher searcher = new Searcher(numbers);
Assigment.masterMax[thrnum-1] = searcher.max;
Assigment.masterMin[thrnum-1] = searcher.min;

}


I am trying to sort the array after every thread has completed inputting into it and then print out the first and the last i.e the smallest and the largest values.The problem is that the arrays masterMin and masterMax seem to be still having all zero entries after the supposed population through Assigment.masterMin[thrnum-1] = searcher.min; Searcher is another class which is actually giving me non-zero values for search.max and search.max as expected.

Twista
  • 241
  • 3
  • 11
  • 1
    The code is not thread-safe with regards to accessing the array from multiple threads. Are you supposed to use threads here? Have you learned about `synchronized`? – Kayaman Feb 20 '20 at 10:17
  • You have to make sure all your threads have ended before sorting and printing the values – jhamon Feb 20 '20 at 10:20
  • @Kayaman, my reasoning was that each thread is accessing it's own index so I could not foresee a concurrency conflict. Using `sychronized` with `run()` had already been tried but with no difference – Twista Feb 20 '20 at 10:22
  • 1
    the problem is that the sorting and print is made will the threads are still running (even maybe they are just starting and have not insert anything yet). – jhamon Feb 20 '20 at 10:23
  • 4
    Does this answer your question? [How to wait for a number of threads to complete?](https://stackoverflow.com/questions/1252190/how-to-wait-for-a-number-of-threads-to-complete) – jhamon Feb 20 '20 at 10:23

2 Answers2

0

@jhamon has already explained the reason. (That the threads have not completed)

How to solve it? Convert the part where you are starting threads to something like this using ExecutorService.

ExecutorService svc = Executors.newFixedThreadPool( 10 );
for(int i = 1; i < 11;i++){
 //Thread thread = new Thread(new NumGenerator(i));
 //thread.start();
    svc.submit( () -> new NumGenerator( i ) );
    System.out.println("Thread " + i + " generating nums");
}

svc.shutdown();
svc.awaitTermination( 2, TimeUnit.MINUTES );

//Now, print the values.
Sree Kumar
  • 2,012
  • 12
  • 9
  • Sorry, by writing "something like this", I meant that this is only an indicative code. You will have to write a complete one. The method you have mentioned, ```awaitTermination(long, TimeUnit)``` is the right one. – Sree Kumar Feb 20 '20 at 10:49
  • You can always [edit] your answer. – Abra Feb 20 '20 at 10:51
0

As pointed by @jhamon threads have not completed. But the approach give by @Sree is not ideal as shutting down a Executor just for the sake of waiting for the threads to complete is not the best of the approaches. You can try the following :

ExecutorService executor = Executors.newFixedThreadPool( 10 );
List<Callable<Object>> calls = new ArrayList<>();
for(int i = 1; i < 11;i++){
        calls.add(Executors.callable(new NumGenerator(i)));
    System.out.println("Thread " + i + " generating nums");
}
List<Future<Object>> futures = svc.invokeAll(calls);

invokeAll() will not return until all the tasks are completed (either by failing or completing successful execution).

lazy.coder
  • 411
  • 3
  • 14
  • _shutting down a Executor just for the sake of waiting for the threads to complete is not the best of the approaches_ Why is it not a good approach? According to the use-case described by the OP, it appears that after the threads complete he has no more need for the `ExecutorService` and according to the _javadoc_ for interface `ExecutorService`: _An unused ExecutorService should be shut down to allow reclamation of its resources_ – Abra Feb 20 '20 at 11:00
  • @Abra: take an example where you want to repeat this process of waiting on threads to complete 10 times. it doesn't make sense to create ExecutorService 10 times and shut them down as creating an ExecutorService itself is expensive. – lazy.coder Feb 20 '20 at 11:49
  • @Abra Semantically, shutdown and wait to finish isn't correct because you do not want to wait for the Executor to finish. You actually want to wait on the tasks. Hence why to use invokeAll, then shutdown the executor. In this contrived example you might not notice a difference though. – matt Feb 20 '20 at 14:10
  • _take an example where you want to repeat this process of waiting on threads to complete 10 times_ Not the case here. – Abra Feb 20 '20 at 17:47
  • @matt _You actually want to wait on the tasks_ The `ExecutorService` terminates once all the tasks terminate, doesn't it? – Abra Feb 20 '20 at 17:52
  • @Abra No, it needs to have shutdown called otherwise you could keep using the ExecutorService. – matt Feb 21 '20 at 06:17