On Sat, Jan 29, 2011 at 10:10 PM, Blaise Gassend
<blaise@willowgarage.com> wrote:
> Hi Jack,
>
>> The idea is to keep the lock, but add a bool class member for when the
>> reconfig callback needs to interrupt the poll() thread. The poll()
>> loop is almost the same:
>
> I guess your method is a small improvement over doing the sleep each
> time, as it sometimes avoids the sleep. It isn't clear to me that
> there is a benefit over doing the sleep every time. Retrospectively, I
> probably shouldn't have dissed the "sleep each time" method in my
> initial reply. It really solves the problem with maximum simplicity.
> There is really nothing wrong introducing a few hundred milliseconds
> of sleep in a loop is just going to waits on hardware for tens of ms.

The sleep is simplest and does seem to work.

I wanted to avoid the sleep while actively polling. In general, we
don't know how fast the camera is providing data. Even a millisecond
sleep in that loop could limit the frame rate on slow machines with
very fast cameras. (Unlikely, but possible: I have seen Bayer decoding
in the poll loop slow down the frame rate on some machines.)

> The benefit of my method is that it guarantees that once the configure
> thread starts waiting, the other thread will never start a new
> iteration before configuration completes. And I agree that in your
> case that guarantee is completely unnecessary. (My case was a bit more
> complex as the two threads needed to rendez-vous.)
>
>> I've thought about portability to machines with weak storage ordering,
>> but I think the scoped_lock should always invoke any memory fences
>> required for the underlying processor architecture, so it should be
>> OK.
>
> I think that technically you should mark your flag as volatile,
> otherwise the compiler can legitimately assume that it does not change
> in other threads, and assume that the condition in the while loop is
> constant. In practice this is probably a non-issue as you are calling
> a function in a different module from the while loop, so the compiler
> can't be sure that the function does not change the flag. However, if
> the function being called was in the same file, the compiler might
> inline it and consider that the condition in the while does not
> change.

Good point. I'll declare the flag volatile. The state_ should be volatile, too.
-- 
 joq