views:

232

answers:

5

Hi,

I am using the following spinlock approach:

while(!hasPerformedAction()){
    //wait for the user to perform the action
    //can add timer here too
}

setHasPerformedAction(false);

return getActionPerfomed();

this basically waits for a user to perform an action and then returns it. Currently something requests an answer from the user before continuing, this is why I wait until input is received. However I was wondering if this is inefficient and if we are waiting for a while (i.e. <= 30 secs) will it slow down the pc that is running this app. Are there any other alternatives using this approach i.e. locks, semaphores if so what is the syntax?

Thanks,

Aly

+1  A: 

There's a gui in your tags, so I'll assume that the action is done in the GUI.

The "recommended" way to do this kind of thing is for the main thread to put itself to sleep (perhaps using wait()) and for the GUI action to notify() it back into wakefulness once the action has been performed.

Carl Smotricz
would the code be something like if(!hasActionPerformed){wait();}and in the action listener for the Gui component we set the actionPerformed and the call notifyAll()? Will errors be caused as the Gui stuff is done on the event dispatch thread?
Aly
Carl Smotricz
I see Paul Wagland has explained wait/notify better, if you want to do that.
Carl Smotricz
+1  A: 

There are some classes for that in java.util.concurrent.locks. Have a look at Class LockSupport

Regards Mike

DerMike
+3  A: 

In fact, not only is this inefficient, it is not even guaranteed to work, since there is no "happens-before" edge in the code that you are showing. To create a happens-before edge you need to:

  1. Access a volatile variable
  2. Synchronise on a shared resource
  3. Use a concurrent utils lock.

As mentioned in another comment, the easiest solution, is simply to ensure that your flag is a volatile variable, and simply throw a short sleep in your loop.

However, the best thing to do would be to synchronize/wait/notify on a shared variable.

The methods that you need to read up on are wait and notify. For a better description on how to use these, read this article. An example code snippet is shown below;

Thread 1

Object shared = new Object();
startThread2(shared);
synchronized (shared) {
  while (taskNotDone())
    shared.wait();
}

Thread 2

// shared was saved at the start of the thread
// do some stuff
markTaskAsDone();
synchronized (shared) {
  shared.notify();
}
Paul Wagland
could you perhaps provide a small example in code?
Aly
@Aly: Added a code sample for you.
Paul Wagland
Thanks, The shared variable in this case is a boolean, and we cannot synchronize on a volatile boolean so I'm still a bit stuck
Aly
@Aly: You can either use a different variable as a second piece of shared state, use an AtomicBoolean (as opposed to bool) or simply insert a sleep into your loop, since if the boolean is volatile, then it is threadSafe.
Paul Wagland
@PaulWagland thanks!
Aly
+2  A: 

What you've written is called busy looping, which you should never do.

You may want to keep doing that, but at least sleep a bit as to not be busy looping, which still wouldn't be that great:

while( !hasPerformedAction() ) {
    (sleep a bit here)
}

Another way to do it would to enqueue the user actions on a blocking queue: then you could simply be using a queue.take and the queue implementation would take care of the issue for you.

And another way would be to use a callback to be notified of the user actions.

Webinator
People look down on the simplistic "sleep a bit here" solution, but in practice a 1ms or 10ms `sleep()` drops the observed system load from 100% to close to 0%. The reason is that the app is spending most of its time sleeping: 1ms is a long time compared to the time taken to wake up, check a flag and go back to sleep. So while not elegant, this is a very pragmatic solution requiring minimal change.
Carl Smotricz
Having said that, *all* the other solutions offered here are more elegant, more efficient, more self-documenting, more robust... more better. ;)
Carl Smotricz
@Carl: The problem is that this is actually not guaranteed to work, since there is no guarantee that the thread will see the update that the other thread has made. Most of the time it *happens* to work, but that cannot be relied upon.
Paul Wagland
@Paul: You're right, of course; that's why I pointed the OP to your mechanism using wait/notify. OldEnthusiast and I hearken back to times when Java multithreading was a lot simpler ;) . I'll now reluctantly retract my upvote for this solution in view of the danger of it not working.
Carl Smotricz
+2  A: 

The java tutorials have a good lesson on concurrency in java:

http://java.sun.com/docs/books/tutorial/essential/concurrency/

If you are going to be modifying UI from another thread, you will want to remember to do:

SwingUtils.invokeLater(actionToInvoke);
aperkins