[asterisk-dev] [Code Review] 3405: Add ast_spinlock capability

Tue Apr 22 13:35:34 CDT 2014

> On April 22, 2014, 10:58 a.m., rmudgett wrote:
> > branches/12/include/asterisk/spinlock.h, lines 403-405
> > <https://reviewboard.asterisk.org/r/3405/diff/5/?file=57747#file57747line403>
> >
> >     This statement isn't quite true since these prototypes are declared after the functions themselves are defined above.  GCC doesn't require static functions to have prototypes if the references to them are after the function definition since the function defintition itself can serve as the prototype.  The only real effect these prototypes have is to ensure that the various implementations above provide the same API.

It's the typedef of ast_spinlock_t that get's caught.  You can't use it in a prototype until it's been defined.  The other reason for the prototypes is so that I don't have to repeat the <gasp>documentation</gasp> for each implementation.

- George

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3405/#review11714
-----------------------------------------------------------

On April 19, 2014, 10:57 a.m., George Joseph wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3405/
> -----------------------------------------------------------
> 
> (Updated April 19, 2014, 10:57 a.m.)
> 
> 
> Review request for Asterisk Developers, Corey Farrell, Joshua Colp, and rmudgett.
> 
> 
> Bugs: ASTERISK-23553
>     https://issues.asterisk.org/jira/browse/ASTERISK-23553
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Now ready for prime time...
> 
> This patch adds support for spinlocks in Asterisk.
> 
> There are cases in Asterisk where it might be desirable to lock a short critical code section but not incur the context switch and yield penalty of a mutex or rwlock.  The primary spinlock implementations execute exclusively in userspace and therefore don't incur those penalties.  Spinlocks are NOT meant to be a general replacement for mutexes.  They should be used only for protecting short blocks of critical code such as simple compares and assignments.  Operations that may block, hold a lock, or cause the thread to give up it's timeslice should NEVER be attempted in a spinlock.
> 
> The first use case for spinlocks is in astobj2 - internal_ao2_ref.  Currently the manipulation of the reference counter is done with an ast_atomic_fetchadd_int which works fine.  When weak reference containers are introduced however, there's an additional comparison and assignment that'll need to be done while the lock is held.  A mutex would be way too expensive here, hence the spinlock.  Given that lock contention in this situation would be infrequent, the overhead of the spinlock is only a few more machine instructions than the current ast_atomic_fetchadd_int call.
> 
> 7 implementations were tested on various i366, x86_64, ARM and Sparc platforms running Linux, FreeBSD, OSX and Solaris.  The test suite and results can be found here... https://github.com/gtjoseph/spintest
> 
> Summary...
> 
> Tested various spinlock implementations. 
> 
> + GCC Atomics      		gcc_atomics
> + x86 assembly     		gas_x86
> + ARM assembly     		gas_arm
> + Sparc assembly     		gas_sparc
> + OSX Atomics      		osx_atomics
> + pthreads spinlock		pthread_spinlock
> + pthreads mutex   		pthread_mutex
> + pthreads adaptive mutex       pthread_mutex_adaptive_np
> 
> Not all implementations were available on all platforms.  For instance, the gas_x86 implementation won't be available on an arm or sparc platform.
> 
> Each implementation was tested with the same locking scenario which is a short block of compares and assignments representing the most anticipated Asterisk use cases.  The test case was run in a tight loop of 25,000,000 iterations executed in parallel by 1..5 simultaneous threads.
> 
> The test suite was run on the following platforms:
> 
> + Darwin-OSX-x86_64-2core
> + FreeBSD-BSD-amd64-4core
> + FreeBSD-BSD-i386-1core
> + Linux-CentOS-i686-1core
> + Linux-Debian-i686-4core
> + Linux-Debian-x86_64-4core
> + Linux-Fedora-armv7l-1core
> + Linux-Fedora-i686-1core
> + Linux-Fedora-x86_64-12core
> + Linux-Fedora-x86_64-1core
> + Linux-Fedora-x86_64-2core
> + Linux-Fedora-x86_64-4core
> + Linux-Fedora-x86_64-8core
> + Linux-Gentoo-sparc64-32core
> + SunOS-Solaris-i86pc-4core
> 
> For each platform tested, the real, user and system times were captured in csv form and the final real and system times graphed.
> 
> Observations:
> 
> + The GCC Atomics implementation was available on all platforms and generally had the best performance.
> + The native assembly implementations generally performed on par with the GCC Atomics implementation.
> + The pthread_spinlock implementation was not available on Darwin but performed well on the other platforms.
> + The OSX Atomics implementation performed well on Darwin.
> + The pthread_mutex_adaptive implementation was not available on all platforms and it's performance was noticeably worse on the supported platforms.
> + The pthread_mutex implementation was available on all platforms but clearly showed the effects of context switching (high sys time) as lock contention grew.
> 
> Conclusions:
> 
> + GCC Atomics should be the first implementation choice.
> + The native assembly implementations should be the second choices.
> + The pthread_spinlock implementation should be the third choice on non-Darwin platforms.
> + OSX Atomics should be the third choice on Darwin.
> + The pthread_mutex_adaptive implementation should be eliminated.
> + The pthread_mutex implementation should be the implementation of last resort.
> + If none of the implementations are available, a compilation #error will be raised.
> 
> 
> Diffs
> -----
> 
>   branches/12/include/asterisk/spinlock.h PRE-CREATION 
>   branches/12/include/asterisk/autoconfig.h.in 412427 
>   branches/12/configure.ac 412427 
>   branches/12/configure UNKNOWN 
> 
> Diff: https://reviewboard.asterisk.org/r/3405/diff/
> 
> 
> Testing
> -------
> 
> See above.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140422/b479d380/attachment.html>