Search code examples
javagame-loop

Why is this print statement changing how this loop executes?


This is the run method from my game loop thread (a modified version of another SO answer). The running boolean is basically there as a way to pause and unpause the game, and it works just fine normally. I tried to add a constructor that lets the game start out as paused, or running == false and it didn't work. The game would never start, despite successfully setting running to true. I added some print statements to figure out what was going on. Then a weird thing happened. The addition of the print statement fixed the problem.

private void reset() {
    initialTime = System.nanoTime();
    deltaU = 0; deltaF = 0;
    frames = 0; ticks = 0;
    timer = System.currentTimeMillis();
}
public void run() {
    Thread.currentThread().setPriority(Thread.MAX_PRIORITY);

    reset();
    boolean wasRunning=false;
    while (true) {
        System.out.println("in loop");
        if (running) {
            System.out.println("running");
            if(!wasRunning){
                wasRunning=true;
                reset();
            }
            long currentTime = System.nanoTime();
            deltaU += (currentTime - initialTime) / timeU;
            deltaF += (currentTime - initialTime) / timeF;
            initialTime = currentTime;

            if (deltaU >= 1) {
                update();
                ticks++;
                deltaU--;
            }

            if (deltaF >= 1) {
                render();
                frames++;
                deltaF--;
            }

            if (System.currentTimeMillis() - timer > 1000) { //prints warning if dropped frames
                if (printTime) {
                    System.out.println(String.format("UPS: %s, FPS: %s", ticks, frames));
                }
                if(ticks!=60 || frames!=60)
                    System.out.println(String.format("UPS: %s, FPS: %s", ticks, frames));
                frames = 0;
                ticks = 0;
                timer += 1000;
            }
        }
        else
            wasRunning=false;
    }
}

the output is

in loop
running
in loop
running
in loop
running...

And the game runs with a lot of studder (though the fps warning is oddly silent). when I comment out the System.out.println("in loop"), there is no output, and setting running to true does nothing.

So the fact that there is a print statement is actually causing the loop to execute, but omitting it causes it to fail.

Note that if running is set to true at first, the print statements are removed, the pause and unpause functionality works as expected.

I also tried renaming running to running2 just to make sure it wasn't overriding anything.


Solution

  • tl;dr: You should try marking running as volatile.

    I assume that running is set from another thread? If that's the case, if running is initially false and is only set to true by another thread, then it's likely a race-condition.

    The addition of the println() call before the if statement adds enough time for the other thread to modify running to true before it's checked. Removing the println() call causes running to be checked before the other thread can set it to true.

    If this is the case, you really should restructure your code to avoid this race condition. Not being able to see it all, it's hard to make a recommendation as to how you'd approach this.

    EDIT: After reading the comment below, it appears like the infinite while loop would "catch" the change to running, however this may not be the case if running was not marked as volatile.

    volatile has defined semantics in Java, and one of them is that the variable is never thread-locally cached, i.e. accesses to a volatile variable always go to main memory and will always reflect the current state, even if updated by other threads.

    See here for more information on the volatile keyword.

    EDIT: Added tl;dr.

    EDIT: Quick/layman's explanation of volatile: The JVM can be pretty hard-core in the number of optimizations it makes for efficiency.

    For a non-volatile field access in a method, the JVM may make assumptions that no other threads are accessing it; in doing so, it may make a thread-local copy of that field so it can modify it without having to deal with the contention of accessing "main memory". This increases performance but has the drawback we've seen: If another thread modifies running, the current thread won't see it, because it's using a copy that's local/contextual to just itself.

    Marking the field as volatile fixes it because the semantics of the volatile under the current Java Memory Model basically ensure that all reads/writes of volatile fields go back to "main memory", establishing a happens-before relationship.

    So why not just mark everything as volatile? Or for that matter, why isn't everything implicitly volatile instead of the other way around? The answer is performance: A volatile access is generally going to be a lot slower than a non-volatile access, so the Java language designers have decided to make its usage optional, only when you know you need the safety. (Same goes for synchronized accesses)