[svn-commits] rmudgett: branch rmudgett/ao2_enhancements r371645 - /team/rmudgett/ao2_enhan...
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Thu Aug 23 16:40:41 CDT 2012
Author: rmudgett
Date: Thu Aug 23 16:40:37 2012
New Revision: 371645
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=371645
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.
* Made the hash container integrity check validate the bucket list links.
Modified:
team/rmudgett/ao2_enhancements/main/astobj2.c
Modified: team/rmudgett/ao2_enhancements/main/astobj2.c
URL: http://svnview.digium.com/svn/asterisk/team/rmudgett/ao2_enhancements/main/astobj2.c?view=diff&rev=371645&r1=371644&r2=371645
==============================================================================
--- team/rmudgett/ao2_enhancements/main/astobj2.c (original)
+++ team/rmudgett/ao2_enhancements/main/astobj2.c Thu Aug 23 16:40:37 2012
@@ -768,6 +768,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;
};
/*!
@@ -959,6 +961,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.
*
@@ -967,8 +975,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;
};
@@ -978,7 +985,7 @@
*/
int ao2_container_count(struct ao2_container *c)
{
- return c->elements;
+ return ast_atomic_fetchadd_int(&c->elements, 0);
}
#if defined(AST_DEVMODE)
@@ -1027,14 +1034,16 @@
/* 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);
break;
}
#endif /* defined(AST_DEVMODE) */
- ast_atomic_fetchadd_int(&self->elements, 1);
res = 1;
break;
@@ -1299,6 +1308,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);
@@ -1528,6 +1544,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);
@@ -1752,6 +1775,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);
}
@@ -1799,8 +1834,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 {
@@ -1908,7 +1941,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;
@@ -1930,6 +1963,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) */
}
/*
@@ -2013,7 +2050,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) {
@@ -2046,7 +2082,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) {
@@ -2071,7 +2106,6 @@
}
AST_DLLIST_INSERT_TAIL(&bucket->list, node, links);
}
- node->is_linked = 1;
return AO2_CONTAINER_INSERT_NODE_INSERTED;
}
@@ -2487,6 +2521,7 @@
}
}
+ /* No more nodes in the container left to traverse. */
__ao2_ref(prev, -1);
return NULL;
}
@@ -2638,7 +2673,6 @@
}
#endif /* defined(AST_DEVMODE) */
-#if defined(AST_DEVMODE)
/*!
* \internal
*
@@ -2656,14 +2690,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)
/*!
@@ -2723,120 +2755,137 @@
{
int bucket_exp;
int bucket;
- int count_node_forward;
- int count_node_backward;
- int count_obj_forward;
- int count_obj_backward;
+ int count_obj;
int count_total_obj;
+ int count_total_node;
+ void *obj_last;
struct hash_bucket_node *node;
- struct hash_bucket_node *last;
+ 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) {
- count_node_forward = 0;
- count_node_backward = 0;
- count_obj_forward = 0;
- count_obj_backward = 0;
-
- /* Check forward list. */
- last = NULL;
- for (node = AST_DLLIST_FIRST(&self->buckets[bucket].list);
- node;
- node = AST_DLLIST_NEXT(node, links)) {
- ++count_node_forward;
+ if (!AST_DLLIST_FIRST(&self->buckets[bucket].list)
+ && !AST_DLLIST_LAST(&self->buckets[bucket].list)) {
+ /* The bucket list is empty. */
+ continue;
+ }
+
+ count_obj = 0;
+ obj_last = NULL;
+
+ /* Check bucket list links and nodes. */
+ node = AST_DLLIST_LAST(&self->buckets[bucket].list);
+ if (!node) {
+ ast_log(LOG_ERROR, "Bucket %d list tail is NULL when it should not be!\n",
+ bucket);
+ return -1;
+ }
+ if (AST_DLLIST_NEXT(node, links)) {
+ ast_log(LOG_ERROR, "Bucket %d list tail node is not the last node!\n",
+ bucket);
+ return -1;
+ }
+ node = AST_DLLIST_FIRST(&self->buckets[bucket].list);
+ if (!node) {
+ ast_log(LOG_ERROR, "Bucket %d list head is NULL when it should not be!\n",
+ bucket);
+ return -1;
+ }
+ if (AST_DLLIST_PREV(node, links)) {
+ ast_log(LOG_ERROR, "Bucket %d list head node is not the first node!\n",
+ bucket);
+ return -1;
+ }
+ for (; node; node = next) {
+ /* Check backward link. */
+ prev = AST_DLLIST_PREV(node, links);
+ if (prev) {
+ if (node != AST_DLLIST_NEXT(prev, links)) {
+ ast_log(LOG_ERROR, "Bucket %d list node's prev node does not link back!\n",
+ bucket);
+ return -1;
+ }
+ } else if (node != AST_DLLIST_FIRST(&self->buckets[bucket].list)) {
+ ast_log(LOG_ERROR, "Bucket %d backward list chain is broken!\n",
+ bucket);
+ return -1;
+ }
+
+ /* Check forward link. */
+ next = AST_DLLIST_NEXT(node, links);
+ if (next) {
+ if (node != AST_DLLIST_PREV(next, links)) {
+ ast_log(LOG_ERROR, "Bucket %d list node's next node does not link back!\n",
+ bucket);
+ return -1;
+ }
+ } else if (node != AST_DLLIST_LAST(&self->buckets[bucket].list)) {
+ ast_log(LOG_ERROR, "Bucket %d forward list chain is broken!\n",
+ bucket);
+ return -1;
+ }
if (bucket != node->my_bucket) {
- ast_log(LOG_ERROR, "Node not in correct bucket!\n");
+ ast_log(LOG_ERROR, "Bucket %d node claims to be in bucket %d!\n",
+ bucket, node->my_bucket);
return -1;
}
+ ++count_total_node;
if (!node->common.obj) {
/* Node is empty. */
continue;
}
- ++count_obj_forward;
+ ++count_obj;
/* Check container hash key for expected bucket. */
bucket_exp = abs(self->hash_fn(node->common.obj, OBJ_POINTER));
bucket_exp %= self->n_buckets;
if (bucket != bucket_exp) {
- ast_log(LOG_ERROR, "Hash does not match bucket!\n");
+ ast_log(LOG_ERROR, "Bucket %d node hashes to bucket %d!\n",
+ bucket, bucket_exp);
return -1;
}
/* Check sort if configured. */
- if (last && self->common.sort_fn) {
- if (self->common.sort_fn(last->common.obj, node->common.obj, OBJ_POINTER) > 0) {
- ast_log(LOG_ERROR, "Bucket nodes out of order!\n");
+ if (self->common.sort_fn) {
+ if (obj_last
+ && self->common.sort_fn(obj_last, node->common.obj, OBJ_POINTER) > 0) {
+ ast_log(LOG_ERROR, "Bucket %d nodes out of sorted order!\n",
+ bucket);
return -1;
}
- }
- last = node;
- }
-
- /* Check backward list. */
- last = NULL;
- for (node = AST_DLLIST_LAST(&self->buckets[bucket].list);
- node;
- node = AST_DLLIST_PREV(node, links)) {
- ++count_node_backward;
-
- if (bucket != node->my_bucket) {
- ast_log(LOG_ERROR, "Node not in correct bucket!\n");
- return -1;
- }
-
- if (!node->common.obj) {
- /* Node is empty. */
- continue;
- }
- ++count_obj_backward;
-
- /* Check container hash key for expected bucket. */
- bucket_exp = abs(self->hash_fn(node->common.obj, OBJ_POINTER));
- bucket_exp %= self->n_buckets;
- if (bucket != bucket_exp) {
- ast_log(LOG_ERROR, "Hash does not match bucket!\n");
- return -1;
- }
-
- /* Check sort if configured. */
- if (last && self->common.sort_fn) {
- if (self->common.sort_fn(node->common.obj, last->common.obj, OBJ_POINTER) > 0) {
- ast_log(LOG_ERROR, "Bucket nodes out of order!\n");
- return -1;
- }
- }
- last = node;
- }
-
- /* Check bucket forward/backward node count. */
- if (count_node_forward != count_node_backward) {
- ast_log(LOG_ERROR, "Forward/backward node count does not match!\n");
+ obj_last = node->common.obj;
+ }
+ }
+
+ /* Check bucket obj count statistic. */
+ if (count_obj != self->buckets[bucket].elements) {
+ ast_log(LOG_ERROR, "Bucket %d object count of %d does not match stat of %d!\n",
+ bucket, count_obj, self->buckets[bucket].elements);
return -1;
}
- /* Check bucket forward/backward obj count. */
- if (count_obj_forward != count_obj_backward) {
- ast_log(LOG_ERROR, "Forward/backward object count does not match!\n");
- return -1;
- }
-
- /* Check bucket obj count statistic. */
- if (count_obj_forward != self->buckets[bucket].elements) {
- ast_log(LOG_ERROR, "Bucket object count stat does not match!\n");
- return -1;
- }
-
/* Accumulate found object counts. */
- count_total_obj += count_obj_forward;
+ count_total_obj += count_obj;
}
/* Check total obj count. */
if (count_total_obj != ao2_container_count(&self->common)) {
- ast_log(LOG_ERROR, "Total object count does not match ao2_container_count()!\n");
+ ast_log(LOG_ERROR,
+ "Total object count of %d does not match ao2_container_count() of %d!\n",
+ count_total_obj, ao2_container_count(&self->common));
+ return -1;
+ }
+
+ /* 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;
}
@@ -2856,8 +2905,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) */
@@ -3089,7 +3138,7 @@
ast_cli(a->fd, "testing callbacks again\n");
ao2_t_callback(c1, 0, print_cb, a, "test callback");
- ast_verbose("now you should see an error message:\n");
+ ast_verbose("now you should see an error and possible assertion failure messages:\n");
ao2_t_ref(&i, -1, ""); /* i is not a valid object so we print an error here */
ast_cli(a->fd, "destroy container\n");
More information about the svn-commits
mailing list