Win a copy of Design for the Mind this week in the Design forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Thread question

 
Brian Spindler
Greenhorn
Posts: 29
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Image you have a method doWork() that can take several minutes to complete, you want to spawn a bunch of these from your getWorkDone() method and wait for them to complete then return. Help me to improve the following code:


[ June 16, 2007: Message edited by: Brian Spindler ]
 
Stan James
(instanceof Sidekick)
Ranch Hand
Posts: 8791
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Look at ThreadPoolExecutor awaitTermination. Does that do what you need?

There are some other ways. I did exactly what you did before I found join(). This technique loses the benefits of your thread pool if you have more tasks than you'd like to have run all at once.

FutureTask is a bit cleaner than that because you don't have to touch the Threads. You could pass a bunch of FutureTasks to an executor and then call get() on each one.

Now you can control the number of threads working at the same time and reuse them if necessary.

Any of that sound good?
[ June 16, 2007: Message edited by: Stan James ]
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I like Stan's suggestions here. If you want to stick with the code you've got written though, the big problem I see is here:

Both setRunning() and getRunnint() are synchronized, but that's useless here, because there's a gab between the call to getRunning() and the call to setRunning(). It's equivalent to this:

To protect against interference from other threads, you would need a synchronized block or method that contains both method calls. For example:

or more simply:

This sort of problem is common with many synchronized methods - any time you need to call two or more different synchronized methods in succession, you need to consider what happens if another thread calls a method in between those method calls. Typically the answer is to place the synchronization at a higher level than it was before.
 
Nicholas Jordan
Ranch Hand
Posts: 1282
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In addition to suggestions posted by Stan and Jim.

The one thing that stands out for me is the Thread.sleep(1 * 1000);

I suggest a join(), or more preferrably CyclicBarrier as established practice. I always waste some processor cycles on checking some condition in a loop or something, considering any sort of wait() to be unreliable

Also, see: Concurrent Programming with J2SE 5.0
[ June 17, 2007: Message edited by: Nicholas Jordan ]
 
Brian Spindler
Greenhorn
Posts: 29
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks Ranchers! I think I've got enough to go on. At this point I'm leaning towards the FutureTask approach, it seems perfect for my needs. I need to submit N tasks and wait for them to finish processing before I can return the computation.

And thanks Jim for the heads up on the synchronization issues.
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic