[Asterisk-code-review] lock: Improve performance of DEBUG THREADS. (asterisk[master])

[Corey Farrell]

Corey Farrell has posted comments on this change. ( https://gerrit.asterisk.org/10293 )

Change subject: lock: Improve performance of DEBUG_THREADS.
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.asterisk.org/#/c/10293/1/include/asterisk/lock.h
File include/asterisk/lock.h:

https://gerrit.asterisk.org/#/c/10293/1/include/asterisk/lock.h@130
PS1, Line 130: /*! \brief Structure for mutex and tracking information.
             :  *
             :  * We have tracking information in this structure regardless of DEBUG_THREADS being enabled.
             :  * The information will just be ignored in the core if a module does not request it..
             :  *
> I'm not in favor of this patch as it just about precludes the most needed improvement to DEBUG_THREA […]
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.

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.

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.



-- 
To view, visit https://gerrit.asterisk.org/10293
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iabd650908901843e9fff47ef1c539f0e1b8cb13b
Gerrit-Change-Number: 10293
Gerrit-PatchSet: 2
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Fri, 28 Sep 2018 12:07:10 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180928/939c9fec/attachment-0001.html>