From: Dana Jansens <danakj@orodu.net>
Date: Mon, 2 Nov 2009 14:51:29 +0000 (-0500)
Subject: Remove left_child and right_child from GTreeNode in favor of parent.
X-Git-Url: http://git.openbox.org/?a=commitdiff_plain;h=bf0e75da0778d41845eed6c9556dbb0da3131806;p=dana%2Fcg-glib.git

Remove left_child and right_child from GTreeNode in favor of parent.

For persistence to work, a node needs to know where its incoming pointers are.  If node->right/left can point to the next/previous node in the tree, then we might have an incoming pointer from our previous node, but don't have a pointer to it.  Finding the next/previous node takes up to log(n) time, which isn't cool to do every time we change a node.  So, if we want next/previous pointers, we need to always have the pointer, so that it can be recipricated, and thus would require two extra pointer spots beyond left/right to store that.

Because a node points to its children, they also need to be able to find their incoming pointer in their parent.  We add a pointer for this, which allows us to still navigate the tree, and thus we don't need to add next/previous pointers also.

Also remove test code that tests the right/left pointers as next/prev.
---

diff --git a/glib/gtree.c b/glib/gtree.c
index 1a2e2e1e..3ea6f711 100644
--- a/glib/gtree.c
+++ b/glib/gtree.c
@@ -74,10 +74,6 @@ struct _GTreeNodeVersion
   GTreeNode *left;        /* left subtree */
   GTreeNode *right;       /* right subtree */
   GTreeNode *parent;      /* parent node */
-  guint8     left_child;  /* the node has a left child (else left points to the
-                             previous node in the tree) */
-  guint8     right_child; /* the node has a right child (else right points to
-                             the next node in the tree) */
 };
 
 struct _GTreeNodeData
@@ -102,8 +98,6 @@ struct _GTreeNode
 #define left(i)        v[i].left
 #define right(i)       v[i].right
 #define parent(i)      v[i].parent
-#define left_child(i)  v[i].left_child
-#define right_child(i) v[i].right_child
 
 static GTreeNode* g_tree_node_new                   (GTree           *tree,
                                                      gpointer         key,
@@ -167,8 +161,6 @@ g_tree_node_new (GTree   *tree,
   node->v[0].left = NULL;
   node->v[0].right = NULL;
   node->v[0].parent = NULL;
-  node->v[0].left_child = FALSE;
-  node->v[0].right_child = FALSE;
 
   return node;
 }
@@ -327,7 +319,7 @@ g_tree_first_node (GTree *tree,
 
   tmpv = g_tree_node_find_version (tmp = rv->root, version);
 
-  while (tmpv->left_child)
+  while (tmpv->left)
     tmpv = g_tree_node_find_version (tmp = tmpv->left, version);
 
   return tmp;
@@ -341,14 +333,26 @@ g_tree_node_previous (GTreeNode *node,
   GTreeNodeVersion *nodev, *tmpv;
 
   nodev = g_tree_node_find_version (node, version);
-  if (!nodev->left)
-    return NULL;
-  tmpv = g_tree_node_find_version (tmp = nodev->left, version);
-
-  if (nodev->left_child)
-    while (tmpv->right_child)
-      tmpv = g_tree_node_find_version (tmp = tmpv->right, version);
 
+  if (nodev->left)
+    {
+      tmpv = g_tree_node_find_version (tmp = nodev->left, version);
+      while (tmpv->right)
+        tmpv = g_tree_node_find_version (tmp = tmpv->right, version);
+    }
+  else
+    {
+      tmp = nodev->parent;
+      while (tmp)
+        {
+          tmpv = g_tree_node_find_version (tmp, version);
+          if (tmpv->right == node)
+              break;
+          node = tmp;
+          nodev = tmpv;
+          tmp = tmpv->parent;
+        }
+    }
   return tmp;
 }
 		  
@@ -360,41 +364,66 @@ g_tree_node_next (GTreeNode *node,
   GTreeNodeVersion *nodev, *tmpv;
 
   nodev = g_tree_node_find_version (node, version);
-  if (!nodev->right)
-    return NULL;
-  tmpv = g_tree_node_find_version (tmp = nodev->right, version);
-
-  if (nodev->right_child)
-    while (tmpv->left_child)
-      tmpv = g_tree_node_find_version (tmp = tmpv->left, version);
 
+  if (nodev->right)
+    {
+      tmpv = g_tree_node_find_version (tmp = nodev->right, version);
+      while (tmpv->left)
+        tmpv = g_tree_node_find_version (tmp = tmpv->left, version);
+    }
+  else
+    {
+      tmp = nodev->parent;
+      while (tmp)
+        {
+          tmpv = g_tree_node_find_version (tmp, version);
+          if (tmpv->left == node)
+              break;
+          node = tmp;
+          nodev = tmpv;
+          tmp = tmpv->parent;
+        }
+    }
   return tmp;
 }
 
-static void
-g_tree_remove_all (GTree *tree)
+static inline void
+g_tree_node_remove (GTree *tree, GTreeNode *node)
 {
-  GTreeNode *node;
-  GTreeNode *next;
-
-  g_return_if_fail (tree != NULL);
+  if (!node)
+    return;
 
-  // XXX worry about removing all versions, not just the latest
+  g_tree_node_remove (tree, node->left(NOW));
+  g_tree_node_remove (tree, node->right(NOW));
 
-  node = g_tree_first_node (tree, tree->version);
+  if (tree->key_destroy_func)
+    tree->key_destroy_func (node->data->key);
+  if (tree->value_destroy_func)
+    tree->value_destroy_func (node->data->value);
 
-  while (node)
+  g_slice_free1 (sizeof(GTreeNodeVersion) *
+                 (node->version(NOW) == 0 ? 1 : MAX_IN_DEGREE + 1),
+                 node->v);
+  if (--node->data->ref_count == 0)
     {
-      next = g_tree_node_next (node, tree->version);
-
       if (tree->key_destroy_func)
-	tree->key_destroy_func (node->data->key);
+        tree->key_destroy_func (node->data->key);
       if (tree->value_destroy_func)
-	tree->value_destroy_func (node->data->value);
-      g_slice_free (GTreeNode, node);
-
-      node = next;
+        tree->value_destroy_func (node->data->value);
+      g_slice_free (GTreeNodeData, node->data);
     }
+  g_slice_free (GTreeNode, node);
+
+}
+
+static void
+g_tree_remove_all (GTree *tree)
+{
+  g_return_if_fail (tree != NULL);
+
+  // XXX worry about removing all versions, not just the latest
+
+  g_tree_node_remove (tree, tree->root(NOW));
 
   tree->roots[0].root = NULL;
   tree->nnodes = 0;
@@ -550,14 +579,8 @@ g_tree_node_next_version (GTree     *tree,
       prev = g_tree_node_previous (node, tree->version);
       next = g_tree_node_next (node, tree->version);
       parent = node->parent(NOW);
-      if (node->left_child(NOW))
-        left = node->left(NOW);
-      else
-        left = NULL;
-      if (node->right_child(NOW))
-        right = node->right(NOW);
-      else
-        right = NULL;
+      left = node->left(NOW);
+      right = node->right(NOW);
 
       if (prev && prev->right(NOW) == node)
         {
@@ -751,21 +774,15 @@ g_tree_insert_internal (GTree    *tree,
         }
       else if (cmp < 0)
         {
-          if (node->left_child(NOW))
+          if (node->left(NOW))
               node = node->left(NOW);
           else
             {
               child = g_tree_node_new (tree, key, value);
-
               node = g_tree_node_next_version (tree, node);
 
-              /* child is created at the current version */
-              child->v[0].left = node->left(NOW);
-              child->v[0].right = node;
               child->v[0].parent = node;
-
               node->v[0].left = child;
-              node->v[0].left_child = TRUE;
 
 	      tree->nnodes++;
 
@@ -774,21 +791,15 @@ g_tree_insert_internal (GTree    *tree,
         }
       else
         {
-          if (node->right_child(NOW))
+          if (node->right(NOW))
               node = node->right(NOW);
           else
             {
               child = g_tree_node_new (tree, key, value);
-
               node = g_tree_node_next_version (tree, node);
 
-              /* child is created at the current version */
-              child->v[0].right = node->right(NOW);
-              child->v[0].left = node;
               child->v[0].parent = node;
-
               node->v[0].right = child;
-              node->v[0].right_child = TRUE;
 
 	      tree->nnodes++;
 
@@ -927,7 +938,7 @@ g_tree_remove_internal (GTree         *tree,
         break;
       else if (cmp < 0)
         {
-          if (!node->left_child(NOW))
+          if (!node->left(NOW))
             return FALSE;
 
 	  parent = node;
@@ -936,7 +947,7 @@ g_tree_remove_internal (GTree         *tree,
         }
       else
         {
-          if (!node->right_child(NOW))
+          if (!node->right(NOW))
             return FALSE;
 
 	  parent = node;
@@ -946,14 +957,14 @@ g_tree_remove_internal (GTree         *tree,
     }
 
   /* rotate the node down the tree, maintaining the heap property */
-  while (node->left_child(NOW) || node->right_child(NOW))
+  while (node->left(NOW) || node->right(NOW))
     {
       /* we're changing this node, make sure our pointer will stay valid
          when we rotate it */
       node = g_tree_node_next_version (tree, node);
 
-      if (!node->left_child(NOW) ||
-          (node->right_child(NOW) &&
+      if (!node->left(NOW) ||
+          (node->right(NOW) &&
            g_tree_priority (node->left(NOW)) >
            g_tree_priority (node->right(NOW))))
         {
@@ -1009,15 +1020,9 @@ g_tree_remove_internal (GTree         *tree,
     {
       parent = g_tree_node_next_version (tree, parent);
       if (is_leftchild)
-        {
-          parent->v[0].left_child = FALSE;
-          parent->v[0].left = node->left(NOW);
-        }
+          parent->v[0].left = NULL;
       else
-        {
-          parent->v[0].right_child = FALSE;
-          parent->v[0].right = node->right(NOW);
-        }
+          parent->v[0].right = NULL;
     }
 
   tree->nnodes--;
@@ -1317,8 +1322,8 @@ g_tree_node_height(GTreeNode *node)
 {
   gint l = 0, r = 0;
   if (node == NULL) return 0;
-  if (node->left_child(NOW)) l = g_tree_node_height (node->left(NOW));
-  if (node->right_child(NOW)) r = g_tree_node_height (node->right(NOW));
+  if (node->left(NOW)) l = g_tree_node_height (node->left(NOW));
+  if (node->right(NOW)) r = g_tree_node_height (node->right(NOW));
   return 1 + MAX(l, r);
 }
 
@@ -1384,7 +1389,7 @@ g_tree_find_node (GTree           *tree,
           GTreeNodeVersion *nodev = g_tree_node_find_version (node, version);
           if (search_type == G_TREE_SEARCH_SUCCESSOR)
             remember = node;
-	  if (!nodev->left_child)
+	  if (!nodev->left)
 	    return remember;
 
 	  node = nodev->left;
@@ -1394,7 +1399,7 @@ g_tree_find_node (GTree           *tree,
           GTreeNodeVersion *nodev = g_tree_node_find_version (node, version);
           if (search_type == G_TREE_SEARCH_PREDECESSOR)
             remember = node;
-	  if (!nodev->right_child)
+	  if (!nodev->right)
 	    return remember;
 
 	  node = nodev->right;
@@ -1410,13 +1415,13 @@ g_tree_node_pre_order (GTreeNode     *node,
   if ((*traverse_func) (node->data->key, node->data->value, data))
     return TRUE;
 
-  if (node->left_child(NOW))
+  if (node->left(NOW))
     {
       if (g_tree_node_pre_order (node->left(NOW), traverse_func, data))
 	return TRUE;
     }
 
-  if (node->right_child(NOW))
+  if (node->right(NOW))
     {
       if (g_tree_node_pre_order (node->right(NOW), traverse_func, data))
 	return TRUE;
@@ -1430,7 +1435,7 @@ g_tree_node_in_order (GTreeNode     *node,
 		      GTraverseFunc  traverse_func,
 		      gpointer       data)
 {
-  if (node->left_child(NOW))
+  if (node->left(NOW))
     {
       if (g_tree_node_in_order (node->left(NOW), traverse_func, data))
 	return TRUE;
@@ -1439,7 +1444,7 @@ g_tree_node_in_order (GTreeNode     *node,
   if ((*traverse_func) (node->data->key, node->data->value, data))
     return TRUE;
 
-  if (node->right_child(NOW))
+  if (node->right(NOW))
     {
       if (g_tree_node_in_order (node->right(NOW), traverse_func, data))
 	return TRUE;
@@ -1453,13 +1458,13 @@ g_tree_node_post_order (GTreeNode     *node,
 			GTraverseFunc  traverse_func,
 			gpointer       data)
 {
-  if (node->left_child(NOW))
+  if (node->left(NOW))
     {
       if (g_tree_node_post_order (node->left(NOW), traverse_func, data))
 	return TRUE;
     }
 
-  if (node->right_child(NOW))
+  if (node->right(NOW))
     {
       if (g_tree_node_post_order (node->right(NOW), traverse_func, data))
 	return TRUE;
@@ -1495,7 +1500,7 @@ g_tree_node_search (GTreeNode       *node,
           GTreeNodeVersion *nodev = g_tree_node_find_version (node, version);
           if (search_type == G_TREE_SEARCH_SUCCESSOR)
             remember = node;
-	  if (!nodev->left_child)
+	  if (!nodev->left)
 	    return remember;
 
 	  node = nodev->left;
@@ -1505,7 +1510,7 @@ g_tree_node_search (GTreeNode       *node,
           GTreeNodeVersion *nodev = g_tree_node_find_version (node, version);
           if (search_type == G_TREE_SEARCH_PREDECESSOR)
             remember = node;
-	  if (!nodev->right_child)
+	  if (!nodev->right)
 	    return remember;
 	  
 	  node = nodev->right;
@@ -1523,17 +1528,13 @@ g_tree_node_rotate_left (GTree     *tree,
 
   right = g_tree_node_next_version (tree, node->right(NOW));
 
-  if (right->left_child(NOW))
+  if (right->left(NOW))
     {
       node->v[0].right = g_tree_node_next_version (tree, right->left(NOW));
       node->v[0].right->v[0].parent = node;
     }
   else
-    {
-      node->v[0].right_child = FALSE;
-      node->v[0].right = right;
-      right->v[0].left_child = TRUE;
-    }
+      node->v[0].right = NULL;
   right->v[0].left = node;
   right->v[0].parent = node->parent(NOW);
   node->v[0].parent = right;
@@ -1551,17 +1552,13 @@ g_tree_node_rotate_right (GTree     *tree,
 
   left = g_tree_node_next_version (tree, node->left(NOW));
 
-  if (left->right_child(NOW))
+  if (left->right(NOW))
     {
       node->v[0].left = g_tree_node_next_version(tree, left->right(NOW));
       node->v[0].left->v[0].parent = node;
     }
   else
-    {
-      node->v[0].left_child = FALSE;
-      node->v[0].left = left;
-      left->v[0].right_child = TRUE;
-    }
+    node->v[0].left = NULL;
   left->v[0].right = node;
   left->v[0].parent = node->v[0].parent;
   node->v[0].parent = left;
@@ -1574,7 +1571,6 @@ static void
 g_tree_node_check (GTreeNode *node,
                    guint      version)
 {
-  GTreeNode *tmp;
   GTreeNodeVersion *nv, *tmpv;
 
   if (node)
@@ -1583,20 +1579,14 @@ g_tree_node_check (GTreeNode *node,
 
       g_assert (nv->left == NULL || nv->left != nv->right);
 
-      if (nv->left_child)
+      if (nv->left)
 	{
-	  tmp = g_tree_node_previous (node, version);
-	  g_assert (g_tree_node_find_version (tmp, version)->right == node);
-
           tmpv = g_tree_node_find_version (nv->left, version);
           g_assert (tmpv->parent == node);
 	}
 
-      if (nv->right_child)
+      if (nv->right)
 	{
-	  tmp = g_tree_node_next (node, version);
-	  g_assert (g_tree_node_find_version (tmp, version)->left == node);
-
           tmpv = g_tree_node_find_version (nv->right, version);
           g_assert (tmpv->parent == node);
 	}
@@ -1607,9 +1597,9 @@ g_tree_node_check (GTreeNode *node,
           g_assert (tmpv->left == node || tmpv->right == node);
         }
 
-      if (nv->left_child)
+      if (nv->left)
 	g_tree_node_check (nv->left, version);
-      if (nv->right_child)
+      if (nv->right)
 	g_tree_node_check (nv->right, version);
     }
 }
@@ -1623,15 +1613,15 @@ g_tree_node_dump (GTreeNode *node,
 
   g_print ("%*s%c\n", indent, "", *(char *)node->data->key);
 
-  if (nv->left_child)
+  if (nv->left)
     g_tree_node_dump (nv->left, version, indent + 2);
-  else if (nv->left)
-    g_print ("%*s<%c\n", indent + 2, "", *(char *)nv->left->data->key);
+  else if ((node = g_tree_node_previous (node, version)))
+    g_print ("%*s<%c\n", indent + 2, "", *(char *)node->data->key);
 
-  if (nv->right_child)
+  if (nv->right)
     g_tree_node_dump (nv->right, version, indent + 2);
-  else if (nv->right)
-    g_print ("%*s>%c\n", indent + 2, "", *(char *)nv->right->data->key);
+  else if ((node = g_tree_node_next (node, version)))
+    g_print ("%*s>%c\n", indent + 2, "", *(char *)node->data->key);
 }