views:

1062

answers:

5
+1  A: 

Read your code carefully. The two cases aren't doing the same thing, and it has nothing to do with threads.

When you join a thread, other threads will run in the background, yes.

JesperE
Would you please tell me what do you mean by "The two cases aren't doing the same thing" ?
Geo
The reason one of the cases is much slower is that it does not do the same thing. Hint: how many times are you calling isPrime() and getNextPrime() respectively?
JesperE
A: 

Running a test, the second one doesn't seem to take 9 seconds--in fact, it takes at least as long as the first (which is to be expected, threding can't help the way it's implemented in your example.

Thread.join will only return when the thread.joined terminates, then the current thread will continue, the one you called join on will be dead.

For a quick reference--think threading when starting one iteration does not depend on the result of the previous one.

Bill K
I tried them both on my laptop.The times I wrote in the post are the times I get.
Geo
Perhaps something about how our VMs are handling things differently. Could be that your thread.join is broken, that would explain the runtime difference... Also, the second one threw an exception you weren't handling. I'm not sure how you ran it. What VM are you using?
Bill K
+2  A: 

You can test this better by making the exact code in your first example run with threads. Sub your main method with this:

    private static int currentPrime;
public static void main(String[] args) throws InterruptedException {
 for (currentPrime = 0; currentPrime < 10000; currentPrime++) {
  Thread t = new Thread(new Runnable() {
   public void run() {
    getNextPrime(currentPrime);
   }});
  t.run();
  t.join();
 }
}

This will run in the same time as the original.

To answer your "join" question: yes, other threads can be running in the background when you use "join", but in this particular case you will only have one active thread at a time, because you are blocking the creation of new threads until the last thread is done executing.

James Van Huis
What if I was adding the created thread to a list , and then joining them , after I exit the loop ? And if I do this , will I have 10000 threads running simultaneously ?
Geo
Well, the sample I wrote has a static int which is being manipulated inside the loop, so you would need to make some modifications in order for that to work. However, if you made the code thread safe, you could queue up those threads and run them simultaneously if you wanted to.
James Van Huis
However, with a huge number of threads, you should look at java.util.concurrent.ThreadPoolExecutor instead of attempting to execute all of them in one single shot.
James Van Huis
+2  A: 

JesperE is right, but I don't believe in only giving hints (at least outside a classroom):

Note this loop in the non-threaded version:

for(int i = 2;i < nextPrime;i++) {
  if(nextPrime % i == 0) {
    prime = false;
  }
}

As opposed to this in the threaded version:

for(int i = 2;i < from;i++) {
  if((number % i) == 0) {
    return false;
  }
}

The first loop will always run completely through, while the second will exit early if it finds a divisor.

You could make the first loop also exit early by adding a break statement like this:

for(int i = 2;i < nextPrime;i++) {
  if(nextPrime % i == 0) {
    prime = false;
    break;
  }
}
Rasmus Faber
+2  A: 

By putting the join() in the loop, you're starting a thread, then waiting for that thread to stop before running the next one. I think you probably want something more like this:

public static void main(String[] args) {
   int primeStart = 5;

   // Make thread-safe list for adding results to
   List list = Collections.synchronizedList(new ArrayList());

   // Pull thread pool count out into a value so you can easily change it
   int threadCount = 10000;
   Thread[] threads = new Thread[threadCount];

   // Start all threads
   for(int i = 0;i < threadCount;i++) {
     // Pass list to each Runnable here
     // Also, I added +i here as I think the intention is 
     //    to test 10000 possible numbers>5 for primeness - 
     //    was testing 5 in all loops
     PrimeRunnable pr = new PrimeRunnable(primeStart+i, list);
     Thread[i] threads = new Thread(pr);
     threads[i].start();  // thread is now running in parallel
   }

   // All threads now running in parallel

   // Then wait for all threads to complete
   for(int i=0; i<threadCount; i++) {
     threads[i].join();
   }
}

By the way pr.getLastPrime() will return 0 in the case of no prime, so you might want to filter that out before adding it to your list. The PrimeRunnable has to absorb the work of adding to the final results list. Also, I think PrimeRunnable was actually broken by still having incrementing code in it. I think this is fixed, but I'm not actually compiling this.

public class PrimeRunnable implements Runnable {    
    private int from;
    private List results;   // shared but thread-safe

    public PrimeRunnable(int from, List results) {
        this.from = from;
        this.results = results;
    }

    public void isPrime(int number) {
        for(int i = 2;i < from;i++) {
                if((number % i) == 0) {
                        return;
                }
        }
        // found prime, add to shared results
        this.results.add(number);
    }

    public void run() {
        isPrime(from);      // don't increment, just check one number
    }    
}

Running 10000 threads in parallel is not a good idea. It's a much better idea to create a reasonably sized fixed thread pool and have them pull work from a shared queue. Basically every worker pulls tasks from the same queue, works on them and saves the results somewhere. The closest port of this with Java 5+ is to use an ExecutorService backed by a thread pool. You could also use a CompletionService which combines an ExecutorService with a result queue.

An ExecutorService version would look like:

public static void main(String[] args) {
   int primeStart = 5;

   // Make thread-safe list for adding results to
   List list = Collections.synchronizedList(new ArrayList());

   int threadCount = 16;  // Experiment with this to find best on your machine
   ExecutorService exec = Executors.newFixedThreadPool(threadCount);

   int workCount = 10000;  // See how # of work is now separate from # of threads?
   for(int i = 0;i < workCount;i++) {
     // submit work to the svc for execution across the thread pool 
     exec.execute(new PrimeRunnable(primeStart+i, list));
   }

   // Wait for all tasks to be done or timeout to go off
   exec.awaitTermination(1, TimeUnit.DAYS);
}

Hope that gave you some ideas. And I hope the last example seemed a lot better than the first.

Alex Miller
Great example! Thank you !
Geo
+1: Awesome, so helpful.
sixtyfootersdude