<p><a href="https://gerrit.asterisk.org/10293">View Change</a></p><p>1 comment:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/#/c/10293/1/include/asterisk/lock.h">File include/asterisk/lock.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/10293/1/include/asterisk/lock.h@130">Patch Set #1, Line 130:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">/*! \brief Structure for mutex and tracking information.<br> *<br> * We have tracking information in this structure regardless of DEBUG_THREADS being enabled.<br> * The information will just be ignored in the core if a module does not request it..<br> *<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I'm not in favor of this patch as it just about precludes the most needed improvement to DEBUG_THREA […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I believe my updated patch should address your concerns while still accomplishing my goal of reducing memory overhead of ao2_alloc.  One thing for the 'double-check' inside the global lock I checked the pointer rather than rechecking the setup flag (I assume this is better as we don't need to read the volatile flag while we have the lock).  Please let me know if you think marking 'setup' as volatile is best, or if it would be better for ast_get_reentrancy to use volatile on the flags argument.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I agree with you about 'struct ast_lock_tracking_flags' being ABI compatible.  I verified that (on x86_64 anyways) this does not change sizeof(struct ast_mutex_info).  The contents of the tracking field is private to main/lock.c, the field is only in the public header to allow static storage of locks.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Once you've given this a review I'll cherry-pick to all branches.  On current 'master' built without AO2_DEBUG or DEBUG_THREADS the memory overhead is 88 bytes per ao2_alloc (with mutex locking).  The same build settings with this patch plus ao2-reduce-storage the memory overhead is 64 bytes.  This 24 bytes per object may not seem like much but I just did a basic test using AO2_DEBUG, 'astobj2 show stats' reported that 9410 objects were allocated.  If half those objects were allocated with no locking that's still 100k saved in the minimum startup.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/10293">change 10293</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/10293"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Iabd650908901843e9fff47ef1c539f0e1b8cb13b </div>
<div style="display:none"> Gerrit-Change-Number: 10293 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 (1000185) </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 28 Sep 2018 12:07:10 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>