<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/3405/">https://reviewboard.asterisk.org/r/3405/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 22nd, 2014, 11:58 a.m. CDT, <b>rmudgett</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/3405/diff/5/?file=57747#file57747line403" style="color: black; font-weight: bold; text-decoration: underline;">branches/12/include/asterisk/spinlock.h</a>
<span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">403</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm">/* Prototypes need to be defined last because ast_spinlock_t will</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">404</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * not have been defined until now.</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">405</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> */</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On April 22nd, 2014, 1:35 p.m. CDT, <b>George Joseph</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes, you have to declare the prototypes after the typedef of ast_spinlock_t but that isn't important to what they are actually doing. These prototypes just give you a convienient place to put the API documentation and to ensure that the various implementations above them provide a consistent API. As far as the compiler is concerned, these prototypes are redundant because it has already seen the function definitions.</pre>
<br />
<p>- rmudgett</p>
<br />
<p>On April 22nd, 2014, 1:35 p.m. CDT, George Joseph wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers, Corey Farrell, Joshua Colp, and rmudgett.</div>
<div>By George Joseph.</div>
<p style="color: grey;"><i>Updated April 22, 2014, 1:35 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-23553">ASTERISK-23553</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">See above.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>branches/12/include/asterisk/spinlock.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>branches/12/include/asterisk/autoconfig.h.in <span style="color: grey">(412900)</span></li>
<li>branches/12/configure.ac <span style="color: grey">(412900)</span></li>
<li>branches/12/configure <span style="color: grey">(UNKNOWN)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3405/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>