----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3405/#review11697 -----------------------------------------------------------
branches/12/include/asterisk/spinlock.h <https://reviewboard.asterisk.org/r/3405/#comment21464> Spinlocks are useless on a non-multicore machines. The assembly versions should have some check to see if the machine the code is being built for is multicore. branches/12/include/asterisk/spinlock.h <https://reviewboard.asterisk.org/r/3405/#comment21462> Always use curly braces. branches/12/include/asterisk/spinlock.h <https://reviewboard.asterisk.org/r/3405/#comment21463> Curlies branches/12/include/asterisk/spinlock.h <https://reviewboard.asterisk.org/r/3405/#comment21465> I suppose a comment saying the below prototypes are to ensure that the spinlock implementations above provice the same API. - rmudgett On April 17, 2014, 12:26 p.m., George Joseph wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3405/ > ----------------------------------------------------------- > > (Updated April 17, 2014, 12:26 p.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 > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
