[asterisk-commits] rmudgett: branch rmudgett/ao2_red_black r371629 - /team/rmudgett/ao2_red_blac...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed Aug 22 18:53:13 CDT 2012


Author: rmudgett
Date: Wed Aug 22 18:53:10 2012
New Revision: 371629

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=371629
Log:
* Made AST_DEVMODE keep track of the number of nodes in the generic
container.  Also keeps track of the maximum number of empty nodes.

* Made the generic container node indicate if it is linked into the
container.

Modified:
    team/rmudgett/ao2_red_black/main/astobj2.c

Modified: team/rmudgett/ao2_red_black/main/astobj2.c
URL: http://svnview.digium.com/svn/asterisk/team/rmudgett/ao2_red_black/main/astobj2.c?view=diff&rev=371629&r1=371628&r2=371629
==============================================================================
--- team/rmudgett/ao2_red_black/main/astobj2.c (original)
+++ team/rmudgett/ao2_red_black/main/astobj2.c Wed Aug 22 18:53:10 2012
@@ -757,6 +757,8 @@
 	void *obj;
 	/*! Container holding the node.  (Does not hold a reference.) */
 	struct ao2_container *my_container;
+	/*! TRUE if the node is linked into the container. */
+	unsigned int is_linked:1;
 };
 
 /*!
@@ -948,6 +950,12 @@
 	uint32_t options;
 	/*! Number of elements in the container. */
 	int elements;
+#if defined(AST_DEVMODE)
+	/*! Number of nodes in the container. */
+	int nodes;
+	/*! Maximum number of empty nodes in the container. (nodes - elements) */
+	int max_empty_nodes;
+#endif	/* defined(AST_DEVMODE) */
 	/*!
 	 * \brief TRUE if the container is being destroyed.
 	 *
@@ -956,8 +964,7 @@
 	 *
 	 * \note There should not be any empty nodes in the container
 	 * during destruction.  If there are then an error needs to be
-	 * issued about container node reference leaks.  Someone forgot
-	 * to call ao2_iterator_destroy() somewhere.
+	 * issued about container node reference leaks.
 	 */
 	unsigned int destroying:1;
 };
@@ -967,7 +974,7 @@
  */
 int ao2_container_count(struct ao2_container *c)
 {
-	return c->elements;
+	return ast_atomic_fetchadd_int(&c->elements, 0);
 }
 
 #if defined(AST_DEVMODE)
@@ -1015,7 +1022,10 @@
 		/* Insert the new node. */
 		switch (self->v_table->insert(self, node)) {
 		case AO2_CONTAINER_INSERT_NODE_INSERTED:
+			node->is_linked = 1;
+			ast_atomic_fetchadd_int(&self->elements, 1);
 #if defined(AST_DEVMODE)
+			++self->nodes;
 			switch (self->v_table->type) {
 			case AO2_CONTAINER_RTTI_HASH:
 				hash_ao2_link_node_stat(self, node);
@@ -1024,7 +1034,6 @@
 				break;
 			}
 #endif	/* defined(AST_DEVMODE) */
-			ast_atomic_fetchadd_int(&self->elements, 1);
 
 			res = 1;
 			break;
@@ -1286,6 +1295,13 @@
 				/* update number of elements */
 				ast_atomic_fetchadd_int(&self->elements, -1);
 #if defined(AST_DEVMODE)
+				{
+					int empty = self->nodes - self->elements;
+
+					if (self->max_empty_nodes < empty) {
+						self->max_empty_nodes = empty;
+					}
+				}
 				switch (self->v_table->type) {
 				case AO2_CONTAINER_RTTI_HASH:
 					hash_ao2_unlink_node_stat(self, node);
@@ -1514,6 +1530,13 @@
 			/* update number of elements */
 			ast_atomic_fetchadd_int(&iter->c->elements, -1);
 #if defined(AST_DEVMODE)
+			{
+				int empty = iter->c->nodes - iter->c->elements;
+
+				if (iter->c->max_empty_nodes < empty) {
+					iter->c->max_empty_nodes = empty;
+				}
+			}
 			switch (iter->c->v_table->type) {
 			case AO2_CONTAINER_RTTI_HASH:
 				hash_ao2_unlink_node_stat(iter->c, node);
@@ -1737,6 +1760,18 @@
 
 	ao2_rdlock(self);
 	prnt(fd, "Number of objects: %d\n", self->elements);
+	prnt(fd, "Number of nodes: %d\n", self->nodes);
+	prnt(fd, "Number of empty nodes: %d\n", self->nodes - self->elements);
+	/*
+	 * XXX
+	 * If the max_empty_nodes count gets out of single digits you
+	 * likely have a code path where ao2_iterator_destroy() is not
+	 * called.
+	 *
+	 * Empty nodes do not harm the container but they do make
+	 * container operations less efficient.
+	 */
+	prnt(fd, "Maximum empty nodes: %d\n", self->max_empty_nodes);
 	if (self->v_table->stats) {
 		self->v_table->stats(self, fd, prnt);
 	}
@@ -1783,8 +1818,6 @@
 	AST_DLLIST_ENTRY(hash_bucket_node) links;
 	/*! Hash bucket holding the node. */
 	int my_bucket;
-	/*! TRUE if the node is linked into the container. */
-	unsigned int is_linked:1;
 };
 
 struct hash_bucket {
@@ -1892,7 +1925,7 @@
 {
 	struct hash_bucket_node *doomed = v_doomed;
 
-	if (doomed->is_linked) {
+	if (doomed->common.is_linked) {
 		struct ao2_container_hash *my_container;
 		struct hash_bucket *bucket;
 
@@ -1914,6 +1947,10 @@
 
 		bucket = &my_container->buckets[doomed->my_bucket];
 		AST_DLLIST_REMOVE(&bucket->list, doomed, links);
+
+#if defined(AST_DEVMODE)
+		--my_container->common.nodes;
+#endif	/* defined(AST_DEVMODE) */
 	}
 
 	/*
@@ -1997,7 +2034,6 @@
 				}
 				if (cmp < 0) {
 					AST_DLLIST_INSERT_AFTER_CURRENT(node, links);
-					node->is_linked = 1;
 					return AO2_CONTAINER_INSERT_NODE_INSERTED;
 				}
 				switch (options & AO2_CONTAINER_ALLOC_OPT_DUPS_MASK) {
@@ -2030,7 +2066,6 @@
 				}
 				if (cmp > 0) {
 					AST_DLLIST_INSERT_BEFORE_CURRENT(node, links);
-					node->is_linked = 1;
 					return AO2_CONTAINER_INSERT_NODE_INSERTED;
 				}
 				switch (options & AO2_CONTAINER_ALLOC_OPT_DUPS_MASK) {
@@ -2055,7 +2090,6 @@
 		}
 		AST_DLLIST_INSERT_TAIL(&bucket->list, node, links);
 	}
-	node->is_linked = 1;
 	return AO2_CONTAINER_INSERT_NODE_INSERTED;
 }
 
@@ -2618,7 +2652,6 @@
 }
 #endif	/* defined(AST_DEVMODE) */
 
-#if defined(AST_DEVMODE)
 /*!
  * \internal
  *
@@ -2636,14 +2669,12 @@
 	/* Check that the container no longer has any nodes */
 	for (idx = self->n_buckets; idx--;) {
 		if (!AST_DLLIST_EMPTY(&self->buckets[idx].list)) {
-			ast_log(LOG_ERROR,
-				"Ref leak destroying container.  Container still has nodes!\n");
+			ast_log(LOG_ERROR, "Node ref leak.  Hash container still has nodes!\n");
 			ast_assert(0);
 			break;
 		}
 	}
 }
-#endif	/* defined(AST_DEVMODE) */
 
 #if defined(AST_DEVMODE)
 /*!
@@ -2703,15 +2734,16 @@
 {
 	int bucket_exp;
 	int bucket;
-	int count_node;
 	int count_obj;
 	int count_total_obj;
+	int count_total_node;
 	void *obj_last;
 	struct hash_bucket_node *node;
 	struct hash_bucket_node *prev;
 	struct hash_bucket_node *next;
 
 	count_total_obj = 0;
+	count_total_node = 0;
 
 	/* For each bucket in the container. */
 	for (bucket = 0; bucket < self->n_buckets; ++bucket) {
@@ -2721,7 +2753,6 @@
 			continue;
 		}
 
-		count_node = 0;
 		count_obj = 0;
 		obj_last = NULL;
 
@@ -2783,7 +2814,7 @@
 				return -1;
 			}
 
-			++count_node;
+			++count_total_node;
 			if (!node->common.obj) {
 				/* Node is empty. */
 				continue;
@@ -2830,7 +2861,12 @@
 		return -1;
 	}
 
-/* BUGBUG check empty node count. */
+	/* Check total node count. */
+	if (count_total_node != self->common.nodes) {
+		ast_log(LOG_ERROR, "Total node count of %d does not match stat of %d!\n",
+			count_total_node, self->common.nodes);
+		return -1;
+	}
 
 	return 0;
 }
@@ -2848,8 +2884,8 @@
 	.traverse_next = (ao2_container_find_next_fn) hash_ao2_find_next,
 	.traverse_cleanup = (ao2_container_find_cleanup_fn) hash_ao2_find_cleanup,
 	.iterator_next = (ao2_iterator_next_fn) hash_ao2_iterator_next,
+	.destroy = (ao2_container_destroy_fn) hash_ao2_destroy,
 #if defined(AST_DEVMODE)
-	.destroy = (ao2_container_destroy_fn) hash_ao2_destroy,
 	.stats = (ao2_container_statistics) hash_ao2_stats,
 	.integrity = (ao2_container_integrity) hash_ao2_integrity,
 #endif	/* defined(AST_DEVMODE) */
@@ -3379,12 +3415,12 @@
 static void rb_ao2_node_destructor(void *v_doomed)
 {
 	struct rbtree_node *doomed = v_doomed;
-	struct ao2_container_rbtree *my_container;
 
 /* BUGBUG rb_ao2_node_destructor not written */
 
-	my_container = (struct ao2_container_rbtree *) doomed->common.my_container;
-	if (my_container) {
+	if (doomed->common.is_linked) {
+		struct ao2_container_rbtree *my_container;
+
 		/*
 		 * Promote to write lock if not already there.  Since
 		 * adjust_lock() can potentially release and block waiting for a
@@ -3398,10 +3434,15 @@
 		 * negative reference and the destructor called twice for the
 		 * same node.
 		 */
+		my_container = (struct ao2_container_rbtree *) doomed->common.my_container;
 		adjust_lock(my_container, AO2_LOCK_REQ_WRLOCK, 1);
 
 /* BUGBUG remove the node from the tree and rebalance if the container is not being destroyed. */
 /* BUGBUG rb_delete_fixup not written */
+
+#if defined(AST_DEVMODE)
+		--my_container->common.nodes;
+#endif	/* defined(AST_DEVMODE) */
 	}
 
 	/*
@@ -3895,7 +3936,6 @@
 	int cmp;
 
 	if (self->common.destroying) {
-/* BUGBUG need to check what happens when there are empty nodes during destruction. */
 		/* Force traversal to be post order for tree destruction. */
 		flags = OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE | OBJ_ORDER_POST;
 	}
@@ -4125,7 +4165,6 @@
 	return node;
 }
 
-#if defined(AST_DEVMODE)
 /*!
  * \internal
  *
@@ -4140,12 +4179,10 @@
 {
 	/* Check that the container no longer has any nodes */
 	if (self->root) {
-		ast_log(LOG_ERROR,
-			"Ref leak destroying container.  Container still has nodes!\n");
+		ast_log(LOG_ERROR, "Node ref leak.  Red-Black tree container still has nodes!\n");
 		ast_assert(0);
 	}
 }
-#endif	/* defined(AST_DEVMODE) */
 
 #if defined(AST_DEVMODE)
 /*!
@@ -4230,7 +4267,12 @@
 		return -1;
 	}
 
-/* BUGBUG Check empty node count stat. */
+	/* Check total node count. */
+	if (count_node != self->common.nodes) {
+		ast_log(LOG_ERROR, "Total node count of %d does not match stat of %d!\n",
+			count_node, self->common.nodes);
+		return -1;
+	}
 
 	return 0;
 }
@@ -4248,10 +4290,8 @@
 	.traverse_next = (ao2_container_find_next_fn) rb_ao2_find_next,
 	.traverse_cleanup = (ao2_container_find_cleanup_fn) rb_ao2_find_cleanup,
 	.iterator_next = (ao2_iterator_next_fn) rb_ao2_iterator_next,
+	.destroy = (ao2_container_destroy_fn) rb_ao2_destroy,
 #if defined(AST_DEVMODE)
-	.destroy = (ao2_container_destroy_fn) rb_ao2_destroy,
-/* BUGBUG common stats need to keep track of empty node counts (current and max). */
-//	.stats = (ao2_container_statistics) rb_ao2_stats,
 	.integrity = (ao2_container_integrity) rb_ao2_integrity,
 #endif	/* defined(AST_DEVMODE) */
 };




More information about the asterisk-commits mailing list