For those still using Carbon


#1

(executive summary: Jules, at the end I suggest a bug fix that may apply to the Cocoa tree)

Maybe I am not the only one who is shipping a plugin based on the good old carbon Juce, so I am starting this thread to share some fixes on the Carbon code.

Here are the issues that I have run into:

  • the blank window bug : this is fixed by http://www.rawmaterialsoftware.com/juceforum/viewtopic.php?p=17011#17011
  • window repaint problems in protools: commenting the SetControlSupervisor (pluginView, parentView); startTimer (150); seems to work.
  • mouse offset bug in protools : still occuring , but moving the mouse cursor over the lower part of the plugin window make it disappear…

And finally the bug that is really annoying me:

  • Garageband, Logic, Protools all showed some strange event-loop related bugs that render the host GUI very unresponsive with a lag that can reach 10 or 20 seconds between a mouse click and the UI reaction to the click. The shark profiler shows that it spends much cpu time processing messages within very deep hierarchy of windows. The bug happens even when the plugin GUI has not been opened at all.

The most easy host to spot that bug being Garageband 5, because the bug is almost always reproducible on this one. Just take an empty audiounit juce plugin, remove everything from juce_AudioUnitWrapper.cpp (AUView included), except a timer with small period (lets says 20ms), open it in Garageband and everything in Garageband become really slow. Each click in the interface makes it slower. The same stuff happens in Logic, but more randomly (for example after two bounces of your project), and Pro Tools (but not everywhere, not everytime, not on my setup…)

I have never done Carbon dev, so I’m not exactly knowledgable on carbon event loops. Here is what I have observed: when the UI lag happens, the host event queue seems to be stuck with some events that won’t go. Each mouse click in the interface adds new events in the queue, and those events are not processed by the host. Juce is not exactly flooding the event loop of the host, but the juce Timers are causing a constant flux of event being sent to the host. Those events are sent with a “Standard” priority:

Using kEventPriorityHigh instead of Standard has the immediate effect of just locking Garageband, it looks like it is not able to process any event when the Juce evt is the first one in the queue ?

Using kEventPriorityLow makes everything work more smoothly. This is almost enough to remove the strange UI lag, but on some occasions I see the event queue grow very quickly with stuck events.

When this kind of situation happens, the juce InternalTimerThread is not helping because it keeps filling the event queue even when the previous timer event sent has not yet been received: it is using a boolean flag “callbackNeeded” that tells it that “an” event has just been received, but it is not necessariy the last event sent. It can be an event that was sent 10 seconds ago… so this flag is not efficient and should be replaced by two counters send_cnt and recv_cnt. Here is what I am now using in the run() method of the InternalTimerThread:

            if (timeUntilFirstTimer <= 0)
            {
                ++send_cnt;
                postMessage (new Message(send_cnt, 0, 0, 0));

                const uint32 messageDeliveryTimeout = now + 2000;

                while (recv_cnt != send_cnt)
                {
                    wait (4);

That is I have replaced the “callbackNeeded” bool flag with two integers. In the handleMessage, I replaced the callbackNeeded = false; with recv_cnt = msg.intParameter1;

With that fix, the timer thread is no more flooding the host event queue when it does not want to read events fast enough.

Quickly looking at the current (Cocoa) juce_Timer.cpp file, I believe a similar correction should be made to it. Jules, what do you think ?

And finally, a last question that may be plain stupid: what if I was just calling timerCallback from the InternalTimerThread::run() method , using a MessageManagerLock to make it thread-safe ? Would there still be some thread problems ? to me it looks like it would make the timer callbacks much more precise and less dependant on the host message loop.


#2

Interesting stuff.

Looking at the latest code (which might actually be different from the way it was in 1.46), I just can’t see how it could flood the event queue. The callbackNeeded flag (which might be better named “callbackHasHappened” is just used to block the background thread until the message that it has just sent has arrived. There’s a timeout in case the UI thread is stuck, but even so, I can’t see an execution path that would produce more than 1 message every 2 seconds without them being delivered… (?)

It’s very far from being stupid, and in fact would certainly work just fine. But… I don’t think it’d be any faster, because the way the locks work is to send a special message that blocks the message thread to keep it busy until your code has executed, so it’d basically be just as quick as it is now.

But the main reason for not doing that is that some obscure OS-dependent functions have to run on the UI thread, not for thread-safety reasons, but for stupid internal reasons, so changing the timer to make its calls on its own thread might cause some software to break…


#3

mmmm I thought it was obvious but indeed it is not obvious at all, so I had to put back some tracing code in order to fully understand how the timerthread may flood the event queue. The problem is that callbackNeeded = false occurs at two places in InternalTimerThread::handleMessage : one time in the loop over timers, it is set before timerCallback() is invoked. And then one time at the end of the InternalTimerThread::handleMessage()

So if one of the timerCallback() is a bit long for whatever reason, any timer message send between the first “callbackNeeded = false” and the “callbackNeeded = false” at the exit of handleMessage will be wrongly considered as received.

(I’m not sure if my explanation is clear)


#4

ah I see what you mean now! Yes, if the timercallback blocks, then it could indeed happen. Ok, I think the best thing to do is to remove that first “callbackNeeded = false;”, but I’m going to have a good hard look at it, just in case there was a good reason why I put it there!


#5

Hi Julian,

I have couple of issues to clarify.

I had faced a problem with juce 1.45, i was using “void sendChangeMessage (void *objectThatHasChanged)” but some of the messages were lost, was it because of timer issue. And will/could
"void sendSynchronousChangeMessage (void *objectThatHasChanged)" be effected by this issue.

I needed a clarification on this because i have a applications which sends a large amount of changemessages.

Thanks and regards,
vishvesh


#6

ChangeListeners don’t use timers so this can’t be related…


#7

thanks :slight_smile:


#8

It sounds like a good idea for guys still using carbon version of juce to keep the fixes in a single thread. Here are couple of issues I had to face

  1. bool browseForFileToOpen (FilePreviewComponent *previewComponent=0) allows selection of multiple files. The fix is available in this link

    http://www.rawmaterialsoftware.com/juceforum/viewtopic.php?p=17927&highlight=#17927

  2. Time issue - fixed in the current svn version of juce - solution for it in carbon version of juce

    http://www.rawmaterialsoftware.com/juceforum/viewtopic.php?p=17861&highlight=#17861


#9

Hey Jules,

Just wanting to know what the update on this is.

Also I just checked the timer code an noticed the “volatile” keyword is used on the callbackNeeded member. Are you aware that this will do nothing but slow down your code?

Do you actually need a memory barrier at this point? If you are after a memory barrier, ie something to make sure that all previous lines of code have actually finished execution by the time you set a “flag” value to say they are done then the easiest way to do this is an atomic swap operation like the ones below, where only the pointer argument needs to be declared volatile:

	/**
	Atomic swap function with barrier
	*/
	bool AtomicCompareExchange32
	(
	   long volatile * destination,
	   long valuenew,
	   long valueold
	)
	{
	#if defined (NOLOCK_WIN)
		return (valueold == _InterlockedCompareExchange (destination, valuenew, valueold));
	#elif defined (NOLOCK_MAC)
		return OSAtomicCompareAndSwap32Barrier (valueold, valuenew, (volatile int32_t*) destination);
	#endif
	}

Here is an article about it: http://software.intel.com/en-us/blogs/2007/11/30/volatile-almost-useless-for-multi-threaded-programming/

In addition you can actually use the volatile keyword effectively in c++ but you should use it as a modifier of functions never on primitive types: http://www.ddj.com/cpp/184403766


#10

… so we can’t even use ‘volatile’ any more? What’s the world coming to when you can’t even turn a flag on and off without an in-depth knowledge of memory fences and cpu pipelining…

Maybe this calls for a simple class that’s just a thread-safe boolean flag?


#11

[quote=“jules”]… so we can’t even use ‘volatile’ any more? What’s the world coming to when you can’t even turn a flag on and off without an in-depth knowledge of memory fences and cpu pipelining…

Maybe this calls for a simple class that’s just a thread-safe boolean flag?[/quote]

Yes. They already exist. Want me to write one for Juce? A little template class that takes a <= 4 byte thing and allows get and set on it?


#12

Thanks Andy, that’d be great.