1

We all know Java optimises our code quite thoroughly and we all love it. Well, most of the time. Below is a piece of code that really messes with my head:

public class BrokenOptimizationTest {

/**
 * This thread constantly polls another thread object's private field.
 */
public static class ComparingThread extends Thread {
    private int currentValue = 0;
    private AdditionThread otherThread = null;

    public ComparingThread(AdditionThread add) {
        this.otherThread = add;
    }

    @Override
    public void run() {
        while (true) {
            int testValue = currentValue;

            if (BrokenOptimizationTest.shouldDoSomething) {

                do {
                    testValue = otherThread.getValue();
                    BrokenOptimizationTest.doSomething();
                    // System.out.println(testValue); // to see testValue really changes
                }
                while (testValue == currentValue);

            }
            else {

                do {
                    testValue = otherThread.getValue();
                    // System.out.println(testValue); // to see testValue really changes
                }
                while (testValue == currentValue);

            }

            System.out.println("{ testValue: " + testValue + ", currentValue: " + currentValue + " }");

            currentValue = testValue;
        }
    }
}

/**
 * This thread often adds to its pollable value.
 */
public static class AdditionThread extends Thread {
    private int currentValue = 0;
    public long queryCount = 0;

    public int getValue() {
        ++queryCount;
        return currentValue;
    }

    @Override
    public void run() {
        while (true) {
            ++currentValue;

            //I said 'often', so sleep some more
            try {
                Thread.sleep(1);
            }
            catch (InterruptedException e) {}
        }
    }
}

/**
 * Whether or not the program must simulate doing an expensive calculation between consecutive queries.
 */
public static boolean shouldDoSomething = false;

/**
 * Simulates doing an expensive calculation
 */
public static void doSomething() {
    try {
        Thread.sleep(0, 100);
    }
    catch (InterruptedException e) {}
}


/**
 * Call the program with something like "slow" to enable doSomething
 */
public static void main(String[] args) {
    if (args.length >= 1 && (args[0].toLowerCase().contains("slow") || args[0].toLowerCase().contains("dosomething")))
        shouldDoSomething = true;


    AdditionThread addThread = new AdditionThread();
    ComparingThread compThread = new ComparingThread(addThread);
    addThread.start();
    compThread.start();

    /**
     * Print the current program state every now and then.
     */
    while (true) {
        System.out.println("{ currentValue: " + addThread.getValue() + ", activeThreads: " + Thread.activeCount() + ", queryCount: " + addThread.queryCount + " }");
        System.out.flush();

        try {
            Thread.sleep(1000);
        }
        catch (InterruptedException e) {}
    }
}
}

The results may vary between fast, slow single-thread and multi-thread processors. On the computers I tested on (without doSomething), the output looks like this:

{ currentValue: 1, activeThreads: 3, queryCount: 1 }
{ testValue: 1, currentValue: 0 }
{ testValue: 2, currentValue: 1 }
{ testValue: 3, currentValue: 2 }
{ testValue: 4, currentValue: 3 }
{ testValue: 5, currentValue: 4 }
{ testValue: 6, currentValue: 5 }
{ testValue: 7, currentValue: 6 }
{ testValue: 8, currentValue: 7 }
{ testValue: 9, currentValue: 8 }
{ testValue: 10, currentValue: 9 }
{ testValue: 11, currentValue: 10 }
{ testValue: 12, currentValue: 11 }
{ testValue: 13, currentValue: 12 }
{ currentValue: 994, activeThreads: 3, queryCount: 2176924819 }
{ currentValue: 1987, activeThreads: 3, queryCount: 4333727079 }
{ currentValue: 2980, activeThreads: 3, queryCount: 6530688815 }
{ currentValue: 3971, activeThreads: 3, queryCount: 8723797559 }

The first few iterations of the CompareThread work out fine and then Java 'optimizes': the testValue and currentValue are always equal and keep changing their values although the thread never leaves the innermost loop. The only cause I can think of, is that Java performs its execution out of order, like so:

do {
    testValue = otherThread.getValue();
    currentValue = testValue; // moved up from beneath the loop
}
while (testValue == currentValue);

I understand out-of-order execution is allowed in the Java compiler because it can increase performance, but these statements are clearly dependent on one another.

My question is simply: why? Why does Java run the program this way?

Note: if the program is started with the parameter doSomething or if AdditionThread.currentValue is made volatile, the code runs just fine.

Flumble
  • 113
  • 2

1 Answers1

4

You've answered your own question:

if AdditionThread.currentValue is made volatile, the code runs just fine.

The java memory model doesn't make any guarantee that when you read AdditionThread.currentValue from inside ComparingThread that you will see the latest version as exists in AdditionThread. If data is meant to be visible to other threads, you have to use one of the tools provided, volatile, synchronized, java.util.concurrent.* in order to tell the system you need visibility guarantees.

Out-of-order execution is not the optimization that's causing unexpected behaviour, it's just the ComparingThread keeping a copy of AdditionThread.currentValue on its own stack.

Turning on "doSomething" also fixes it since putting threads to sleep will generally cause them to refresh their stack when they wake back up, although that's not formally guaranteed.

Affe
  • 47,174
  • 11
  • 83
  • 83
  • Ah, that would explain it almost entirely. The only thing left bothering me is: why does testValue increase in the console when the println is uncommented? – Flumble May 07 '13 at 20:27
  • On second thought, I can't seem to reproduce what I've posted above about testValue increasing, so it might be just my imagination. Thanks for the quick and complete answer. :) – Flumble May 07 '13 at 21:13
  • println can also have a similar effect to sleep in causing the thread to reset its state when it comes back from blocking on waiting for system resources to be available. So it can "hide" a problem caused by lack of synchronization. See here : http://stackoverflow.com/a/11570673/331052 – Affe May 08 '13 at 02:39