Why does the order of output not getting changed when using random and threadpool?

Question

I am trying to learn about using threadpool through Executors. I am trying to print the factorial result of 10 numbers from 1 to 10 (not necessarily in same order) whichever completes first and then let the thread sleep for 1-5 seconds which will chosen as randomly.

Here is my Callable Task:

 class FactorialCalculator implements Callable{
    @Override
    public Integer call() throws Exception {
        int i= 1, temp = num;
        while (temp > 1){
            i = i*temp;
            temp--;
        }
        int x = ThreadLocalRandom.current().nextInt(1, 6);
        Thread.sleep(x * 1000);
        return i ;
    }

}

Here is my main class:

    public static void main(String[] args) throws InterruptedException, ExecutionException {
        ExecutorService e = Executors.newFixedThreadPool(10);
        Set <Future<Integer>> set= new HashSet<Future<Integer>>();
        int totalSum = 0;
        for (int i = 1; i <= 10; i++) {
            set.add(e.submit(new FactorialCalculator(i)));
        }
        while(!set.isEmpty()) {
            Iterator<Future<Integer>> iter =  set.iterator();
            Future<Integer> fi = iter.next();
            if(fi.isDone()) {
                int temp = fi.get();
                System.out.println(temp);
                iter.remove();  
            }   
        }
   }

I ran this program on Eclipse 10 times, output remains same every time.

Then i compile it through command prompt and then ran it there. Output came different from eclipse but again running multiple times will produce same output here.

Why the output doesn't come randomly?

Thanks in advance.

Update:

Thanks to Ben for pointing out the problem:

Here is the change i made:

Iterator<Future<Integer>> iter =  list.iterator();
        while(!list.isEmpty()) {
            Future<Integer> fi = iter.next();
            if(fi.isDone()) {
                int temp = fi.get();
                System.out.println(temp);
                iter.remove();  
            }
            if(!list.isEmpty() && iter.hasNext() == false) {
                iter =  list.iterator();
            }
        }

score 1 · Accepted Answer · answered Jul 22 '19 at 08:38

1

I believe it's because you're declaring the iterator inside the while loop, rather than outside it. It means you're always getting the first element from the iterable, and checking if it's done. If it's not, you loop again, checking the first element from the set again.

It's only selecting the next item from the set once the current head of the iterable is finished, so you're removing items sequentially.

answered Jul 22 '19 at 08:38

Ben R.

1,965
1
13
23

I don't know who downvoted this answer, it's the correct one. The iterable is started again and again from the first item until its `isDone` is true. – RealSkeptic Jul 22 '19 at 08:59
Isn't that correct to ensure that you are covering all the not isDone elements again? – Vinay Prajapati Jul 22 '19 at 09:05
1

@VinayPrajapati What the OP is trying to do is print the first future that finished, then the second one that finished, etc. So the loop logic is that while the set is not empty, you go through it searching for a done element. If there is one, you print and remove it. You go through it again and again until you're done. Unfortunately, if you retrieve the iterator again and again **without a second inner loop**, you'll only ever poll the first element until it's done. – RealSkeptic Jul 22 '19 at 09:08
2

@VinayPrajapati I read it. If the logic was as I described it, the print would have been random, because the order of finishing the threads is random. However, the loop is incorrect, and therefore it always prints the elements in the order they appear in the iterator. – RealSkeptic Jul 22 '19 at 09:10
@Ben you were right actually. based on your hint i made some changes and now it is printing output randomly everytime. +1 – Nicky Jul 22 '19 at 09:11
@Ben, Vinay See the change i made to prove that it is indeed random. – Nicky Jul 22 '19 at 09:20
@Nicky if this is the correct answer, can you mark it as such please? – Ben R. Jul 22 '19 at 10:04

score 0 · Answer 2 · answered Jul 23 '19 at 05:06

Let's evaluate the code . First the Future are stored in HashSet. The while loop used is to iterate over this HashSet . Now Iterator is initiated iniside this loop and the Iterator returns first element and its been checked for done (fi.isDone()) if it false then the loop ends and process continues (including re initiating the iterator) which will continue till the time first element has evaluated. By the time first result is evaluated and all the other result runs in parallel in different threads (as there are 10 threads ) also gets evaluated.

When first result is done() then it was printed and also take out of the HashSet , so the Hash set for the Next while iteration will be 1 element less and hence Iterator will start from 2nd element . This process (removing of done element and initialising the iterator for remaining HasSet ) will continue till HasSet is empty (condition of while loop )

Now This code forces the to print result in order stored by HashSet (as explained above) (HasSet does not guarantee any order but same element's inserted (no matter in which order) will have same order stored in HasSet and since 1 to 10 same number are fed every time it will be stored in same order every time) For Ex Try Below code to understand it , Bothe iteration will print the same order even though they are inserted in different order.

Set<Integer> intSet = new HashSet<Integer>();

for(int i = 11 ; i <= 20 ; i++){
  intSet.add(i);

}

for(Integer integer : intSet){
  System.out.println(integer);
}

System.out.println("+++++++++++++");

intSet.clear();

for(int i = 20 ; i >= 11 ; i--){
  intSet.add(i);

}

intSet.forEach(integer -> System.out.println(integer));

Why does the order of output not getting changed when using random and threadpool?

2 Answers2