PShape generation in other threads

Median

In my program, I have a main PShape group which comprises multiple child PShape groups (Chunks), which again have many PShapes in each. Each draw call this main PShape group is drawn. The program adds and removes chunks from this main group depending on a view port. When a new chunk is added for the first time it needs to be generated i.e. filled up with data and a PShape group for the chunk needs to be generated. Generating a chunk with all of its children is a pretty heavy task. A task which is needed to be done without bogging down the main draw loop, as this will be used in a real time application (a game :)).

I have tried to make a separate thread where the chunks are 'created', but it makes almost no difference in performance, the application still lags quite heavily when generating chunks. It seems like somehow the creation of PShapes, even though it is in another thread, requires the PApplet instance and somehow pauses it when creating PShapes. Here is the code for my thread:

public class ChunkRenderThread extends Thread
{
    private boolean running;

    ArrayList<Chunk> queue; //the list to render
    ArrayList<Chunk> result; //the generated chunks

    public ChunkRenderThread()
    {
        queue = new ArrayList<Chunk>();
        result = new ArrayList<Chunk>();
    }

    public void start()
    {
        System.out.println("Starting chunkRender thread ... ");
        running = true;
        super.start();
    }

    public void run()
    {
        while(running)
        {
            if (queue.size() > 0) //are the any chunks that need to be rendered?
            {
                Chunk c = queue.get(0);
                c.getShape(); //the shape will be generated and stored by the chunk itself
                queue.remove(c);
                result.add(c);
            }
        }
    }

    public void quit()
    {
        System.out.println("ChunkRender thread " + this + " quit.");
        interrupt();
    }

    public void addChunk(Chunk chunk)
    {
        queue.add(chunk);
    }

    public boolean anyDone()
    {
        return !result.isEmpty();
    }

    public Chunk getResult()
    {
        if (!result.isEmpty())
        {
            Chunk chunk = result.get(0);
            result.remove(chunk);
            return chunk;
        }
        return null;
    }
}

(it should be noted that I use an external IDE)

There is a chunkMangement instance which calls the method addChunk() when a new chunk is loaded. It then later checks anyDone() and subsequently takes and adds the chunk to the main PShape group.

Can anybody give some advice? I'm pretty stuck and out of ideas... :|

quark

You while loop is too 'tight' it gives no time for the other thread. Add a sleep if it has nothing to do.

Also the quit() method does NOT halt the thread it simple interrupts it. The only way to stop the thread is by setting the value of running = false at the moment your thread continues forever.

public void run()
{
    while(running)
    {
        if (queue.size() > 0) //are the any chunks that need to be rendered?
        {
            Chunk c = queue.get(0);
            c.getShape(); //the shape will be generated and stored by the chunk itself
            queue.remove(c);
            result.add(c);
        }
    else {
        try {
          sleep(200);
        } 
          catch(InterruptedException e) {
        }
    }
}

GoToLoop

@quark, sleep() isn't part of PApplet and can't be invoked directly. Rather, it shoulda been Thread.sleep():
http://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#sleep-long-

Actually, that's why convenient method delay() is for...
Although there are more folks who doesn't understand how static Thread.sleep() works: 8-}
https://github.com/processing/processing/pull/2890

quark

@GoToLoop look more carefully and you will see that sleep is being called from inside a class that extends Thread so my code is just fine. In fact I tested it using Processing and the OP is actually using an external IDE. So Thread.sleep() is not needed in this context. :))

GoToLoop

Oh, that's true! Generally folks implements Runnable rather than extends Thread! 8-|
Nonetheless, there'll be less boilerplate replacing sleep() w/ delay()! ;)

Well, unless it doesn't extends PApplet!
In which case, if my pull propose woulda been accepted, we'd be able to invoke PApplet.delay()!
But ignorance and intransigence kicked the static for delay() out! X(

quark

Generally folks implements Runnable instead of extends Thread!

It would make no difference whether it implements Runnable or extends Thread the run method is still executed in a separate thread so Thread.sleep() is still inappropriate.

Nonetheless, there'll be less boilerplate replacing sleep() by delay()!

the delay method is simply a wrapper for Thread.sleep() so again would be inappropriate in this context. ;))

GoToLoop

Got some extra points:

remove(Object) is O(n). We should strive to stick w/ remove(int) for List:

• http://docs.oracle.com/javase/8/docs/api/java/util/List.html#remove-java.lang.Object-
• http://docs.oracle.com/javase/8/docs/api/java/util/List.html#remove-int-

ArrayList isn't appropriate for Queue operations. We should rely on ArrayDeque instead:

• http://docs.oracle.com/javase/8/docs/api/java/util/ArrayDeque.html

GoToLoop

... so Thread.sleep() is still inappropriate.

Wasn't you who suggested just that, in order to loose the while () loop a lil'? (:|
I'm only suggesting that convenient wrapper PApplet's delay() method removes the try/catch boilerplate from our sight! :)>-

Median

Unfortunately adding @quark s code doesn't fix the problem, but I'll leave it there for good measure.

@GoToLoop thanks for tip about using remove(int) and ArrayDeque! I will change it at some point, but the lists contains <15 items, so I don't believe that to be the problem.

I've "transferred" the code so processing accepts it, here it is. When running it in the native Processing IDE, I noticed that it actually ran noticeably faster. I've also changed the code a bit, separating the code which generates the chunk PShape and the code that the returns the PShape of the chunk. This ensures that the method which generate the chunk PShape is only called by the RenderThread, but it still doesn't work :/

quark

Unfortunately adding @quark s code doesn't fix the problem

After looking at your code I am not surprised it didn't work. This thread is not doing any work the getShape method simply returns a reference to a PShape, it doesn't create it. I assumed it was a heavy duty method to be executed in a thread.

public PShape getShape()
{
    return chunkShape;
}

Looking at the Chunk class I see that all the work is being done in the constructor and I suspect that the Chunk objects are being created in the main Thread so clogging it up. Try modifying the class so the Chunks are created in this thread.

NOTHING you do to this run() method is going to help you.

So my suggestion is still valid advice because a very tight loop e.g.

while(true){ // Nothing to do? }

will hog the processor. Putting in a sleep statement is a one way to relinquish control for a short period and give other threads a chance.

@GoToLoop

Wasn't you who suggested just that, in order to loose the while () loop a lil'?

Yes that's true and I provided code to do just that. I also suggested that you misread the original post so your comments about using Thread.sleep() and delay() were not appropriate in this scenario.

Also the quit() method does NOT halt the thread it simple interrupts it. The only way to stop the thread is by setting the value of running = false at the moment your thread continues forever.

You never mentioned this but it is still a valid statement because the proper way to stop a thread is to allow the run() method to complete and exit.

GoToLoop

I also suggested that you misread the original post...

I thought it was clear I was talking directly to you by starting my reply w/ your nick!
Of course I wasn't replying to the thread's question at that time! >-)

Median

@quark

I assumed it was a heavy duty method to be executed in a thread.

That is what I changed:

I've also changed the code a bit, separating the code which generates the chunk PShape and the code that the returns the PShape of the chunk. This ensures that the method which generate the chunk PShape is only called by the RenderThread

The ChunkRenderThread class calls initShape() on the chunks, which I thought meant that that method was executed in that thread. Is this not correct?

GoToLoop

Methods are independent from threads. A method can't change the thread that invoked it!
For example, even though addChunk() method belongs to class ChunkRenderThread, any Thread can call it!

Median

So, since the thread calls the initShape() method in the Chunk object (which is the method which initializes PShape and adds a lot of PShapes to it (a demanding method)), the method "runs" on that thread, correct? If yes, I'm still at a loss as to where the lag is coming from :(

quark

In the code you posted for the ChunkRenderThread there was no call to initShape(), so I am puzzled that it appears in the code I downloaded.

Although initShape() is doing a lot of work, it only does it once to create the PShape.

Am I right in assuming a ChunkTile is 16x16 pixels of a single colour and a Chunk is 32x32 ChunkTiles?

I noticed that the ChunkManager and DataHandler classes both extend Thread but neither has a run method. So why?

It would be better if the entire Chunk, its associated ChunkTiles and their PShapes are created in ONE place rather than splitting it over several calls from several locations. I also suggest that you have a single separate Thread that is used solely for the creation of these Chunks.

Median

In the code you posted for the ChunkRenderThread there was no call to initShape(), so I am puzzled that it appears in the code I downloaded.

It was an experiment, and also to separate the code. Sorry for the confusion.

Am I right in assuming a ChunkTile is 16x16 pixels of a single colour and a Chunk is 32x32 ChunkTiles?

Yes, except the 16x16 pixel rectangle is merely a placeholder.

I noticed that the ChunkManager and DataHandler classes both extend Thread but neither has a run method.

Where do you see this?

I also suggest that you have a single separate Thread that is used solely for the creation of these Chunks.

I forgot to mention that I actually tried this by having the chunkManager class be a thread in itself, but that didn't work either.

quark

DataHandler classes both extend Thread

No it doesn't my mistake. :\">

Apart from the last paragraph in my last comment I am not sure what to suggest.

Which IDE are you using? Is it possible to get a Java profiler for it. This might help show which methods are using the most time.

Median

Thanks for all your help. Unfortunately nothing has really worked, but I did learn some new things about threads which is nice :). Tanks for that. Sorry for bothering you all with my problem. I will try to make it work with some kind of shader instead.