ForkJoinFramework with AtomicLong is not giving consistent result

Question

I'm trying to play with ForkJoinFramework. I know that my code might not be a good use case to use ForkJoin, but it should work at least..

I'm using RecursiveAction to concurrently modify a static AtomicLong variable. But I'm having some issues there, the result is not correct and not consistent.(Everytime I run it, the result is different).

I'm assuming AtomicLong is thread-safe, so the only thing that might be a reason would be some tasks are lost. But why?

`

public class ForkJoinFramework {

    private static AtomicLong sum = new AtomicLong(0);
    static class MyTask extends RecursiveAction {
        private static final long serialVersionUID = 1L;
        int left;
        int right;
        MyTask(int left, int right) {
            this.left = left;
            this.right = right;
        }
        @Override
        protected void compute() {
            if(right - left < 100) {
                for(int i = left; i < right; i++) {
                    sum.getAndIncrement();
                }
            } else {
                ForkJoinTask<Void> leftTask = new MyTask(left, left + (right - left) / 2).fork();
                ForkJoinTask<Void> rightTask = new MyTask(left + (right - left) / 2 + 1, right).fork();
                leftTask.join();
                rightTask.join();
            }
        }
    }
    
    public static void main(String[] args) {
        MyTask myTask = new MyTask(0, 10000000);
        myTask.compute();
        System.out.println("Sum is: " + sum.get());
    }
}

`

Answer 1

You have an off-by-one error. In your for loop, you are not counting the value of right, so if e.g. left is 101 and right is 150, you increment only 49 times.

The value I consistently get is 9868929, because you must be splitting the task into separate ForkJoinTasks 10000001 - 9868929, or 131072, or 2¹⁷, which makes sense as a power of 2 based on your algorithm.

Change

for(int i = left; i < right; i++) {

to

for(int i = left; i <= right; i++) {

and you'll get 10000001, and if you want 10000000, start with 1 instead of 0.

There's nothing wrong with losing tasks or the consistency of AtomicLong; you just had an off-by-one error.