Re: NSSound Code Review

[David Chisnall]

On 29 Jul 2009, at 03:43, David Ayers wrote:

> Hello Stefan,
>
>
> Am Montag, den 27.07.2009, 20:44 -0500 schrieb Stef Bidi:
>
> > I really just want to get this out there so that I can get more eyes
> > on it... this is my first time working this in-depth with GNUstep, so
> > I'm sure mistakes were made (I'm just hoping they aren't massive).
>
> Maybe I'm just reading the code incorrectly, but will playing a sound
> make the application multi-threaded wrt the OpenSTEP/Cocoa API (and
> thereby create all the lazy locks registered for the
> NSWillBecomeMultiThreadedNotification)?

Note that this is deprecated on Cocoa - Cocoa applications are always  
multithreaded because Cocoa uses a few threads internally.  This also  
eliminates problems where people are spawning threads using the POSIX  
or Mach APIs (in their own code or in libraries) which don't trigger  
the notification.  On Cocoa, since 10.2 if I remember correctly, but  
possibly earlier, isMultithreaded always returns YES.

Apple realised - quite correctly - that  
NSWillBecomeMultiThreadedNotification is a horrible hack and very  
unreliable in nontrivial code.

> Triggering locks will give is quite a performance hit on applications
> which would otherwise be single threaded.

Typical mutex implementations just use the CPU's compare and swap  
instruction to set a flag, so the operation is very cheap when there  
is no contention.  A lock which is always held by a single thread has  
very little overhead.  It's nonzero, but compared to the system call  
and I/O overhead of reading some data from a sound file it is trivial  
and not worth thinking about.  A typical pthread mutex implementation  
doesn't even need a system call unless it spins for a little while  
without acquiring the lock (at which point it will put the thread to  
sleep).

Note that GNUstep's current NSLock implementation is far slower than  
it needs to be, because it uses the GNU objc wrappers around pthread,  
rather than using pthreads directly.  I sent a patch to Gregory a  
couple of months ago that fixes this; hopefully it will go in soon as  
pthreads are now supported on every platform where GNUstep runs  
(including Windows).  This patch also included an implementation of  
NSCondition, which can simplify the implementation of NSSound a little.

That said, a number of the locks in the code under discussion are in  
the wrong place, and NSConditionLock appears to be consistently used  
incorrectly.

> I think this really needs to be avoided.  Note I'm not say that the
> process can't become multi-threaded at all.  It's just if it really  
> must
> it should do so at a lower level.  I'm not sure if the ObjC-runtime
> wrappers can be used with triggering the notification.  But GCC's
> gthread is probably a safe abstraction, that is if clang & co also
> support that API.

I can't find any reference to what you mean by GCC's gthread - there  
is a GNOME library by that name, apparently, but that doesn't seem to  
be what you're talking about.

David