0

I have below program to calculate value of Pi using threads, for simplicity I kept to maximum of 2 threads.

public class PiCalculator {

class Pi implements Runnable{
    int start;
    int end;
    volatile double result;

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

    @Override
    public void run() {
        for(int i = start; i < end; i++) {
            result += Math.pow(-1, i) / ((2 * i) + 1);
        }
        System.out.println(Thread.currentThread().getName() + " result =" + result);
    }

    public double getResult(){
        return result;
    }
}

public static void main(String[] args) throws InterruptedException {
    int maxThreads = 2;
    int maxValuePerThread = 1000 / maxThreads;
    int start = 0;
    int end = maxValuePerThread;
    double resultOut = 0d;
    PiCalculator pc = new PiCalculator();
    for(int i = 0; i < 2; i++) {
        Pi p = pc.new Pi(start, end);
        Thread t = new Thread(p);
        t.setName("thread" + i);
        t.start();
        t.join();
        start = start + end;
        end = end + maxValuePerThread;
        resultOut += p.getResult();
    }
    System.out.println("Final result = " + resultOut);
}

}

1) Why am I getting below result? What am I doing wrong?

thread0 result =0.7848981638974463
thread1 result =2.4999956250242256E-4
Final result = 0.7851481634599486

The Pi value is 3.14..... right?

2) When I change the

volatile double result;

to

double result;

I still get the same output, why is that so?

Rome_Leader
  • 2,518
  • 9
  • 42
  • 73
milkyway
  • 53
  • 8
  • What's the point of joining the threads in the loop? They will never run concurrently. – kichik Jul 14 '17 at 17:56
  • 1
    I did not check the formula, maybe it converges to pi/4 – Henry Jul 14 '17 at 18:07
  • @kichik I know it will not be parallel execution. How can I get the result from both these threads after the execution is done and add it? – milkyway Jul 14 '17 at 18:08
  • Create and start all the threads, keep them in a list or array, and then have a second loop to join them and get the value. – kichik Jul 14 '17 at 18:11
  • It never makes sense to call `t.start()` and then immediately call `t.join()` with nothing in between. The only reason to start a thread is so that something else can happen at the same time. But in your program, nothing else happens. The main thread doesn't do anything but wait until the new thread is finished. Even though your program starts two new threads, it _effectively_ is single threaded because there is never more than one thread running at any given time. – Solomon Slow Jul 14 '17 at 19:11
  • Similar implementation here: https://stackoverflow.com/questions/41094056/how-to-write-pi-calculation-program-in-java-using-multi-thread – Babar Jul 14 '17 at 19:24

2 Answers2

1

start=end should be better.

Your problem is just that you don't calculate PI, but PI/4, just multiply the result by 4 and you will get it.

Also, launch your thread concurrently don't call join in the loop, build an array of threads and join in another following loop.

Jean-Baptiste Yunès
  • 34,548
  • 4
  • 48
  • 69
  • What about the volatile? Why the result is same even if I add volatile or remove volatile? – milkyway Jul 14 '17 at 18:13
  • `volatile` is not useful here, that variable is private to each thread. – Jean-Baptiste Yunès Jul 14 '17 at 18:21
  • Can you please explain, what private has to do with it this variable? and how does joining in another following loop works? – milkyway Jul 14 '17 at 19:06
  • Concurrency is a rather complicated topic and much too vast to explain in a comment. I suggest reading up on it more generally. Effectively it addresses the problem of parallel threads reading and writing to the same bit of memory. – Paul Lammertsma Jul 14 '17 at 19:40
  • @milkyway each thread has its own `result` variable and each is the only responsible of its management (this was the meaning of *private* in my comment), so there is no need to protect it against concurrency. – Jean-Baptiste Yunès Jul 15 '17 at 06:57
0

regarding question 2, volatile means the jvm shouldn't cache it see here, so it doesn't, but that doesn't matter in your code, because result is a member of the Pi class, so when you make two Pi class instances, and give one to each thread, the threads are using completely separate result variables. also, because you start then immediately join the threads, The code you have posted is equivalent to

int maxThreads = 2;
int maxValuePerThread = 1000 / maxThreads;
int start = 0;
int end = maxValuePerThread;
double resultOut = 0d;
PiCalculator pc = new PiCalculator();
for(int i = 0; i < 2; i++) {
    Pi p = pc.new Pi(start, end);
    p.run();
    start = start + end;
    end = end + maxValuePerThread;
    resultOut += p.getResult();
}

what <thread a>.join does is it tell the thread that calls it (in this case the main thread) to stop executing until <thread a> finishes executing

if you are meaning to have both threads access your double value at the same time, you could move result out of Pi and put it in PiCalculator

and then to run both threads at the same time, change your loop to

int maxThreads = 2;
int maxValuePerThread = 1000 / maxThreads;
int start = 0;
int end = maxValuePerThread;
PiCalculator pc = new PiCalculator();
Thread[] threads=new Thread[2];
for(int i=0; i<2;i++){
    threads[i]=new Thread(pc.new Pi(start,end));
    threads[i].start();
    start = start + end;
    end = end + maxValuePerThread;
}

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

if you moved result to a member of the top level class, and both threads have been adding to it, it will have the summed up result already, no need to sum the threads

regarding question 1, pi/4 is .785398, so i'm guessing that's what your algorithm calculates, and the error is caused by the fact that doubles loose precision,

edit: it looks like you're using the leibniz formula which converges to pi/4 but is infinite, so the fact that you stopped at 1000 explains the error

also Math.pow(-1,i) is equivalent to 1-(i%2), which would be a lot faster

Austin_Anderson
  • 900
  • 6
  • 16
  • Thanks for the reply and explanation. I would appreciate if you can please explain how moving out join in another loop helps- for(int i = 0; i < 2; i++) { threads[i].join(); } – milkyway Jul 14 '17 at 21:25
  • sure, the main change here is that instead of `threads[0].start();` `threads[1].join();` `threads[0].start();` `threads[1].join();`
    you get `threads[0].start();` `threads[1].start();` `threads[0].join();` `threads[1].join();` so both can run at the same time, then both join
    – Austin_Anderson Jul 14 '17 at 21:29