Re: [fluid-dev] Thread safety

[josh]

Hello David,

Looks like you have done a pretty thorough analysis of FluidSynth  
threading, something which I have not done and still lack some  
knowledge concerning the current code base.  Despite this, I'll try to  
be intelligent in my questions and opinions ;)

Quoting David Henningsson <address@hidden>:
> I've been looking at the threading strategy in Fluidsynth, which seems
> to need some review. While thinking about this, my first priority have
> been to avoid concurrency issues, at all costs (an underrun is bad, but
> a segfault is 100 times worse), and my second priority has been to avoid
> unnecessary thread synchronization costs when it is not needed.
>
> Given that priority list, we should make the fluid_synth_t class
> single-threaded and avoid mutexes altogether. One reason for this is
> that one of Fluidsynth's most important usages are as a virtual
> instrument in MusE/Rosegarden/etc, and there I assume it is used
> single-threaded anyway. And given the current behavior (some amount of
> synchronization but not fully functional), I think we could say that
> getting rid of the synchronization would be backwards compatible.

This would simplify things greatly and eliminate mutex locking stalls  
in the audio thread, which is good.  My only concern is if there is  
any software which currently uses FluidSynth in a multi-threaded  
fashion and expects it to work, which would require changes in said  
software, were we to make the FluidSynth synthesis core single  
threaded.  I think this number is likely pretty low and I don't think  
it should prevent us from making these changes, but it would be good  
to let the authors of these programs know.  Although these programs  
might very well be unstable with the current FluidSynth.

> This means some additional work for the audio thread, i e it has to
> process midi events. I think this is unavoidable for the moment. Btw, if
> we want to speed up audio processing, I think parallellizing voices (i e
> two CPU cores could render two different voices simultaneously) could be
> a fun thing to investigate. But that is in the distant future (at least
> not scheduled for 1.1.0).


Processing voices amongst multiple processors would indeed be cool.   
Perhaps we should keep that in mind as things are redesigned, but save  
it for a future version, as you suggest.  I think there is a lot that  
can be done to optimize and clean up the current synthesis core, which  
would be a good place to start in improving performance.  The last  
work I did on this was to improve the interpolation algorithm (which  
wasn't previously interpolating around loop start/end points).  I was  
able to improve the interpolation and speed things up a little at the  
same time.


> Back to the threading. Leaving fluid_synth_t single-threaded means that
> other threads cannot call fluid_synth_noteon and friends. For the midi
> player, this is already implemented with the sample timers. For the midi
> drivers, I'm working on a patch that will use the sequencer as a buffer
> for this case.


I wonder what applications currently use these explicitly and outside  
of the audio thread?  I can name one at this point, Swami.  But that  
is easy enough for me to change.  I'm still not seeing exactly how  
this new single threaded architecture would operate though.  Would the  
sequencer act as a queue for events from other threads?  Or would the  
requirement be that the audio thread would process these events itself  
somehow?


> So the thread synchronization is limited to the sequencer. Here's the
> suggested rules for using the sequencer in a thread-safe way:
>
> - Only one thread at a time can call fluid_sequencer_process. This would
> normally be the audio thread, via the sample timer callback.
> - Any thread can call fluid_sequencer_send_at. The shortcut from
> fluid_sequencer_send_at to fluid_sequencer_send_now is removed. This is
> a subtle change of behavior but it is more consistent with the
> documentation (i e "MAKES A COPY").
> - fluid_sequencer_get_tick can be called from any thread (and should be
> implemented with glib atomic integers).
> - fluid_sequencer_send_now and fluid_sequencer_process are the only
> public API calls that can result in midi events being sent. Midi events
> are sent before the call returns and in the context of the calling thread.
> - Don't mess around with registering clients, or anything else, while
> another thread makes a call to the sequencer.
>
> A question would be whether this means we should leave up to the
> libfluidsynth user to insert a sequencer in the midi processing chain,
> or if we should create it automatically behind the scenes somehow (and
> if so, what component should create it?). What do you think?

I don't think I fully understand this last bit.  I guess all events  
would go through the sequencer?  If that were the case, I could see  
how the fluid_synth_t would create it.  Maybe even the  
fluid_synth_noteon and friends could go through this?

> // David

I think my understanding of your proposed changes is still not  
complete.  Any assistance in helping me to understand would be  
appreciated.

Cheers!

Josh