2

I'm very new to multithreading - I've done a few hours of research and tested a few different ways. I've recently implemented this code:

public class resetDataThread implements Runnable {
    VertexHashMap allVertices;
    public resetDataThread(VertexHashMap allVertices){
        this.allVertices = allVertices;
    }            

    @Override
    public void run() {
        for (int i = 0; i < allVertices.data.length; i++)
        {
            if (allVertices.data[i] != null){
                allVertices.data[i].inClosed = false;
                allVertices.data[i].inOpen = false;
            }           
        }        
        System.out.println("Thread Finished!");        
    }    
}

Please note, VertexHashMap is a self implemented hash map that stores Vertices.

Runnable r = new resetDataThread(allVertices);
Thread t1 = new Thread(r);

I call the method like so: 

t1.run();

This line of code gets called many times without creating new instances of r or t1. My reason for implementing this is so my program can fetch new data and output results while it resets the storage's boolean variables (allVertices). My main aim is execution speed of my program, so I figured I could multi thread this section of code.

So my question is, is this safe, suitable or a good way to do this?

Example

...code that changes allVertices variables
t1.run(); //this will run while otherSectionOfProgram and yetAnotherSectionOfProgram is executed
otherSectionOfProgram();
yetAnotherSectionOfProgram();

The example above can be called many times however the contents of allVertices will always be the same (except for the boolean settings), so I'm just wanting to make sure it's okay to use the run() method the way I have - as I want to ensure I practise good java/programming.

Craig
  • 675
  • 1
  • 7
  • 23
  • Here's a mantra that you can recite to contemplate the several answers that you got below: `start()` is the method that the library provides for you to call when it's time to start a new thread. `run()` is the method that _you_ provide for the _library_ to call _in_ the new thread. – Solomon Slow Apr 27 '15 at 21:50

5 Answers5

2

If what you're after is to have a single background thread running then call t1.start() to start a background thread.

If you want to get more advanced with creating threads look into the Executor Service for java Java Executor Service

It's also worth noting that unless allVertices is pretty large in size it shouldn't take much time to run the run() method anyway, but it's easy enough to time it to see how much difference it makes.

EDIT: Also make sure otherSectionOfProgram(); and yetAnotherSectionOfProgram(); do not modify allVertices if you aren't creating a new instance of that because your background thread will be operating on it as well.

codesmaller
  • 74
  • 7
  • Sorry if it is unclear - I am wanting it to operate on the same allVertices variable. Each time run() gets called, I'd like it to change all the booleans to false, as that is the 'starting' position, as allVertices is a global variable that holds data that will be used in future calculations, and those future calculations require the booleans to be false, else the calculation will fail. – Craig Apr 27 '15 at 18:26
  • With the way this is written each thread will do the same work, so it won't be any faster. – codesmaller Apr 27 '15 at 18:28
  • So your goal is to set all the booleans to false as quick as possible? – codesmaller Apr 27 '15 at 18:29
  • Won't it be doing the work in a thread while the main body of my program executes other lines of code? – Craig Apr 27 '15 at 18:29
  • Yes I'd like to set the booleans to false as quick as possible, however I need to do that while my main program executes separate lines of code. I've edited the main question to try and explain this better. – Craig Apr 27 '15 at 18:31
  • So it sounds like you're after creating a background thread to do some work while your main program is running? I thought you meant you were calling t1.run() in a loop – codesmaller Apr 27 '15 at 18:32
  • That's correct, however the whole code can get called multiple times, but allVertices will always keep the same content but the booleans stored in it need to be reset to false. – Craig Apr 27 '15 at 18:40
0
t1.run();

This line of code gets called many times

Call start() on your thread, not run().

What's the difference between Thread start() and Runnable run()

Community
  • 1
  • 1
Mike Samuel
  • 118,113
  • 30
  • 216
  • 245
0

You start created threads with t.start() method, t.run() won't start a new thread.

Also, calling t.start() multiple times still won't create a new thread each time it's called, your thread t is unusable(dead) after it's done once. You need to create new Thread for every ... well... thread.

zubergu
  • 3,646
  • 3
  • 25
  • 38
0

For starting new thread you should call start() method.
For program safety, you should take care about mutual exclusion, I should know logic of program to advise you more about the correct solution, but you may want synchronize allVertices if another sections of program may change it simultaneously or if you want to create more than one instance of your thread:

@Override
public void run() {
    synchronized(allVertices){
        for (int i = 0; i < allVertices.data.length; i++)
        {
            if (allVertices.data[i] != null){
                allVertices.data[i].inClosed = false;
                allVertices.data[i].inOpen = false;
            }           
        }
    }
    System.out.println("Thread Finished!");        
}
Ehsan Khodarahmi
  • 4,772
  • 10
  • 60
  • 87
0

OK, first of all, calling Thread.run() will not start a new Thread. It will run the method on the calling thread. To actually start a new thread you have to use Thread.start(). In the code you posted there is no multithreading happening.

Secondly, the line VertexHashMap is a self implemented hash map that stores Vertices. screams non-thread-safe to me. You know there is a special ConcurrentHashMap version of HashMap to deal with concurrent (i.e. multithreaded) use? Which structure did you base your class on? Did you make your own objects thread-safe? Inter-thread visibility problems may arise.

Thirdly, reusing threads is tricky business. Just calling run multiple times won't even make a single Thread, but calling start more than once will throw an exception. As some of the other answers pointed out - using an ExecutorService is a great alternative - you can set it up to use whatever number of threads you want (and have flexibility in how it behaves under load), and have it reuse its threads (instead of re-inventing the complex machinery behind it).

Finally - you may want to think about visibility of your objects. In your case you have the VertexHashMap as default visibility. This means that other classes can see and use it directly. This may be a problem if, for example, you are in the process of clearing it, or re-populating. Then the user gets a half-ready object, whose methods may easily fail. Java has a Future interface just for that - it's an interface to be used with multithreading that "promises" to return a ready object. It's the basis for Swings SwingWorker and Androids AsyncTask - objects that do tasks in the background thread, and then return ready results to the foreground for further use.

Ordous
  • 3,844
  • 15
  • 25