[LV2] worker thread question

[David Robillard]

On Sat, 2012-12-29 at 19:27 +0100, hermann meyer wrote:
> Am 29.12.2012 18:59, schrieb David Robillard:
> > On Sat, 2012-12-29 at 08:55 +0100, hermann meyer wrote:
> >> Am 29.12.2012 05:22, schrieb hermann meyer:
> >>> Am 28.12.2012 23:59, schrieb David Robillard:
> >>>> More threads, more problems.  Do non-rt things in a worker, and I/O and
> >>>> processing in run().  More threads complicated things massively for no
> >>>> win.  The idea of a "watch thread" doesn't really make any sense.
> >>> Thanks for taking you time
> >>>
> >>> bye
> >>> hermann
> >> So I have add my own watch thread, for the case you like to review the
> >> source, here it is:
> >>
> >> http://sourceforge.net/p/guitarix/git/ci/11f2c1985e4c553ccd66b5fce2acd0a760923b49/tree/trunk/src/LV2/gxamp.lv2/gxamp.cpp
> >>
> >> I have just 2 float values to check here, so it wouldn't spare much
> >> processing, but as I said, that will properly change in the next time.
> > Oh my.  Bloat aside, this code is very broken.
> >
> > You can not call schedule_work from any thread.  It MUST be called from
> > the run() context, and ONLY the run() context.
> >
> > Once again, the entire point of the worker is to schedule non-RT work
> > from run().  If you aren't going to do that, then just don't use it.
> It's all non-rt in the worker thread, there isn't any rt-work in there. :-)
> 
> > You also seem to be reading port buffers from another thread?!  You can
> > not access port buffers from any context but run().  Since there is no
> > port lock or anything like that it should be obvious that this can't be
> > thread-safe.  This seems to be the main source of all your confusion:
> > ports are accessed in run(), and ONLY from run().  This is true for
> > control ports, audio ports, MIDI, and messages as used by the
> > eg-sampler.  It is a deliberate design decision.  If you want to compare
> > port values in another thread, then you will have to transport the
> > values to that other thread somehow (e.g. via a ringbuffer which is how
> > the worker is implemented).  This is why you won't speed anything up
> > that way.
> Those ports will never accessed from run(), they are desired to do only 
> non realtime work.

> When the worker access the ports, the watch thread is blocked.
> The values will ever only get read by those threads, never in run().
> There is no assumption anywhere that the values are actual at the time 
> they get read, it isn't simply not necessary in that context. I have 
> test this code in several hosts, without any problem.

Ports may only be accessed from run(), period.  This is not a choice the
plugin gets to make; the buffers are connected/set by the host.

That you happen to get lucky is beside the point.

> On the other side, may it is possible to introduce non-rt ports in the 
> API ?
> I will happily switch to use them then.

As I said, this was tried, and it was much, much more complex and raises
a massive set of problems for little to no benefit.  It is particularly
tedious to implement on the host side, which is why nobody implemented
it.  Doing I/O in run() is a deliberate design decision arrived at after
a lot of thought and experimentation.  It is this way for good reasons.

In short, you're breaking very important rules in order to "optimize"
away nothing significant, with a result that's much more complicated,
error prone, and slow than the simple and correct solution.

This code is broken and needs to be fixed.  Do not break the threading
rules of the API.  Seriously.

-dr

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.lv2plug.in/pipermail/devel-lv2plug.in/attachments/20121229/d7f901f3/attachment-0002.pgp>