Java factorial calculation with thread pool

Question

I achieved to calculate factorial with two threads without the pool. I have two factorial classes which are named Factorial1, Factorial2 and extends Thread class. Let's consider I want to calculate the value of !160000. In Factorial1's run() method I do the multiplication in a for loop from i=2 to i=80000 and in Factorial2's from i=80001 to 160000. After that, i return both values and multiply them in the main method. When I compare the execution time it's much better (which is 5000 milliseconds) than the non-thread calculation's time (15000 milliseconds) even with two threads.

Now I want to write clean and better code because I saw the efficiency of threads at factorial calculation but when I use a thread pool to calculate the factorial value, the parallel calculation always takes more time than the non-thread calculation (nearly 16000). My code pieces look like:

for(int i=2; i<= Calculate; i++)
{
    myPool.execute(new Multiplication(result, i));
}

run() method which is in Multiplication class:

public void run() 
{
        s1.Mltply(s2); // s1 and s2 are instances of my Number class
                       // their fields holds BigInteger values
}

Mltply() method which is in Number class:

public void Multiply(int number)
{
    area.lock(); // result is going wrong without lock
    Number temp = new Number(number);
    value = value.multiply(temp.value); // value is a BigInteger
    area.unlock();       
}

In my opinion this lock may kills the all advantage of the thread usage because it seems like all that threads do is multiplication but nothing else. But without it, i can't even calculate the true result. Let's say i want to calculate !10, so thread1 calculates the 10*9*8*7*6 and thread2 calculate the 5*4*3*2*1. Is that the way I'm looking for? Is it even possible with thread pool? Of course execution time must be less than the normal calculation...

I appreciate all your help and suggestion.

EDIT: - My own solution to the problem -

public class MyMultiplication implements Runnable 
{
    public static BigInteger subResult1;
    public static BigInteger subResult2;
    int thread1StopsAt;
    int thread2StopsAt;
    long threadId;
    static boolean idIsSet=false;

    public MyMultiplication(BigInteger n1, int n2)  // First Thread
    {
        MyMultiplication.subResult1 = n1;
        this.thread1StopsAt = n2/2;

        thread2StopsAt = n2;

    }
    public MyMultiplication(int n2,BigInteger n1)   // Second Thread
    {
        MyMultiplication.subResult2 = n1;
        this.thread2StopsAt = n2;

        thread1StopsAt = n2/2;
    }
    @Override
    public void run() 
    {
        if(idIsSet==false)
        {
            threadId = Thread.currentThread().getId(); 
            idIsSet=true;            
        }
        if(Thread.currentThread().getId() == threadId)
        {
            for(int i=2; i<=thread1StopsAt; i++)
            {
                subResult1 = subResult1.multiply(BigInteger.valueOf(i));
            }
        }
        else
        {
            for(int i=thread1StopsAt+1; i<= thread2StopsAt; i++)
            {
                subResult2 = subResult2.multiply(BigInteger.valueOf(i));
            }
        }            
    }   
}
public class JavaApplication3 
{
    public static void main(String[] args) throws InterruptedException 
    {
        int calculate=160000;
        long start = System.nanoTime(); 
        BigInteger num = BigInteger.valueOf(1);
        for (int i = 2; i <= calculate; i++) 
        {
          num = num.multiply(BigInteger.valueOf(i));
        }
        long end = System.nanoTime();
        double time = (end-start)/1000000.0;
        System.out.println("Without threads: \t" + 
        String.format("%.2f",time) + " miliseconds");    
        System.out.println("without threads Result: " + num);

        BigInteger num1 = BigInteger.valueOf(1);
        BigInteger num2 = BigInteger.valueOf(1);
        ExecutorService myPool = Executors.newFixedThreadPool(2);
        start = System.nanoTime();

            myPool.execute(new MyMultiplication(num1,calculate));  
            Thread.sleep(100);
            myPool.execute(new MyMultiplication(calculate,num2));

        myPool.shutdown();
        while(!myPool.isTerminated())   {}  // waiting threads to end
        end = System.nanoTime();
        time = (end-start)/1000000.0;
        System.out.println("With threads: \t" +String.format("%.2f",time) 
        + " miliseconds");    
        BigInteger result = 

        MyMultiplication.subResult1.
        multiply(MyMultiplication.subResult2);
        System.out.println("With threads Result: " + result);
        System.out.println(MyMultiplication.subResult1);
        System.out.println(MyMultiplication.subResult2);
    }   
}

input : !160000 Execution time without threads : 15000 milliseconds Execution time with 2 threads : 4500 milliseconds

Thanks for ideas and suggestions.

Answer 1

You may calculate !160000 concurrently without using a lock by splitting 160000 into disjunct junks as you explaint by splitting it into 2..80000 and 80001..160000.

But you may achieve this by using the Java Stream API:

IntStream.rangeClosed(1, 160000).parallel()
    .mapToObj(val -> BigInteger.valueOf(val))
    .reduce(BigInteger.ONE, BigInteger::multiply);

It does exactly what you try to do. It splits the whole range into junks, establishes a thread pool and computes the partial results. Afterwards it joins the partial results into a single result.

So why do you bother doing it by yourself? Just practicing clean coding?

On my real 4 core machine computation in a for loop took 8 times longer than using a parallel stream.