CODEWITHSUNDEEP
javamultithreading
AlexanderDickinson
AlexanderDickinson

Reputation: 47

Why do multiple threads not work in this program?

I have written a program that finds the number with the greatest number of divisors from the range 1 to 100,000:

 /**
    A program that finds the number with the largest number of divisors in the range of 1 to 100,000. This version simply uses threads.
    */

    public class ThreadDivisors{

        private static final int SAMPLE_SIZE = 100000;
        private static final int THREAD_COUNT = 2;
        private volatile static int maxDividend;
        private volatile static int maxDivisorCount = 0;

        public static void main(String[] args){

            int start = 1;
            int end = SAMPLE_SIZE / THREAD_COUNT;
            DivisorThread[] threads = new DivisorThread[THREAD_COUNT];

            for(int i = 0; i < THREAD_COUNT; i++){
                threads[i] = new DivisorThread(start, end);
                System.out.println("Thread created starting at " + start + " and ending at " + end);//debug
                start += SAMPLE_SIZE / THREAD_COUNT;
                end += SAMPLE_SIZE / THREAD_COUNT;
            }

            for(int i = 0; i < THREAD_COUNT; i++){
                threads[i].start();
            }

            for(int i = 0; i < THREAD_COUNT; i++){
                while(threads[i].isAlive()){
                    try{
                        threads[i].join();
                    }
                    catch(InterruptedException e){
                        System.out.println(e.getMessage());
                        e.printStackTrace();
                    }
                }
            }

            System.out.println("The number with the most divisors is " + maxDividend);
            System.out.println(maxDivisorCount);

        }

        static class DivisorThread extends Thread{

            static int start;
            static int end;

            public DivisorThread(int start, int end){
                this.start = start;
                this.end = end;
            }

            public void run(){

                System.out.println("Thread running...");
                divisorCount(start, end);

            }

        }

        private static void divisorCount(int start, int end){

            int divisorCount;//number of divisors for number being divided
            int maxThreadDividend = start;
            int maxThreadDivisorCount = 0;

            for (int dividend = start; dividend <= end; dividend++){//iterate through dividends

                divisorCount = 0;

                for (int divisor = start; divisor <= dividend; divisor++){//iterate through divisors

                    if (dividend % divisor == 0){
                        divisorCount++;
                    }//end if

                }//end for

                if (divisorCount > maxThreadDivisorCount){
                    maxThreadDivisorCount = divisorCount;
                    maxThreadDividend = dividend;
                    System.out.println(maxThreadDividend);
                    System.out.println(maxThreadDivisorCount);
                }

            }//end for

            report(maxThreadDivisorCount, maxThreadDividend);

        }
        /*This subroutine updates global variables that keep track of which number has the most divisors.*/
        private synchronized static void report(int maxThreadDivisorCount, int maxThreadDividend){

            if(maxThreadDivisorCount > maxDivisorCount){
                maxDivisorCount = maxThreadDivisorCount;
                maxDividend = maxThreadDividend;
            }

        }

    }

It works with one thread (the global variable THREAD_COUNT determines this), but not with more. For instance, when I set THREAD_COUNT to 2, I get the following output:

Thread created starting at 1 and ending at 50000 Thread created starting at 50001 and ending at 100000 Thread running... 50001 1 Thread running... 50001 1 The number with the most divisors is 50001 1

The subroutine divisorCount() seems to just stop with the starting point of whatever thread has the largest starting point...it does not go further. What might be causing this problem?

Upvotes: 1

Views: 104

Answers (1)

Vogel612
Vogel612

Reputation: 5647

The problem is your inner for loop. Your divisors must not start at start, but at 1 (or 2 depending on wheter you start the divisor counter with 0 or 1 (or even 2, more later))

That should fix your problem.

On a unrelated note I think you can remove the volatile keyword from your maxDivisor and maxDividend variables since all acesses to them are in a synchronized block.

Another thing. There will be no divisors greater than half of the checked dividend (aside from the number itself) Adjusting your limit accordingly should give you a significant performance boost:

for (int dividend = start; dividend <= end; dividend++){

    divisorCount = 2;// 1 and dividend are always divisors

    for (int divisor = 2; divisor <= dividend / 2; divisor++){

Upvotes: 3

Related Questions