How to speed up calculation using multiple threads?

Question

I'm trying to compute Pi, but what I really want to achieve is efficiency when using more than one thread. The algorithm is simple: I randomly generate points in the unit square and after that count how many of them are in the circle inscribed within the square. (more here: http://math.fullerton.edu/mathews/n2003/montecarlopimod.html) My idea is to split the square horizontally and to run a different thread for each part of it. But instead of speed up, all I get is a delay. Any ideas why? Here is the code:

public class TaskManager {

public static void main(String[] args) {

    int threadsCount = 3;
    int size = 10000000;
    boolean isQuiet = false;

    PiCalculator pi = new PiCalculator(size);   
    Thread tr[] = new Thread[threadsCount];

    long time = -System.currentTimeMillis();

    int i;
    double s = 1.0/threadsCount;
    int p = size/threadsCount;

    for(i = 0; i < threadsCount; i++) {
        PiRunnable r = new PiRunnable(pi, s*i, s*(1.0+i), p, isQuiet);
        tr[i] = new Thread(r);
    }

    for(i = 0; i < threadsCount; i++) {
        tr[i].start();
    }

    for(i = 0; i < threadsCount; i++) {
        try {
            tr[i].join();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }       

    double myPi = 4.0*pi.getPointsInCircle()/pi.getPointsInSquare();
    System.out.println(myPi + " time = " + (System.currentTimeMillis()+time));
}
}

public class PiRunnable implements Runnable {

PiCalculator pi;
private double minX;
private double maxX;
private int pointsToSpread;

public PiRunnable(PiCalculator pi, double minX, double maxX, int pointsToSpread, boolean isQuiet) {
    super();
    this.pi = pi;
    this.minX = minX;
    this.maxX = maxX;
    this.pointsToSpread = pointsToSpread;
}


@Override
public void run() {
    int n = countPointsInAreaInCircle(minX, maxX, pointsToSpread);  
    pi.addToPointsInCircle(n);
}

public int countPointsInAreaInCircle (double minX, double maxX, int pointsCount) {
    double x;
    double y;

    int inCircle = 0;

    for (int i = 0; i < pointsCount; i++) { 
        x = Math.random() * (maxX - minX) + minX;
        y = Math.random();          

        if (x*x + y*y <= 1) {
            inCircle++;
        }
    }

    return inCircle;

}

}


public class PiCalculator {

private int pointsInSquare;
private int pointsInCircle;

public PiCalculator(int pointsInSquare) {
    super();
    this.pointsInSquare = pointsInSquare;
}

public synchronized void addToPointsInCircle (int pointsCount) {
    this.pointsInCircle += pointsCount;
}

public synchronized int getPointsInCircle () {
    return this.pointsInCircle;
}

public synchronized void setPointsInSquare (int pointsInSquare) {
    this.pointsInSquare = pointsInSquare;
}

public synchronized int getPointsInSquare () {
    return this.pointsInSquare;
}

}

Some results: -for 3 threads: "3.1424696 time = 2803" -for 1 thread: "3.1416192 time = 2337"

Answer 1

Your threads could be fighting/waiting for Math.random() which is synchronized, you should create an instance of java.util.Random for each thread. Also in this case speedup with multiple threads will only happen if you have more than one core/cpu.

From the javadoc of Math.random():

This method is properly synchronized to allow correct use by more than one thread. However, if many threads need to generate pseudorandom numbers at a great rate, it may reduce contention for each thread to have its own pseudorandom-number generator.