nvl: use a red-black tree to keep track of name-value pairs

Instead of using an unbalanced binary search tree, use a balanced one.  This
should both improve the performance and make it more predictable.

Signed-off-by: Josef 'Jeff' Sipek <[email protected]>

Author: jeffpc

fmt_cbor.c

@@ -234,21 +234,21 @@ int cbor_pack_map_end(struct buffer *buffer, size_t npairs)
 
 int cbor_pack_map_val(struct buffer *buffer, struct val *val)
 {
-	struct bst_tree *tree = &val->_set_nvl.values;
+	struct rb_tree *tree = &val->_set_nvl.values;
 	struct nvpair *cur;
 	size_t npairs;
 	int ret;
 
 	if (val->type != VT_NVL)
 		return -EINVAL;
 
-	npairs = bst_numnodes(tree);
+	npairs = rb_numnodes(tree);
 
 	ret = cbor_pack_map_start(buffer, npairs);
 	if (ret)
 		return ret;
 
-	bst_for_each(tree, cur) {
+	rb_for_each(tree, cur) {
 		ret = cbor_pack_str(buffer, cur->name);
 		if (ret)
 			return ret;

include/jeffpc/nvl.h

@@ -34,7 +34,7 @@ struct nvlist {
 
 /* do not access these directly */
 struct nvpair {
-	struct bst_node node;
+	struct rb_node node;
 
 	struct str *name;
 	struct val *value;

include/jeffpc/val.h

@@ -31,7 +31,7 @@
 #include <jeffpc/error.h>
 #include <jeffpc/types.h>
 #include <jeffpc/list.h>
-#include <jeffpc/bst.h>
+#include <jeffpc/rbtree.h>
 
 /*
  * A typed value structure.
@@ -101,7 +101,7 @@ struct val {
 			 * a balanced binary search tree implementation, we
 			 * should switch to it.
 			 */
-			struct bst_tree values;
+			struct rb_tree values;
 		} nvl;
 
 		/*
@@ -132,7 +132,7 @@ struct val {
 			size_t nelem;
 		} _set_array;
 		struct {
-			struct bst_tree values;
+			struct rb_tree values;
 		} _set_nvl;
 	};
 };

nvl.c

@@ -49,7 +49,7 @@ static struct nvpair *find(struct nvlist *nvl, const char *name)
 		.name = &name_str,
 	};
 
-	return bst_find(&nvl->val._set_nvl.values, &key, NULL);
+	return rb_find(&nvl->val._set_nvl.values, &key, NULL);
 }
 
 /*
@@ -58,13 +58,13 @@ static struct nvpair *find(struct nvlist *nvl, const char *name)
 
 const struct nvpair *nvl_iter_start(struct nvlist *nvl)
 {
-	return bst_first(&nvl->val._set_nvl.values);
+	return rb_first(&nvl->val._set_nvl.values);
 }
 
 const struct nvpair *nvl_iter_next(struct nvlist *nvl,
 				   const struct nvpair *prev)
 {
-	return bst_next(&nvl->val._set_nvl.values, (void *) prev);
+	return rb_next(&nvl->val._set_nvl.values, (void *) prev);
 }
 
 /*
@@ -133,7 +133,7 @@ static inline int do_nvl_set(struct nvlist *nvl, const char *cname,
 			return -ENOMEM;
 		}
 
-		bst_add(&nvl->val._set_nvl.values, pair);
+		rb_add(&nvl->val._set_nvl.values, pair);
 	} else {
 		str_putref(name);
 	}
@@ -211,7 +211,7 @@ static int unset(struct nvlist *nvl, const char *name, enum val_type type,
 	if (matchtype && (pair->value->type != type))
 		return -ERANGE;
 
-	bst_remove(&nvl->val._set_nvl.values, pair);
+	rb_remove(&nvl->val._set_nvl.values, pair);
 
 	__nvpair_free(pair);
 

sexpr.c

@@ -381,13 +381,13 @@ bool sexpr_equal(struct val *lhs, struct val *rhs)
 			goto out;
 		}
 		case VT_NVL: {
-			struct bst_tree *ltree = &lhs->_set_nvl.values;
-			struct bst_tree *rtree = &rhs->_set_nvl.values;
+			struct rb_tree *ltree = &lhs->_set_nvl.values;
+			struct rb_tree *rtree = &rhs->_set_nvl.values;
 			struct nvpair *lcur;
 			struct nvpair *rcur;
 
-			lcur = bst_first(ltree);
-			rcur = bst_first(rtree);
+			lcur = rb_first(ltree);
+			rcur = rb_first(rtree);
 
 			while (lcur && rcur) {
 				ret = (str_cmp(lcur->name, rcur->name) == 0);
@@ -399,8 +399,8 @@ bool sexpr_equal(struct val *lhs, struct val *rhs)
 				if (!ret)
 					goto out;
 
-				lcur = bst_next(ltree, lcur);
-				rcur = bst_next(rtree, rcur);
+				lcur = rb_next(ltree, lcur);
+				rcur = rb_next(rtree, rcur);
 			}
 
 			/* if both sides reached the end, then they are equal */

val_dump.c

@@ -126,12 +126,12 @@ static void do_val_dump_file(FILE *out, struct val *val, int indent)
 			break;
 		}
 		case VT_NVL: {
-			struct bst_tree *tree = &val->_set_nvl.values;
+			struct rb_tree *tree = &val->_set_nvl.values;
 			struct nvpair *cur;
 
-			fprintf(out, " items=%zu\n", bst_numnodes(tree));
+			fprintf(out, " items=%zu\n", rb_numnodes(tree));
 
-			bst_for_each(tree, cur) {
+			rb_for_each(tree, cur) {
 				doindent(out, indent);
 				fprintf(out, "name='%s' ", str_cstr(cur->name));
 				do_val_dump_file(out, cur->value, indent + 1);

val_nvl.c

@@ -81,24 +81,24 @@ struct val *val_alloc_nvl(void)
 	if (IS_ERR(val))
 		return val;
 
-	bst_create(&val->_set_nvl.values, val_nvl_cmp, sizeof(struct nvpair),
+	rb_create(&val->_set_nvl.values, val_nvl_cmp, sizeof(struct nvpair),
 		   offsetof(struct nvpair, node));
 
 	return val;
 }
 
 void __val_free_nvl(struct val *val)
 {
-	struct bst_cookie cookie;
+	struct rb_cookie cookie;
 	struct nvpair *cur;
 
 	ASSERT(val);
 	ASSERT3U(refcnt_read(&val->refcnt), ==, 0);
 	ASSERT3U(val->type, ==, VT_NVL);
 
-	memset(&cookie, 0, sizeof(struct bst_cookie));
-	while ((cur = bst_destroy_nodes(&val->_set_nvl.values, &cookie)))
+	memset(&cookie, 0, sizeof(struct rb_cookie));
+	while ((cur = rb_destroy_nodes(&val->_set_nvl.values, &cookie)))
 		__nvpair_free(cur);
 
-	bst_destroy(&val->_set_nvl.values);
+	rb_destroy(&val->_set_nvl.values);
 }