• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Bear Bibeault
  • Devaka Cooray
  • Liutauras Vilda
  • Jeanne Boyarsky
Sheriffs:
  • Knute Snortum
  • Junilu Lacar
  • paul wheaton
Saloon Keepers:
  • Ganesh Patekar
  • Frits Walraven
  • Tim Moores
  • Ron McLeod
  • Carey Brown
Bartenders:
  • Stephan van Hulst
  • salvin francis
  • Tim Holloway

Thread question  RSS feed

 
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 ]
 
(instanceof Sidekick)
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 ]
 
Wanderer
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.
 
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.
 
Consider Paul's rocket mass heater.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!