Threading design question

[Big Jim]

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]

// 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);

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)
{
log.info("Interrupted ...");
}
publisher.doProcessing();
}

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

 

[Ed]

Big Jim skrev:

Guys, I've a loop where I receive a prompt when I need to go to the DB,

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.

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

 

[Big Jim]

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:
does that change your answer at all?

 

[Big Jim]

Big Jim skrev:



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

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:

any opinions on that?

 

[wesley.hall]

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]

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).

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]

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]

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.

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.

 

[Tom Forsmo]

yes, my "prompt listener" calls back to this class using the interrupt()
method, that's what I meant in the bit below that says:
>

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.

 

[Daniel Pitts]

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?

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]

public void waitToStartProcessing() throws InterruptedException {
long timeUntilEnd = System.currentTimeMillis() + pollSeconds *
1000;
while (System.currentTimeMillis() < timeUntilEnd) {
synchronized(waitLock) {
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]

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]

[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

 

[Wesley Hall]

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]

[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.

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/ScheduledThreadPoolExecutor.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.

 

[Daniel Pitts]

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.