Sync GUI from Audio Thread: any way to copy bulk of data with one instruction?

[mystran]

Also perhaps with some modifications the scheme could run so that the GUI always reads the latest update. I have an idea, but would want work it out fully before saying more, plus not sure it'd be really worth it

The scheme I outlined works fine even for running stuff like IIR filter banks in the GUI thread (eg. if you just want to draw one for visualisation, there's no need to waste ASIO time on it), without having to take a one-frame hit. The problem with the one (GUI) frame lag is not really so much with the latency itself, but if the GUI framerate is not totally stable (which is usually the case in practice) the variation makes animation jittery and inconsistent (well, much more so than just varying frame-rates on their own). This is obviously assuming that the audio is running with short buffers (if it's not, then things get really hairy if you want to keep the animation smooth; personally I've never bothered though).

[Nowhk]

I fixed it (I guess) writing this code:

Code: Select all

// audio thread (called every samples)
void EQ3Bands::ProcessControls(int controlRateBlockSize) {
	bool hasProcessed = false;
	hasProcessed = pFilterFrequency->Process(controlRateBlockSize) || hasProcessed;
	hasProcessed = pFilterQ->Process(controlRateBlockSize) || hasProcessed;
	hasProcessed = pFilterGain->Process(controlRateBlockSize) || hasProcessed;

	if (hasProcessed) {
		pFilter->SetCutOff(pFilterFrequency->GetWarpedValue());
		pFilter->SetQ(pFilterQ->GetWarpedValue());
		pFilter->SetGain(pFilterGain->GetWarpedValue());
		pFilter->CalculateCoefficients();

		isCoefficientsRefreshed = true;
	}

	// signal to update the gui thread
	if (isCoefficientsRefreshed && mMutexCoefficientsDraw.try_lock()) {
		pFilter->CacheCoefficients();

		isCoefficientsRefreshed = false;
		mMutexCoefficientsDraw.unlock();

		RedrawCachedBitmap();
	}
}

// gui thread (called at 60FPS)
bool EQ3Bands::Draw(IGraphics *pGraphics) {
	// ...

	mMutexCoefficientsDraw.lock();
	// calculate filter frequency response
	mMutexCoefficientsDraw.unlock();
	
	// ...
}

Audio thread: it update coeffs only if controls has been changed. Once they do, I place a flag to (later) draw them, calling a try_lock sync with the gui. Once there are coeffs, there is no lock and I can take control of that critical section (since try-lock won't do it sometimes), it updates the draw-coeffs (a copy of the real one) to be draw later, and it sends a message to the gui thread.

Draw thread: if audio thread is updating coeffs, block. Else, take the lock (atomic operation) and draw them.

It seems satisfy any requirements. What do you think about?

[stratum]

Sorry for making an unrelated comment, but the compiler optimizer may cause the second two lines not to be executed at all if the first sets hasProcessed to true. Probably it's not the case here but it's better to be suspicious (these kind of problems usually happen with if expressions (i.e. inside 'if (...)' ) and not outside ).

Code: Select all

hasProcessed = pFilterFrequency->Process(controlRateBlockSize) || hasProcessed;
   hasProcessed = pFilterQ->Process(controlRateBlockSize) || hasProcessed;
   hasProcessed = pFilterGain->Process(controlRateBlockSize) || hasProcessed;

[Nowhk]

Sorry for making an unrelated comment, but the compiler optimizer may cause the second two lines not to be executed at all if the first sets hasProcessed to true. Probably it's not the case here but it's better to be suspicious (these kind of problems usually happen with if expressions (i.e. inside 'if (...)' ) and not outside ).

Code: Select all

hasProcessed = pFilterFrequency->Process(controlRateBlockSize) || hasProcessed;
   hasProcessed = pFilterQ->Process(controlRateBlockSize) || hasProcessed;
   hasProcessed = pFilterGain->Process(controlRateBlockSize) || hasProcessed;

Pretty interessant! Nice to point it out mate! Not sure how can compiler know this if that var is evalutated at runtime. But I trust you...
How would you fix it: 3 different if? I'd love to avoid "conditional" as much as I can

And apart this, what do you think about the "drawing" code?

[noizebox]

I'd also like to point out -- since a lot of people suggest "lock-free queues" -- that what you really want is wait-free queue. This difference is very important (and not just semantics), because a "lock-free" algorithm is allowed to wait (eg. busy loop) while a "wait-free" algorithm is allowed to use locks (as long as it never waits for them, so operations like try_wait() are acceptable).

You are of course correct, I'm one of those guilty of using the term 'lock free' sloppily, when meaing 'lock free', but I dont think that I'm the only one

I use a scheme similar to this for moving (non-trivial) data from the GUI thread to the audio thread (eg. the pointer can point to an arbitrarily complex data structure), but when you swap a new pointer in place, you have to keep the data for the previous pointer alive until you know the reader (or all of them if you want multiple) are done and this requires some locking in order to figure out when it's safe to reuse the old object (or free it or whatever you want to do with it).

In the situation suggested by the OP I assumed that the audio thread would swap the pointers every time so that most of the time the reader would not read the struct, but when it dit, it would have the latest values available. So a queue that drops the oldest value when updating basically.

There is a very small possibility that the reader thread might collide with the writer and read while the writer thread updates the data, for that to happen the copying of the coefficients would have to take at least 2 control rate updates. Which I suppose could only happen if the reader (gui) thread is interrupted in the middle of copying the coefficients. I'm assuming that it's only the writer updating the pointer.

To make that 100% safe you would need a flag being set by the reader as well and the writer would have to wait (or better, skip) if the above situation would arrise. Or use a third object as you suggested.

[stratum]

Sorry for making an unrelated comment, but the compiler optimizer may cause the second two lines not to be executed at all if the first sets hasProcessed to true. Probably it's not the case here but it's better to be suspicious (these kind of problems usually happen with if expressions (i.e. inside 'if (...)' ) and not outside ).

Code: Select all

hasProcessed = pFilterFrequency->Process(controlRateBlockSize) || hasProcessed;
   hasProcessed = pFilterQ->Process(controlRateBlockSize) || hasProcessed;
   hasProcessed = pFilterGain->Process(controlRateBlockSize) || hasProcessed;

Pretty interessant! Nice to point it out mate! Not sure how can compiler know this if that var is evalutated at runtime. But I trust you...
How would you fix it: 3 different if? I'd love to avoid "conditional" as much as I can

And apart this, what do you think about the "drawing" code?

Nevermind it was a silly comment.
The optimization is applied if you write something like if (ptr && ptr->blah()) though.
Your code looks correct, including that try_lock and drawing, except I couldn't understand why you call RedrawCachedBitmap from the audio thread.

[mystran]

Sorry for making an unrelated comment, but the compiler optimizer may cause the second two lines not to be executed at all if the first sets hasProcessed to true. Probably it's not the case here but it's better to be suspicious (these kind of problems usually happen with if expressions (i.e. inside 'if (...)' ) and not outside ).

Code: Select all

hasProcessed = pFilterFrequency->Process(controlRateBlockSize) || hasProcessed;
   hasProcessed = pFilterQ->Process(controlRateBlockSize) || hasProcessed;
   hasProcessed = pFilterGain->Process(controlRateBlockSize) || hasProcessed;

If it happens (either like this or inside an if()-condition) when you do something that has side-effects (and isn't a method declared as const) and you are not calling an operator overload (which work like normal function calls rather than short-circuit booleans, essentially breaking the whole language), then you should file for a compiler bug.

The semantics of && and || is to always evaluate them sequentially from left to right, but only until the return value is determined (by the evaluation from the left to right, since we are not allowed to look to the right at all before we've evaluated the term on the left). As far as the compiler is concerned they are flow-control operations (ie. branches).

There are probably billions(!) of lines of code that relies on this behaviour all over, so I'm a bit sceptical about any compiler ever failing to do the right thing. The two most common use-cases combined into one would look something like this (which in some cases is a lot more legible than a lot of if-else branches):

Code: Select all

  (maybeNullPointer && maybeNullPointer->doStuff()) || logError("couldn't doStuff");

[mystran]

There is a very small possibility that the reader thread might collide with the writer and read while the writer thread updates the data, for that to happen the copying of the coefficients would have to take at least 2 control rate updates. Which I suppose could only happen if the reader (gui) thread is interrupted in the middle of copying the coefficients. I'm assuming that it's only the writer updating the pointer.

The rule of thumb for multi-threaded programming is that unless you can (at least semi-formally) prove that something never happens in the wrong order, it's going to cause you problems at some point in the future. Having a proof doesn't guarantee that the code is actually safe or that it works in practice, but it increases the odds considerably.

[mystran]

Audio thread: it update coeffs only if controls has been changed. Once they do, I place a flag to (later) draw them, calling a try_lock sync with the gui. Once there are coeffs, there is no lock and I can take control of that critical section (since try-lock won't do it sometimes), it updates the draw-coeffs (a copy of the real one) to be draw later, and it sends a message to the gui thread.

Draw thread: if audio thread is updating coeffs, block. Else, take the lock (atomic operation) and draw them.

It seems satisfy any requirements. What do you think about?

This is basically what I used to use (and some of my plugins still do) and it's safe and the only real downside is the slight overhead from the mutex (ie. might cause a trip to the kernel, not that it's really a big deal).

[Max M.]

Draw thread: if audio thread is updating coeffs, block. Else, take the lock (atomic operation) and draw them.

I would use:
..., take the lock, copy coeffcs, unlock, calc and draw.
It's not very good idea to keep the lock w/o a reason (in an edge case this may lead to a situation where the audio thread skips the lock too often).

[noizebox]

The rule of thumb for multi-threaded programming is that unless you can (at least semi-formally) prove that something never happens in the wrong order, it's going to cause you problems at some point in the future. Having a proof doesn't guarantee that the code is actually safe or that it works in practice, but it increases the odds considerably.

As I wrote, an additional flag set by the reader thread will handle that. The reader thread will in that situation have the choice between skipping the copy or, what is likely preferable in this case, overwrite the last update without swapping the pointer, since the reader is still processing the second last update.

[Nowhk]

Draw thread: if audio thread is updating coeffs, block. Else, take the lock (atomic operation) and draw them.

I would use:
..., take the lock, copy coeffcs, unlock, calc and draw.
It's not very good idea to keep the lock w/o a reason (in an edge case this may lead to a situation where the audio thread skips the lock too often).

The reason is "draw plot using coefficients; please audio, don't refresh copy coeffs right now". Its a valid reason Even if the audio skips the lock, it skips simply the ones for the draw, not the real audio coefficients (which are always calculated sample by sample).

What you suggest imply blocking the audio thread, which is even worse I believe. No?

[Max M.]

What you suggest imply blocking the audio thread, which is even worse I believe. No?

No, it's the same non-blocking lock as in your variant. The difference is that the gui thread locks only for "get my copy of the coefficients in an atomic way" (this should be the only proper reason to lock). If you lock for too long (e.g. during the whole drawing process) you may get into a situation (yet again in theoretic edge case, e.g. where some other cpu-hungry thread keeps interrupting your gui-thread while it's in lock) where the audio thread may never (or too rare) be able to update the coefficients (their shared copy) at all.

[Nowhk]

No, it's the same non-blocking lock as in your variant. The difference is that the gui thread locks only for "get my copy of the coefficients in an atomic way" (this should be the only proper reason to lock).

Not sure what you are suggesting so
It shouldn't be the draw thread that must copy the coeffs, but the audio one!
If the draw thread "lock for copy", this means that on the other side (audio thread), once it try to update the real coeffs (but draw is locking them), the thread will blocks! Which is what I don't want. Audio thread must never block! Am I wrong?

If you lock for too long (e.g. during the whole drawing process) you may get into a situation (yet again in theoretic edge case, e.g. where some other cpu-hungry thread keeps interrupting your gui-thread while it's in lock) where the audio thread may never (or too rare) be able to update the coefficients (their shared copy) at all.

This should be an edge case in which the gui thread ends and (re)start immediatly after; and in between no audio thread run (which usually got higher priority).

i.e. the gui hread (or better, the critical section of the gui thread, which are 200 for-iterations in my example) run entirely for 1/60 of second @ 60FPS, and after it finished the audio thread is delayed instead.

Well, I think that for this case, its an edge case that can be ignored.

[mystran]

No, it's the same non-blocking lock as in your variant. The difference is that the gui thread locks only for "get my copy of the coefficients in an atomic way" (this should be the only proper reason to lock).

Not sure what you are suggesting so
It shouldn't be the draw thread that must copy the coeffs, but the audio one!

What you are supposed to do is copy the coefficients from the audio data to a shared buffer in the audio thread if you can get the lock without waiting. You are then supposed to copy those coefficients from the shared buffer into a GUI-side buffer in the GUI thread so that you don't need to hold the lock while you draw.

Yes, that's right. Both threads should copy and the shared buffer should be treated as a "communication channel" only. Since your communication scheme is probabilistic (ie. on average you can get a certain percentage of messages through in time) you want to maximise that probability by minimising the time you hold the lock with either thread.

i.e. the gui hread (or better, the critical section of the gui thread, which are 200 for-iterations in my example) run entirely for 1/60 of second @ 60FPS, and after it finished the audio thread is delayed instead.

The point of critical sections is to keep them as small as possible. In this case the minimum you need to do is read the data once (to copy it somewhere else). It's especially important in this kind of scheme where the other thread won't wait, since every time you hold the lock when the audio thread tries to write new data, you miss that data. Even if you just copy the data quickly, it is going to happen once in a while, but the longer you hold the lock the more often it will happen (eg. if you hold the lock 50% of the time, then you expect to miss about 50% of the updates).

With a scheme like this, just copy the data on both sides. Even if you are moving around a couple of dozen megabytes per second, one extra copy is not going to make a whole lot of difference (beyond that it might make some sense to worry about it, but not at the expense of holding a critical section longer than necessary).