Java Forum / General / November 2006
Threading design question
Big Jim - 24 Nov 2006 10:04 GMT Guys, I've a loop where I receive a prompt when I need to go to the DB, check if there's anything I need to process and if there is, process it. I also want to poll periodically if I don't get a prompt within a certain time period in case the prompt has failed for any reason. I'm currently using this logic:
// listener is the thread that listens for prompts // "this" is also a thread allowing the listener to interrupt us when it gets a prompt listener.initialise(this); listener.start();
while(true) { try { Thread.sleep(pollSeconds * 1000); } catch(InterruptedException ex) { log.info("Interrupted ..."); } publisher.doProcessing(); }
So, basically I'm using the Interrupted exception to break out of the sleep early. What I'm wondering is whether this is a poor way to do this as I'm using the exception handling mechanism for normal processing i.e. it's not an exceptional situation. Is this poor? Is there a more standard way to do this?
Thanks, Richard.
Tom Forsmo - 24 Nov 2006 12:42 GMT > // listener is the thread that listens for prompts > // "this" is also a thread allowing the listener to interrupt us when it [quoted text clipped - 7 lines] > { > Thread.sleep(pollSeconds * 1000); if this object extends Thread, you don't need to prefix it. if its a Runnable, using Thread.getCurrent().sleep() is the correct way.
> } > catch(InterruptedException ex) [quoted text clipped - 6 lines] > So, basically I'm using the Interrupted exception to break out of the sleep > early. I think this is a wrong interpretation., IE is mandatory and only used in case something causes it to interrupt as en error sort of. What I mean to say is that you do not control what interrupts it, unless you have some code you are not showing us.
> What I'm wondering is whether this is a poor way to do this as I'm using the > exception handling mechanism for normal processing i.e. it's not an > exceptional situation. Is this poor? Is there a more standard way to do > this? I'm sure there is a pattern for this kind of task, I just cant recall what its named. You could use a job scheduler such as Quartz for this, unless its to much for just one single task.
Or you could make a design of it yourself, where you integrate a sort of messaging queue (if you need this to work for several tasks), where the queue request blocks for x ms or is checked every x ms with an occasional execution of the job if nothing is in the queue.
Since you are talking about waiting for seconds and there only seems to be one job, the only thing you need to do is integrate the prompting into the loop as describe in the above paragraph.
tom
Big Jim - 24 Nov 2006 14:33 GMT > I think this is a wrong interpretation., IE is mandatory and only used in > case something causes it to interrupt as en error sort of. What I mean to > say is that you do not control what interrupts it, unless you have some > code you are not showing us. yes, my "prompt listener" calls back to this class using the interrupt() method, that's what I meant in the bit below that says:
>> // listener is the thread that listens for prompts >> // "this" is also a thread allowing the listener to interrupt us when it >> gets a prompt >> listener.initialise(this); >> listener.start(); does that change your answer at all?
>> // listener is the thread that listens for prompts >> // "this" is also a thread allowing the listener to interrupt us when it [quoted text clipped - 47 lines] > > tom Tom Forsmo - 24 Nov 2006 18:46 GMT >> I think this is a wrong interpretation., IE is mandatory and only used in >> case something causes it to interrupt as en error sort of. What I mean to [quoted text clipped - 11 lines] > > does that change your answer at all? Except for the part about the interrupt, everything else I said is still valid. You could also have a look at java.util.Timer. But then again I don't know all the details of your project, so its up to you to decide.
Ed - 24 Nov 2006 12:50 GMT Big Jim skrev:
> Guys, I've a loop where I receive a prompt when I need to go to the DB,
> while(true) > { [quoted text clipped - 17 lines] > > Thanks, Richard. Well, yes: using exception-handling to manage normal execution-flow is usually a bad idea.
But you're not doing that in this case. When the sleep expires, it does not throw the InterruptedException; it merely leaves the try/catch block (skipping over the catch()).
You can see the behaviour if you re-write the above as: private void go() { try { Thread.sleep(1000); publisher.doProcessing(); } catch (InterruptedException e) { System.out.println("Disaster!"); } }
.ed
-- www.EdmundKirwan.com - Home of The Fractal Class Composition.
Download Fractality, free Java code analyzer: www.EdmundKirwan.com/servlet/fractal/frac-page130.html
Big Jim - 24 Nov 2006 14:37 GMT > Big Jim skrev: > [quoted text clipped - 48 lines] > Download Fractality, free Java code analyzer: > www.EdmundKirwan.com/servlet/fractal/frac-page130.html but I am calling interrupt() in the listener thread when I get a prompt hence using the exception mechanism to break early from the sleep and go and process what's in th DB immediately, which is still normal processing:
>> // listener is the thread that listens for prompts >> // "this" is also a thread allowing the listener to interrupt us when it >> gets a prompt >> listener.initialise(this); >> listener.start(); any opinions on that?
wesley.hall@gmail.com - 24 Nov 2006 15:26 GMT > but I am calling interrupt() in the listener thread when I get a prompt > hence using the exception mechanism to break early from the sleep and go and > process what's in th DB immediately, which is still normal processing: You are right, it is not a big deal in the grand scheme of things, but it would be slightly preferable to declare a class level field like this...
private Object waitLock = new Object();
then do...
while(true) { try { synchronized(waitLock) { waitLock.wait(pollSeconds * 1000); } } catch(InterruptedException ex) { log.info("Interrupted ..."); } publisher.doProcessing(); }
Then to wake the thread do this...
synchronized(waitLock) { waitLock.notify(); }
This will do what you are doing but without throwing the InterrupedException as a matter of course (it still could throw it though, so you must handle this).
Big Jim - 24 Nov 2006 15:46 GMT >> but I am calling interrupt() in the listener thread when I get a prompt >> hence using the exception mechanism to break early from the sleep and go [quoted text clipped - 35 lines] > InterrupedException as a matter of course (it still could throw it > though, so you must handle this). Thanks Wesley, that's the sort of thing I was thinking of, I guess I'd provide the synchronized(waitLock){waitLock.notify();} in a public "wakeUp" method that my prompt listener could call instead of interrupt() which would also remove the need for my processing class to be a thread.
Just out of interest, which method do you think would be more efficient - The one that's always using the execption handling mechanism by explicitly calling interrupt() or the one with the synchronization overhead?
wesley.hall@gmail.com - 24 Nov 2006 16:20 GMT > Just out of interest, which method do you think would be more efficient - > The one that's always using the execption handling mechanism by explicitly > calling interrupt() or the one with the synchronization overhead? Excellent question. The answer is a I truely dont know.
My initial reaction would be to tell you not to get too wrapped up in very small performance considerations (especially in loops where you wait for much longer periods of time) and go with the version you prefer from a style and form perspective.
If you think this could result in a big performance bottleneck (if the thread could be woken > 100 times a second), then the best thing to do would be to run a few simple benchmarks and see what you get.
Big Jim - 24 Nov 2006 18:32 GMT >> Just out of interest, which method do you think would be more efficient - >> The one that's always using the execption handling mechanism by [quoted text clipped - 11 lines] > thread could be woken > 100 times a second), then the best thing to do > would be to run a few simple benchmarks and see what you get. It probably doesn't matter in this case, just for interest though, if I get time I'll test it and post the results.
Thanks, Richard.
Daniel Pitts - 24 Nov 2006 18:54 GMT > >> but I am calling interrupt() in the listener thread when I get a prompt > >> hence using the exception mechanism to break early from the sleep and go [quoted text clipped - 46 lines] > The one that's always using the execption handling mechanism by explicitly > calling interrupt() or the one with the synchronization overhead? When designing an system, don't worry so much about overhead and speed. Why? Once you have an easy to understand system in place, it is easier to change small areas of the system without destroying others. Once you have this, run a profiler. The profiler will tell you where your system is taking the most time.
You would often be surprised by what actually takes time in your system. I've been surprised many times myself. I once thought I was going to have to find a new algorithm for drawing an arc, but it turned out the delay was caused by a slow angle addition. It was a much easier fix.
In any case, I think you'll still need a thread. waitLock.wait(time); will block the thread that executes it. Also, wait isn't guarantied to block the full time, even if no notify is called.
The best way to handle this is to have some sort of condition in a loop.
public void waitToStartProcessing() throws InterruptedException { long timeUntilEnd = System.currentTimeMillis() + pollSeconds * 1000; while (System.currentTimeMillis() < timeUntilEnd) { synchronized(waitLock) { waitLock.wait(timeUntilEnd - System.currentTimeMillis()); } } }
public void processWhenReady() throws InterruptedException { waitToStartProcessing(); doProcessing(); }
public void forceProcessing() { synchronized(waitLock) { waitLock.notifyAll(); } }
Hope this helps.
Daniel.
Christian Kaufhold - 24 Nov 2006 19:07 GMT > public void waitToStartProcessing() throws InterruptedException { > long timeUntilEnd = System.currentTimeMillis() + pollSeconds * [quoted text clipped - 3 lines] > waitLock.wait(timeUntilEnd - > System.currentTimeMillis()); Danger here: If System.currentTimeMillis() changes between the two calls, the argument may become zero, which means waiting forever.
> } > } > }
> public void processWhenReady() throws InterruptedException { > waitToStartProcessing(); > doProcessing(); > }
> public void forceProcessing() { > synchronized(waitLock) { > waitLock.notifyAll(); > } > } This does not work, you now need an extra flag for that (as usual).
Christian
Daniel Pitts - 24 Nov 2006 19:28 GMT > > public void waitToStartProcessing() throws InterruptedException { > > long timeUntilEnd = System.currentTimeMillis() + pollSeconds * [quoted text clipped - 6 lines] > Danger here: If System.currentTimeMillis() changes between the two calls, > the argument may become zero, which means waiting forever. Ah, yes. You are absolutely right.
private boolean stillWaiting = true;
public void waitToStartProcessing() throws InterruptedException { long timeUntilEnd = System.currentTimeMillis() + pollSeconds * 1000; long curTime = System.currentTimeMillis() ; synchronized(waitLock) { while (curTime < timeUntilEnd || stillWaiting) { waitLock.wait(timeUntilEnd - curTime); curTime = System.currentTimeMillis(); } } }
> This does not work, you now need an extra flag for that (as usual). public void forceProcessing() { synchronized(waitLock) { stillWaiting = false; waitLock.notifyAll(); } }
> Christian I wrote it on the fly, but this version should work much better.
Although, I seem to recall that Lock has a better mechanism for this... <http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/locks/Lock.html.> Read that and the Condition class. I believe Condition has an "awayUntil(Date date)" method, which is exactly what you want.
Christian Kaufhold - 24 Nov 2006 21:47 GMT [wait for flag or timeout]
> Although, I seem to recall that Lock has a better mechanism for this... > <http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/locks/Lock.html.> > Read that and the Condition class. I believe Condition has an > "awayUntil(Date date)" method, which is exactly what you want. It still may wakeup spuriously.
Christian
Daniel Pitts - 25 Nov 2006 19:21 GMT > [wait for flag or timeout] > [quoted text clipped - 4 lines] > > It still may wakeup spuriously. Ah, you are right.
Well, anyway, the techniques described here are useful anyway.
You could also use the ScheduledThreadPoolExecutor class as well. This might be the cleanest solution. <http://java.sun.com/j2se/1.5.0/docs/api/java/util/concurrent/ScheduledThreadPool Executor.html>
Add the task to be executed, and save the Future object. When the user says to execute now, cancel the Future, and execute the task.
Wesley Hall - 24 Nov 2006 23:36 GMT > When designing an system, don't worry so much about overhead and speed. > Why? Once you have an easy to understand system in place, it is easier > to change small areas of the system without destroying others. Once > you have this, run a profiler. The profiler will tell you where your > system is taking the most time. I couldn't agree more.
> You would often be surprised by what actually takes time in your > system. I've been surprised many times myself. I once thought I was > going to have to find a new algorithm for drawing an arc, but it turned > out the delay was caused by a slow angle addition. It was a much > easier fix. I've been there too :)
> In any case, I think you'll still need a thread. waitLock.wait(time); > will block the thread that executes it. Also, wait isn't guarantied to > block the full time, even if no notify is called. > > The best way to handle this is to have some sort of condition in a > loop. <snip more complex code>
I disagree with your assertion... "The best way". It would be more accurate to say that your code may help to provide wait times that are closer to real-time than a simple call to wait(time) because in this form time is not real-time accurate.
"The best way (tm)" would be the simplest that get the job done, which may well be the OP's original solution. The wait approach would provide the same solution without routinely throwing exceptions, so is worth consideration.
The more complex solution that you provide is workable, but in reality, unless a 'very close to real-time' property is required (and given the nature of the original code, I don't imagine that complete millisecond accuracy is required, it is probably overkill.
Infact, it might be argued that it would not be conducive with the statement you make, "don't worry so much about overhead and speed.". Then again, perhaps the simple, 'wait' solution would too and exception catching is OK in this simple scenario :)
Daniel Pitts - 25 Nov 2006 21:33 GMT > I disagree with your assertion... "The best way". It would be more > accurate to say that your code may help to provide wait times that are > closer to real-time than a simple call to wait(time) because in this > form time is not real-time accurate. "wait(2000)" may immediately return spuriously, in which case it was useless. My code does more than approach real-time accuracy (actually, it doesn't even do that), it just ensures that at least pollSeconds have elapsed.
I should be more careful when I use the phrase "The best way". In any case, the "most commonly accepted way" (which is what I meant by "the best way") to use the "wait" methods is to have the wait call in a conditional loop. The use of wait and notify is often to allow one thread to halt execution until a certain condition is true. the notify methods should be called when the specified condition has changed, but the wait may spuriosly return when that condition has NOT changed.
The condition that the OP stated was: Either a poll timeout or a user request has occured. So, I assert that given the OP's intent and common best-practices, my solutions is likely "the best way". (Note the "likely".)
Although there are many ways two achieve his goal. Some are more flexible and provide clearer intent than others.
- Daniel.
Free MagazinesGet these publications absolutely FREE for up to 12 months. There are no hidden fees and no obligation. Simply choose a title, complete the application form and submit it. Read more ...
|
|
|