[ros-users] need help: set_parameters service callback very sluggish with nodelet-based camera1394 driver

[Andreas Tropschug]

On Sat, 2011-01-29 at 23:28 -0600, Jack O'Quin wrote:
> On Sat, Jan 29, 2011 at 10:10 PM, Blaise Gassend
> <blaise at 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.)
> 

> Is this not the defining use case for pthread yield() ?
It preempts and puts the current thread at the end of the run queue,
effectively directly going to the waiting thread which acquires the
lock. If there is no thread in the run queue, it immediately resumes.

http://www.kernel.org/doc/man-pages/online/pages/man3/pthread_yield.3.html



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