AtomicInteger in multithreading

Question

I want to find out all the prime numbers from 0 to 1000000. For that I wrote this stupid method:

public static boolean isPrime(int n) {
    for(int i = 2; i < n; i++) {
        if (n % i == 0)
            return false;
    }
    return true;
}

It's good for me and it doesn't need any edit. Than I wrote the following code:

private static ExecutorService executor = Executors.newFixedThreadPool(10);
private static AtomicInteger counter = new AtomicInteger(0);
private static AtomicInteger numbers = new AtomicInteger(0);

public static void main(String args[]) {
    long start = System.currentTimeMillis();
    while (numbers.get() < 1000000) {
        final int number = numbers.getAndIncrement();  // (1) - fast
        executor.submit(new Runnable() {
            @Override
            public void run() {
  //               int number = numbers.getAndIncrement(); // (2) - slow
                if (Main.isPrime(number)) {
                    System.out.println("Ts: " + new Date().getTime() + " " + Thread.currentThread() + ": " + number + " is prime!");
                    counter.incrementAndGet();
                }
            }
        });
    }
    executor.shutdown();
    try {
        executor.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS);
        System.out.println("Primes: " + counter);
        System.out.println("Delay: " + (System.currentTimeMillis() - start));
    } catch (Exception e) {
        e.printStackTrace();
    }
}

Please, pay attention to (1) and (2) marked rows. When (1) is enabled the program runs fast, but when (2) is enabled it works slower.

The output shows small portions with large delay

Ts: 1480489699692 Thread[pool-1-thread-9,5,main]: 350431 is prime!
Ts: 1480489699692 Thread[pool-1-thread-6,5,main]: 350411 is prime!
Ts: 1480489699692 Thread[pool-1-thread-4,5,main]: 350281 is prime!
Ts: 1480489699692 Thread[pool-1-thread-5,5,main]: 350257 is prime!
Ts: 1480489699693 Thread[pool-1-thread-7,5,main]: 350447 is prime!
Ts: 1480489711996 Thread[pool-1-thread-6,5,main]: 350503 is prime!

and threads get equal number value:

 Ts: 1480489771083 Thread[pool-1-thread-8,5,main]: 384733 is prime!
 Ts: 1480489712745 Thread[pool-1-thread-6,5,main]: 384733 is prime!

Please explain me why option (2) is more slowly and why threads get equal value for number despite AtomicInteger multithreading safe?

Answer 1

In the (2) case, up to 11 threads (the ten from the ExecutorService plus the main thread) are contending for access to the AtomicInteger, whereas in case (1) only the main thread accesses it. In fact, for case (1) you could use int instead of AtomicInteger.

The AtomicInteger class makes use of CAS registers. It does this by reading the value, doing the increment, and then swapping the value with the value in the register if it still has the same value that was originally read (compare and swap). If another thread has changed the value it retries by starting again : read - increment - compare-and-swap, until it is succesful.

The advantage is that this is lockless, and therefore potentially faster than using locks. But it performs poorly under heavy contention. More contention means more retries.

Edit

As @teppic points out, another problem makes case (2) slower than case (1). As the increment of numbers happens in the posted jobs, the loop condition remains true for much longer than needed. While all 10 threads of the executor are churning away to determine whether their given number is a prime, the main thread keeps posting new jobs to the executor. These new jobs don't get an opportunity to increment numbers until preceding jobs are done. So while they're on the queue numbers does not increase and the main thread can meanwhile complete one or more loops loop, posting new jobs. The end result is that many more jobs can be created and posted than the needed 1000000.