My Java game loop

I just made this Java game loop and I want someone's opinion on it. It's based off what I've heard and read. Let me know if I went about it correctly and if there's any big problems with it.

public void run() {
    long nanosecond = 1000000000;
    int targetFps = 60;
    int targetUpdates = 20;
    long update = nanosecond / targetUpdates;
    long draw = nanosecond / targetFps;
    long lastUpdate = System.nanoTime();
    long lastDraw = System.nanoTime();
    long lastTime = System.nanoTime();
    int fps = 0;
    int updates = 0;
    while (running) {
        long now = System.nanoTime();
        while ((now - lastUpdate) > update) {
            update();
            lastUpdate = now;
            updates++;
        }
        while ((now - lastDraw) > draw) {
            draw();
            lastDraw = now;
            fps++;
        }
        if ((now - lastTime) >= nanosecond) {
            lastTime = System.nanoTime();
            System.out.println("Fps: "+ fps + " | Updates: " + updates);
            fps = 0;
            updates = 0;
        }
    }
}

Problem statement

Nothing to do with the code, but when asking a question here, it is helpful to give us a statement of what the code does. Reading your code, this is what I think it is intended to do.


This code loops until code elsewhere marks the object field running as false. Every so many nanoseconds, it redraws the screen. Every so many nanoseconds, it updates something (I don't actually know what). The code tracks how many updates and redraws it does and every second reports that information. After reporting, it clears the numbers for next time.

I wish there was more detail, but this at least describes what the code itself does. You could improve it with more information about what happens when a draw or update occurs.

The title might be "Game loop that tracks frame speed and update frequency".

Constants

long nanosecond = 1000000000;
    int targetFps = 60;
    int targetUpdates = 20;
    long update = nanosecond / targetUpdates;
    long draw = nanosecond / targetFps;

These values seem to be constant for the run of the program. Consider defining them outside the method as constants.

public final long NANOSECONDS_PER_SECOND = 1000000000;
public final int FPS_TARGET = 60;
public final int UPDATE_TARGET = 20;

public final long UPDATE_INTERVAL = NANOSECONDS_PER_SECOND / UPDATE_TARGET;
public final long DRAW_INTERVAL = NANOSECONDS_PER_SECOND / FPS_TARGET;

I also changed the names. Both to comply with naming standards for constants and because I didn't like the originals. Naming is hard, so these names may still be imperfect, but I find them easier to follow.

I don't like nanosecond as it doesn't describe what the variable does. It certainly does not represent a nanosecond. The name second would actually be more accurate, but it's really the ratio.

I prefer to put TARGET last in the name.

I find both update and draw unclear. They aren't updates and draws. They are the intervals at which those things happen.

Functionality

long lastUpdate = System.nanoTime();
    long lastDraw = System.nanoTime();
    long lastTime = System.nanoTime();

I'll explain more about why later, but I'd actually write this as

long nextUpdate = System.nanoTime() + UPDATE_INTERVAL;
    long nextDraw = System.nanoTime() + DRAW_INTERVAL;
    long nextReset = System.nanoTime() + NANOSECONDS_PER_SECOND;

This is a functional change. Rather than tracking the most recent whatever, track the next one.

I changed Time to Reset because that's what you do when the time period is up.

More naming

int fps = 0;
    int updates = 0;

I'd prefer

int frameCount = 0;
    int updateCount = 0;

The latter is just because I like naming counts as such. I find it easier to keep track of what I'm doing. I'd expect updates to be a collection representing the updates that occurred.

But for fps, that's not what it is. FPS means Frames Per Second in my lexicon. So what you would be counting here is frames, not "fps".

Back to functionality

long now = System.nanoTime();
        while ((now - lastUpdate) > update) {
            update();
            lastUpdate = now;
            updates++;
        }
        while ((now - lastDraw) > draw) {
            draw();
            lastDraw = now;
            fps++;
        }
        if ((now - lastTime) >= nanosecond) {
            lastTime = System.nanoTime();
            System.out.println("Fps: "+ fps + " | Updates: " + updates);
            fps = 0;
            updates = 0;
        }

I might change this to

long now = System.nanoTime();
        while (now > nextUpdate) {
            update();
            nextUpdate = now + UPDATE_INTERVAL;
            updateCount++;
        }

        while (now > nextDraw) {
            draw();
            nextDraw = now + DRAW_INTERVAL;
            frameCount++;
        }

        if (now >= nextReset) {
            nextReset = now + NANOSECONDS_PER_SECOND;
            System.out.println("Fps: "+ frameCount + " | Updates: " + updateCount);
            frameCount = 0;
            updateCount = 0;
        }

Now, rather than subtracting the last time from the current time and comparing it to the desired interval, we just compare the already calculated next time to the current time. So we compare more often and therefore more accurately.

Also consider

```
long now = System.nanoTime();
while (now > nextUpdate) {
update();
nextUpdate += UPDATE_INTERVAL;
updateCount++;
}

while (now > nextDraw) {
draw();
nextDraw += DRAW_INTERVAL;
frameCount++;
}

if (now >= nextReset) {
nextReset += NANOSECONDS_PER_SECOND;
double interval = (double) (now - lastReset);
double fps = frameCount / interval;
double updatesPerSecond = updateCount / interval;
System.out.println(

StackExchange Code Review Q#159571, answer score: 5