3
\$\begingroup\$

I'm trying to make my game as friendly to multi-core as possible. as such, i'm putting each animation on a new thread. I was hoping someone could check out my proof of concept code in java;

an example of usage is as follows:

    ThreadedAnimation ta = new ThreadedAnimation();
    Thread tat = new Thread(ta);
    tat.start();
    ta.getCurrentFrame();

where the threadedAnimation class is as follows:

public class ThreadedAnimation implements Runnable{
boolean init = false, toFinish = false, toStop = false; //synchronized info
short[] delay; //delay in ms
Image[] frames; //array for the frames
public Image currentFrame = null; //for pulling the frame
int currentFrameNumber = 0; // simmillar representation. to go barebones, I would just return the int and not the number.

public ThreadedAnimation(short[] del, Image[] fram){
    if(del.length == fram.length){
        currentFrame = fram[0];
    }else{
        System.out.println("bad animation detected");
    }
}
public ThreadedAnimation(){
    delay = null;
    frames = null;
}

@Override
public void run() {
    if(init){
        while(!toFinish && !toStop){
            currentFrameNumber = 0;
            while(!toStop && !(currentFrameNumber> frames.length)){

                long currentTime = System.currentTimeMillis();
                while((System.currentTimeMillis()-currentTime) < delay[currentFrameNumber]){// The reason I don't put TrySleep(delay[i]) is because the thread is interrupted every time we try and get the image.
                    TrySleep(10);
                }
                currentFrameNumber++;
                currentFrame = frames[currentFrameNumber];
            }


        }

    }
}

private void TrySleep(int i) {
    try {
        Thread.sleep(i);
    } catch (InterruptedException e) {
        e.printStackTrace();
    }

}
public synchronized void  finish(){
    toFinish = true;

}

public synchronized void stop(){ //stops animation where it stands and kills
    toStop= true;

}

public synchronized Image getCurrentFrame(){
    return currentFrame;
}

}

\$\endgroup\$
  • 8
    \$\begingroup\$ "I'm trying to make my game as friendly to multi-core as possible. as such, i'm putting each animation on a new thread. " I think you've massively failed to understand how to effectively use multiple cores. :/ \$\endgroup\$ – Sean Middleditch Dec 11 '15 at 4:19
  • 1
    \$\begingroup\$ @SeanMiddleditch explain? \$\endgroup\$ – tuskiomi Dec 11 '15 at 4:25
  • 9
    \$\begingroup\$ An entire thread dedicated to something as simple as a flipbook animation is a waste in terms of resources (threads aren't free), doesn't scale across cores (what if you have fewer animations that you have cores? more animations than you have cores?), leads to a lot of inefficiencies (how are you planning to marshal animation updates and state changes to and from these threads?), won't time well (TrySleep isn't going to line up well with your renderer's draw loop), etc. Spawning a thread-per-task is generally just a bad way to use threads, and especially bad for a game. \$\endgroup\$ – Sean Middleditch Dec 11 '15 at 5:10
  • \$\begingroup\$ @SeanMiddleditch I'm not sure I understand. Why do you have to match up thread and cores? On PCs you can run 100s of tasks and can run them all seemingly simultaneously. \$\endgroup\$ – tuskiomi Dec 11 '15 at 17:34
  • 7
    \$\begingroup\$ not efficiently. The OS takes time to switch between contexts/thread. Each thread has a memory overhead for the stack. Communicating between threads is not free and can quickly become too complicated for the actual world's brightest engineers to understand and debug. There's a limit to the number of threads that a single process can spawn on most OSes. Efficient use of multicore hardware is best performed today with a "task system" design (software.intel.com/en-us/articles/… or github.com/RichieSams/FiberTaskingLib). \$\endgroup\$ – Sean Middleditch Dec 11 '15 at 21:10
2
+50
\$\begingroup\$

First off, threads are expensive. That does not mean that this technique cannot work, just that it is likely to be inefficient.

In your particular case, you will have threads waking up each 10 milliseconds, and as many threads as animations you have. If you will have a thread that wakes each 10 milliseconds anyway, you could use one thread per core (with affinity set to each core) and balance the animation among those threads. That would be more efficient. We could then consider updating each animation a “task”, and voila you have a rudimentary “task” system.

Or at the very least recycle your threads. Notice that when you kill the animation, the thread becomes garbage that the system has to collect and you will be creating a new – expensive – thread for any new animation. Do not do that, do a thread pool.

What you are doing with the animation thread is a fancy clock. It should be possible to calculate what should be the current frame of the animation given the currentTimeMillis and the delays.

If the delays are unpredictable, that means that to calculate the current frame you would: 1) get duration of the animation by adding the delays (value that be stored and reused). 2) Do a mod of the difference of the current time minus the start time by the duration of the animation. 3) Check the resulting reminder against the delays (complexity is O(n)). On the other hand, if the delays are follow a mathematical pattern, then it can probably done much more efficiently. For example, if each frame has the same delay, you do currentFrameNumber = ((currentTime - startTime) % (delay * frames.length)) / delay; (complexity is O(1)).

For your threading solution to be worth doing, it should be more efficient that doing that. Moreover, guess what, you can opt to do that only when you need to show one of the sprites, meaning that you would only have to do it for the sprites on screen. Meanwhile your threads will be working for every animation in the scene… behind the scenes. Or wasting CPU time each 10 milliseconds.

I guess there is some edge case where you will have too many animation, of which, all of them are always on the screen, and having some animation lag in exchange of responsiveness is acceptable... then I would agree that threading the animation is a good idea... but on a thread pool, please.


By the way, you should be able to use the delay to sleep... you say:

the thread is interrupted every time we try and get the image

That should not be the case! I do not know why it would be… I hope you are not pausing the thread to read it. If it is an artifact synchronized (it should not be), then I am going to tell you to get rid of that anyway.


Second, the presented code, I presume is incomplete, otherwise, it will never work because init will always be false.

WHY it takes so much to animate in this method.

It takes forever if init is false.

If I were you I would not have init and I would not have a default constructor that sets fields to null. If you need to do some work before starting the animation, you should be able to work with the delay and frames arrays before calling the constructor. In fact, I would suggest creating an animation class that has those arrays as members… but that is off-topic.


Third, what will be a problem – even for a singular correctly initialized animation – is thread visibility. The thread is updating the field currentFrame, but the CPU could be keeping the value of currentFrame in the local cache of the threads (after all this field is accessed often), resulting in changes made by the animation thread being hidden to (not visible by) other threads.

If this is the problem, it is not that it takes so much to animate in this method, instead that the render thread becomes aware of changes of currentFrame too late.

What you need in this case is volatile. You can turn currentFrameNumber, toStop and toFinish to volatile. You could also turn currentFrame to volatile, or read from frames in getCurrentFrame.

In fact, you not only do not need synchronized but it counterproductive, it is a waste of resources. The synchronized keyword ensures is that at most one thread enters any of the synchronized methods of the class at the same time. Well, multiple executions of finish or stop are idempotent so they don’t need this protection, and getCurrentFrame reads a variable that is being set on a method that is not synchronized (run), thus using synchronized on getCurrentFrame is useless. Furthermore, synchronized does not guarantee visibility… so yeah. Get rid of that.


Final note: You need a game loop that uses sleep to yield CPU time to the animation thread and any other thing you might have running in the background. If your game loop is eating all the CPU time and the animation threads are not getting enough then the animation are going to lag.

Yet, you probably should use thread for something more complex that a "flipbook animation", in particular something independent of the screen, so that there is some remarkable gain in using threads.*

\$\endgroup\$
  • \$\begingroup\$ Thanks. This code is from a long while ago, and I was looking more at the technicalities of it than the code itself. \$\endgroup\$ – tuskiomi Jul 11 '17 at 13:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.