[Asterisk-code-review] astobj2: Record lock usage to refs log when DEBUG THREADS is... (asterisk[master])

Corey Farrell asteriskteam at digium.com
Mon Oct 1 12:38:55 CDT 2018


Corey Farrell has uploaded this change for review. ( https://gerrit.asterisk.org/10347


Change subject: astobj2: Record lock usage to refs log when DEBUG_THREADS is enabled.
......................................................................

astobj2: Record lock usage to refs log when DEBUG_THREADS is enabled.

When DEBUG_THREADS is enabled we can know if the astobj2 mutex / rwlock
was ever used, so it can be recorded in the REF_DEBUG destructor entry.

Create contrib/scripts/reflocks.py to process locking used by
allocator.  This can be used to identify places where
AO2_ALLOC_OPT_LOCK_NOLOCK should be used to reduce memory usage.

Change-Id: I2e3cd23336a97df2692b545f548fd79b14b53bf4
---
M contrib/scripts/refcounter.py
A contrib/scripts/reflocks.py
M main/astobj2.c
3 files changed, 178 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/47/10347/1

diff --git a/contrib/scripts/refcounter.py b/contrib/scripts/refcounter.py
index de3cda0..36fb341 100755
--- a/contrib/scripts/refcounter.py
+++ b/contrib/scripts/refcounter.py
@@ -108,6 +108,10 @@
             else:
                 current_objects[obj]['curcount'] += int(parsed_line['delta'])
 
+            if 'destructor' in parsed_line['state']:
+                # refcounter.py doesn't care about lock-state.
+                parsed_line['state'] = '**destructor**'
+
             current_objects[obj]['log'].append(
                 "[%s] %s:%s %s: %s %s - [%s]" % (
                     parsed_line['thread_id'],
diff --git a/contrib/scripts/reflocks.py b/contrib/scripts/reflocks.py
new file mode 100755
index 0000000..866bd1f
--- /dev/null
+++ b/contrib/scripts/reflocks.py
@@ -0,0 +1,147 @@
+#!/usr/bin/env python
+"""Process a ref debug log for lock usage
+
+ This file will process a log file created by Asterisk
+ that was compiled with REF_DEBUG and DEBUG_THREADS.
+
+ See http://www.asterisk.org for more information about
+ the Asterisk project. Please do not directly contact
+ any of the maintainers of this project for assistance;
+ the project provides a web site, mailing lists and IRC
+ channels for your use.
+
+ This program is free software, distributed under the terms of
+ the GNU General Public License Version 2. See the LICENSE file
+ at the top of the source tree.
+
+ Copyright (C) 2018, CFWare, LLC
+ Corey Farrell <git at cfware.com>
+"""
+
+from __future__ import print_function
+import sys
+import os
+
+from optparse import OptionParser
+
+
+def parse_line(line):
+    """Parse out a line into its constituent parts.
+
+    Keyword Arguments:
+    line The line from a ref debug log to parse out
+
+    Returns:
+    A dictionary containing the options, or None
+    """
+    tokens = line.strip().split(',', 7)
+    if len(tokens) < 8:
+        print("ERROR: ref debug line '%s' contains fewer tokens than "
+              "expected: %d" % (line.strip(), len(tokens)))
+        return None
+
+    processed_line = {'addr': tokens[0],
+                      'file': tokens[3],
+                      'line': tokens[4],
+                      'function': tokens[5],
+                      'state': tokens[6],
+                      }
+    return processed_line
+
+
+def process_file(options):
+    """The routine that kicks off processing a ref file"""
+
+    object_types = {}
+    objects = {}
+    filename = options.filepath
+
+    with open(filename, 'r') as ref_file:
+        for line in ref_file:
+            if 'constructor' not in line and 'destructor' not in line:
+                continue
+            tokens = line.strip().split(',', 7)
+            addr = tokens[0]
+            state = tokens[6]
+            if 'constructor' in state:
+                obj_type = '%s:%s:%s' % (tokens[3], tokens[4], tokens[5])
+                if obj_type not in object_types:
+                    object_types[obj_type] = {
+                        'used': 0,
+                        'unused': 0,
+                        'lockobj': 0,
+                        'none': 0
+                    }
+                objects[addr] = obj_type
+            elif 'destructor' in state:
+                if addr not in objects:
+                    # This error would be reported by refcounter.py.
+                    continue
+                obj_type = objects[addr]
+                del objects[addr]
+                if '**lock-state:unused**' in state:
+                    object_types[obj_type]['unused'] += 1
+                elif '**lock-state:used**' in state:
+                    object_types[obj_type]['used'] += 1
+                elif '**lock-state:lockobj**' in state:
+                    object_types[obj_type]['lockobj'] += 1
+                elif '**lock-state:none**' in state:
+                    object_types[obj_type]['none'] += 1
+
+    for (allocator, info) in object_types.items():
+        stats = [];
+        if info['used'] > 0:
+            if not options.used:
+                continue
+            stats.append("%d used" % info['used'])
+        if info['unused'] > 0:
+            stats.append("%d unused" % info['unused'])
+        if info['lockobj'] > 0 and options.lockobj:
+            stats.append("%d lockobj" % info['lockobj'])
+        if info['none'] > 0 and options.none:
+            stats.append("%d none" % info['none'])
+        if len(stats) == 0:
+            continue
+        print("%s: %s" % (allocator, ', '.join(stats)))
+
+
+def main(argv=None):
+    """Main entry point for the script"""
+
+    ret_code = 0
+
+    if argv is None:
+        argv = sys.argv
+
+    parser = OptionParser()
+
+    parser.add_option("-f", "--file", action="store", type="string",
+                      dest="filepath", default="/var/log/asterisk/refs",
+                      help="The full path to the refs file to process")
+    parser.add_option("-u", "--suppress-used", action="store_false",
+                      dest="used", default=True,
+                      help="Don't output types that have used locks.")
+    parser.add_option("-n", "--show-none", action="store_true",
+                      dest="none", default=False,
+                      help="Show counts of objects with no locking.")
+    parser.add_option("-o", "--show-lockobj", action="store_true",
+                      dest="lockobj", default=False,
+                      help="Show counts of objects with a lockobj.")
+
+    (options, args) = parser.parse_args(argv)
+
+    if not os.path.isfile(options.filepath):
+        print("File not found: %s" % options.filepath, file=sys.stderr)
+        return -1
+
+    try:
+        process_file(options)
+    except (KeyboardInterrupt, SystemExit, IOError):
+        print("File processing cancelled", file=sys.stderr)
+        return -1
+
+    return ret_code
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv))
diff --git a/main/astobj2.c b/main/astobj2.c
index 23109a6..d6a8a4e 100644
--- a/main/astobj2.c
+++ b/main/astobj2.c
@@ -473,6 +473,9 @@
 	int32_t current_value;
 	int32_t ret;
 	struct ao2_weakproxy *weakproxy = NULL;
+#ifdef DEBUG_THREADS
+	const char *lock_state;
+#endif
 
 	if (obj == NULL) {
 		if (ref_log && user_data) {
@@ -592,21 +595,33 @@
 	switch (obj->priv_data.options & AO2_ALLOC_OPT_LOCK_MASK) {
 	case AO2_ALLOC_OPT_LOCK_MUTEX:
 		obj_mutex = INTERNAL_OBJ_MUTEX(user_data);
+#ifdef DEBUG_THREADS
+		lock_state = obj_mutex->mutex.lock.flags.setup ? "used" : "unused";
+#endif
 		ast_mutex_destroy(&obj_mutex->mutex.lock);
 
 		ast_free(obj_mutex);
 		break;
 	case AO2_ALLOC_OPT_LOCK_RWLOCK:
 		obj_rwlock = INTERNAL_OBJ_RWLOCK(user_data);
+#ifdef DEBUG_THREADS
+		lock_state = obj_rwlock->rwlock.lock.flags.setup ? "used" : "unused";
+#endif
 		ast_rwlock_destroy(&obj_rwlock->rwlock.lock);
 
 		ast_free(obj_rwlock);
 		break;
 	case AO2_ALLOC_OPT_LOCK_NOLOCK:
+#ifdef DEBUG_THREADS
+		lock_state = "none";
+#endif
 		ast_free(obj);
 		break;
 	case AO2_ALLOC_OPT_LOCK_OBJ:
 		obj_lockobj = INTERNAL_OBJ_LOCKOBJ(user_data);
+#ifdef DEBUG_THREADS
+		lock_state = "lockobj";
+#endif
 		ao2_t_ref(obj_lockobj->lockobj.lock, -1, "release lockobj");
 
 		ast_free(obj_lockobj);
@@ -614,12 +629,22 @@
 	default:
 		ast_log(__LOG_ERROR, file, line, func,
 			"Invalid lock option on ao2 object %p\n", user_data);
+#ifdef DEBUG_THREADS
+		lock_state = "invalid";
+#endif
 		break;
 	}
 
 	if (ref_log && tag) {
-		fprintf(ref_log, "%p,%d,%d,%s,%d,%s,**destructor**,%s\n",
-			user_data, delta, ast_get_tid(), file, line, func, tag);
+		fprintf(ref_log, "%p,%d,%d,%s,%d,%s,**destructor%s%s**,%s\n",
+			user_data, delta, ast_get_tid(), file, line, func,
+#ifdef DEBUG_THREADS
+			"**lock-state:",
+			lock_state,
+#else
+			"", "",
+#endif
+			tag);
 		fflush(ref_log);
 	}
 

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2e3cd23336a97df2692b545f548fd79b14b53bf4
Gerrit-Change-Number: 10347
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181001/196be9b5/attachment-0001.html>


More information about the asterisk-code-review mailing list