[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