working deleting of versions, just a few memory leaks left.

diff --git a/glib/gtree.c b/glib/gtree.c
index d78f0b6a0e3df35add94c31bdecd863ce9a177e8..26efc38f0f52be586ce5bd1846409c4f386258a7 100644 (file)
--- a/glib/gtree.c
+++ b/glib/gtree.c
@@ -67,6 +67,7 @@ struct _GTree
   guint             nnodes;
   gint              ref_count;
   guint             version;
+  guint             lowest_version;
 };
 
 #define root(i)         roots[i].root
@@ -249,6 +250,7 @@ g_tree_new_full (GCompareDataFunc key_compare_func,
   tree->nnodes             = 0;
   tree->ref_count          = 1;
   tree->version            = 0;
+  tree->lowest_version     = 0;
 
   tree->roots[0].root      = NULL;
   tree->roots[0].version   = 0;
@@ -311,6 +313,7 @@ g_tree_node_find_version(GTreeNode *node,
   for (n = v+(nv-1); n != v; --n)
     if (n->version <= version)
       break;
+  /* searched for something smaller than the lowest version in the node */
   g_assert (n != v);
   return n;
 }
@@ -411,19 +414,24 @@ g_tree_node_remove (GTree *tree, GTreeNode *node)
   if (tree->value_destroy_func)
     tree->value_destroy_func (node->data->value);
 
-  g_slice_free1 (sizeof(GTreeNodeVersion) *
-                 (node->version(NOW) == 0 ? 1 : TABLE_SIZE),
-                 node->v);
   if (--node->data->ref_count == 0)
     {
-      if (tree->key_destroy_func)
-        tree->key_destroy_func (node->data->key);
-      if (tree->value_destroy_func)
-        tree->value_destroy_func (node->data->value);
+      if (!node->data->stolen)
+        {
+          if (tree->key_destroy_func)
+            tree->key_destroy_func (node->data->key);
+          if (tree->value_destroy_func)
+            tree->value_destroy_func (node->data->value);
+        }
       g_slice_free (GTreeNodeData, node->data);
     }
-  g_slice_free (GTreeNode, node);
 
+  g_slice_free1 (sizeof(GTreeNodeVersion) *
+                 (node->version(NOW) == 0 ? 1 : TABLE_SIZE),
+                 node->v);
+  node->v = NULL;
+  node->data = NULL;
+  g_slice_free (GTreeNode, node);
 }
 
 static void
@@ -431,11 +439,14 @@ g_tree_remove_all (GTree *tree)
 {
   g_return_if_fail (tree != NULL);
 
-  // XXX worry about removing all versions, not just the latest
+  /* delete all the history so there is only the latest version left */
+  if (tree->version > 0)
+    g_tree_delete_versions (tree, tree->version-1);
 
   g_tree_node_remove (tree, tree->root(NOW));
 
   tree->roots[0].root = NULL;
+  tree->roots[0].version = tree->version = tree->lowest_version = 0;
   tree->nnodes = 0;
 }
 
@@ -559,223 +570,190 @@ previous_version(guint i, guint n)
     return i-1; /* it was someone else in the array */
 }
 
-/* returns the lowest version > @version in the subtree rooted at @node */
 static guint
-g_tree_node_delete_version (GTree     *tree,
-                            GTreeNode *node,
-                            guint      pnext,
-                            guint      version)
+g_tree_node_delete_versions (GTree     *tree,
+                             GTreeNode *node,
+                             guint      pnext,
+                             guint      version)
 {
-  GTreeNodeVersion *nv;
-  guint next, i, lnext, rnext, to;
+  guint rm, i, nv, next, nextv, lnext, rnext;
 
   if (!node)
     return 0;
 
-  nv = g_tree_node_find_version (node, version);
+  nv = node->nv;
 
-  /* next is the index of the next versioned-pointers in the node */
-  i = nv - node->v;
-  next = next_version (nv - node->v, node->nv);
+  rm = 0;
+  for (i = nv > 1 ? 1 : 0; i < nv && node->v[i].version <= version; i = next)
+    {
+      next = next_version (i, nv);
 
-  /* if the next version of pointers uses the child still then include that
-     version in pnext, otherwise we won't be left with a pointer to it anyways
-     so tell it 0. */
-  if (next < node->nv && node->v[next].left == nv->left)
-    lnext = MIN ((pnext ? pnext : node->v[next].version),
-                 node->v[next].version ? node->v[next].version : pnext);
-  else
-    lnext = 0;
-  lnext = g_tree_node_delete_version (tree, nv->left, lnext, version);
+      if (next < nv)
+        nextv = node->v[next].version - 1;
+      else
+        nextv = version;
+
+      if (next < nv && node->v[i].left &&
+          node->v[next].left == node->v[i].left &&
+          (!pnext || node->v[next].version <= pnext))
+        lnext = node->v[next].version;
+      else if (next == nv || node->v[next].version > pnext)
+        lnext = pnext;
+      else
+        lnext = 0;
+      lnext = g_tree_node_delete_versions (tree, node->v[i].left, lnext,
+                                           nextv);
+
+      if (next < nv && node->v[i].right &&
+          node->v[next].right == node->v[i].right &&
+          (!pnext || node->v[next].version < pnext))
+        rnext = node->v[next].version;
+      else if (next == nv || node->v[next].version > pnext)
+        rnext = pnext;
+      else
+        rnext = 0;
+      rnext = g_tree_node_delete_versions (tree, node->v[i].right, rnext,
+                                           nextv);
+
+      nextv = MIN ((pnext ? pnext : (lnext ? lnext : rnext)),
+                   MIN ((lnext ? lnext : (rnext ? rnext : pnext)),
+                        (rnext ? rnext : (lnext ? lnext : pnext))));
+
+      if (nextv && (next == nv || node->v[next].version > nextv))
+        { /* leave this one behind as there are more pointers coming here */
+          node->v[i].version = nextv;
+          break;
+        }
 
-  if (next < node->nv && node->v[next].right == nv->right)
-    rnext = MIN ((pnext ? pnext : node->v[next].version),
-                 node->v[next].version ? node->v[next].version : pnext);
-  else
-    rnext = 0;
-  rnext = g_tree_node_delete_version (tree, nv->right, rnext, version);
+      /* am i a root? */
+//      if (node->v[i].parent == NULL)
+//        {
+//          GTreeRootVersion *rv;
+//
+//          rv = g_tree_root_find_version (tree, node->v[i].version);
+//          rv->root = NULL;
+//        }
 
-  /* now next is the next version in this node */
-  if (next < node->nv)
-    next = node->v[next].version;
-  else
-    next = 0;
+      ++rm;
+    }
+
+  /* remove the first 'rm' versioned pointers from the node, they are <=
+     'version' */
+  for (i = 1; rm && i+rm < nv; ++i)
+    node->v[i] = node->v[i+rm];
+  node->nv -= rm;
 
-  if (nv->version == version)
+  /* if we removed the last version inside the node, then we can free it */
+  if (node->nv == 0)
     {
-      /* change this pointer's version to this value.
-         this removes the pointer for the current
-         version, but keeps it for versions >= 'to'.
-         if there was nothing seen >= 'version' in any subtrees or the path
-         from the root, then change to version+1 (and stick around)
-         else, change to the smallest version seen so far, because nothing in
-         between exists in this subtree, or in the path from root
-      */
-      to = MAX (nv->version+1, 
-                MIN ((pnext ? pnext : MAX (lnext, rnext)),
-                     MIN ((lnext ? lnext : MAX (pnext, rnext)),
-                          (rnext ? rnext : MAX (pnext, lnext)))));
-
-      if (to < next)
-        {
-          g_assert (to <= tree->version); /* next should be 0 otherwise */
-          next = nv->version = to;
-        }
-      else
+      if (--node->data->ref_count == 0)
         {
-          /* already have a different set of pointers for that version, so we
-             can remove this set entirely */
-          if (i == 0)
+          if (!node->data->stolen)
             {
-              node->v[0] = node->v[node->nv-1];
-              node->nv--;
-            }
-          else
-            {
-              for (; i < node->nv-1; ++i)
-                node->v[i] = node->v[i+1];
-              node->nv--;
+              if (tree->key_destroy_func)
+                tree->key_destroy_func (node->data->key);
+              if (tree->value_destroy_func)
+                tree->value_destroy_func (node->data->value);
             }
+          g_slice_free (GTreeNodeData, node->data);
         }
 
-      /* if we removed the last version inside the node, then we can free it */
-      if (node->nv == 0)
-        {
-          if (--node->data->ref_count == 0)
-            {
-              if (!node->data->stolen)
-                {
-                  if (tree->key_destroy_func)
-                    tree->key_destroy_func (node->data->key);
-                  if (tree->value_destroy_func)
-                    tree->value_destroy_func (node->data->value);
-                }
-              g_slice_free (GTreeNodeData, node->data);
-            }
-
-          g_slice_free1 (sizeof(GTreeNodeVersion) *
-                         (node->version(NOW) == 0 ? 1 : TABLE_SIZE),
-                         node->v);
-          node->v = NULL;
-          node->data = NULL;
-          g_slice_free (GTreeNode, node);
-        }
+      g_slice_free1 (sizeof(GTreeNodeVersion) *
+                     (node->version(NOW) == 0 ? 1 : TABLE_SIZE),
+                     node->v);
+      node->v = NULL;
+      node->data = NULL;
+      g_slice_free (GTreeNode, node);
+      node = NULL;
     }
 
+  //nextv = (node && node->nv ?
+  //         (node->nv > 1 ? node->v[1].version : node->v[0].version) : 0);
+  //if (nextv > version)
+  //  nextv = 0;
+  //return nextv;
 
-  /* return the smallest version > the one being deleted that was seen
-     in this node or its children, or 0 if there was none */
-
-  return MIN ((next ? next : MAX (lnext, rnext)),
-              MIN ((lnext ? lnext : MAX (next, rnext)),
-                   (rnext ? rnext : MAX (next, lnext))));
+  /* return the lowest version left in the node, or 0 if its gone */
+  return (!node || !node->nv ? 0 :
+          (node->nv > 1 ? node->v[1].version : node->v[0].version));
 }
 
 void
-g_tree_delete_version (GTree *tree,
-                       guint  version)
+g_tree_delete_versions (GTree *tree,
+                        guint  version)
 {
   GTreeRootVersion *rv;
-  guint next, rnext;
+  guint rm, i, l, keep, next;
 
-  rv = g_tree_root_find_version (tree, version);
+  g_return_if_fail (tree != NULL);
+  g_return_if_fail (version < tree->version);
 
-  rnext = next_version (rv - tree->roots, tree->nroots);
-  if (rnext == tree->nroots)
-    rnext = rv->version < tree->version ? rv->version+1 : 0;
-  else if (tree->roots[rnext].root == rv->root)
-    rnext = tree->roots[rnext].version;
-  else
-    rnext = 0;
+  if (version < tree->lowest_version)
+    return;
+  tree->lowest_version = version+1;
+
+  rv = g_tree_root_find_version (tree, tree->lowest_version);
+  rm = 0;
+
+  /* mark everything we want to keep with lower versions as belonging to
+     the tree's lowest version */
+  //g_tree_mark_version (rv->root, tree->lowest_version);
 
-  next = g_tree_node_delete_version (tree, rv->root, rnext, version);
-  if (rv->version == version)
+  /* delete older stuff */
+  i = tree->nroots > 1 ? 1 : 0;
+  next = next_version (i, tree->nroots);
+  while (i < tree->nroots &&
+         tree->roots[i].version <= tree->lowest_version)
     {
-      if (next > 0 && next < rnext)
-        {
-          /* the smallest version in the tree is now 'next' */
-          rv->version = next;
-        }
+      guint nextv, v;
+
+      if (next == tree->nroots ||
+          tree->roots[next].version > tree->lowest_version)
+        nextv = tree->lowest_version;
+      else if (next < tree->nroots &&
+               tree->roots[next].root == tree->roots[i].root)
+        nextv = tree->roots[next].version;
       else
-        {
-          rv->root = NULL;
-        }
-    }
+        nextv = 0;
 
-  guint i = rv - tree->roots;
+      if (next < tree->nroots && tree->roots[next].version <= version)
+        v = tree->roots[next].version - 1;
+      else
+        v = version;
 
-  /* we're going to insert a NULL root before 'i'. in this case it should
-     be after 'i'. */
-  if (rv->version < version)
-    i = next_version (i, tree->nroots);
+      l = g_tree_node_delete_versions (tree, tree->root(i), nextv, v);
+      g_assert (l == 0 || l > v);
 
-  /* find the furthest consecutive NULL root node strictly before 'i' */
-  guint j;
-  guint numnulls = 0;
-  guint pi = tree->nroots;
-  for (j = previous_version (i, tree->nroots);
-       j != tree->nroots && tree->roots[j].root == NULL;
-       j = previous_version (j, tree->nroots))
-    {
-      pi = j;
-      ++numnulls;
-    }
+      if (next == tree->nroots ||
+          tree->roots[next].version > tree->lowest_version)
+        tree->roots[i].version = l;
+      else
+        ++rm;
 
-  /* find the first non-NULL root node at or after 'i' */
-  guint ni = i;
-  for (j = next_version (i, tree->nroots);
-       j != tree->nroots && tree->roots[ni].root == NULL;
-       j = next_version (j, tree->nroots))
-    {
-      ni = j;
-      ++numnulls;
+      keep = i;
+      i = next;
+      next = next_version (i, tree->nroots);
     }
 
-  if (!numnulls)
+  if (rm)
     {
-      /* gotta insert before 'i' */
       guint j;
-
-      tree->nroots++;
-      tree->roots = g_renew(GTreeRootVersion, tree->roots, tree->nroots);
-
-      if (i == 0)
-        tree->roots[tree->nroots-1] = tree->roots[i];
-      else
+      for (j = tree->nroots-rm > 1 ? 1 : 0;
+           j < tree->nroots-rm;
+           j = next_version (j, tree->nroots-rm),
+             keep = next_version (keep, tree->nroots))
         {
-          for (j = tree->nroots-1; j > i; --j)
-            tree->roots[j] = tree->roots[j-1];
+          tree->roots[j] = tree->roots[keep];
         }
-      tree->roots[i].version = version;
-      tree->roots[i].root = NULL;
     }
-  else
-    {
-      /* there are some NULL roots we can use already */
-      guint j;
 
-      /* there was no NULL roots before 'rv', so 'rv' must be a NULL root */
-      if (pi == tree->nroots)
-        pi = i;
-
-      /* now 'rv' must be NULL so we'll leave it, and kill the following
-         consecutive NULL roots */
-      j = next_version (pi, tree->nroots);
-      while (ni != tree->nroots)
-        {
-          tree->roots[j] = tree->roots[ni];
-          j = next_version (j, tree->nroots);
-          ni = next_version (ni, tree->nroots);
-        }
-
-      tree->nroots -= numnulls-1;
-      tree->roots = g_renew(GTreeRootVersion, tree->roots, tree->nroots);
-    }
+  tree->nroots -= rm;
+  tree->roots = g_renew(GTreeRootVersion, tree->roots, tree->nroots);
 
 #ifdef G_TREE_DEBUG
   {
     guint i;
-    for (i = 0; i < tree->roots[0].version; ++i)
+    for (i = 0; i <= tree->version; ++i)
       g_tree_node_check (g_tree_root_find_version (tree, i)->root, i);
   }
 #endif
@@ -952,7 +930,7 @@ g_tree_insert (GTree    *tree,
 #ifdef G_TREE_DEBUG
   {
     guint i;
-    for (i = 0; i < tree->roots[0].version; ++i)
+    for (i = 0; i <= tree->version; ++i)
       g_tree_node_check (g_tree_root_find_version (tree, i)->root, i);
   }
 #endif
@@ -986,7 +964,7 @@ g_tree_replace (GTree    *tree,
 #ifdef G_TREE_DEBUG
   {
     guint i;
-    for (i = 0; i < tree->roots[0].version; ++i)
+    for (i = 0; i <= tree->version; ++i)
       g_tree_node_check (g_tree_root_find_version (tree, i)->root, i);
   }
 #endif
@@ -1164,7 +1142,7 @@ g_tree_remove (GTree         *tree,
 #ifdef G_TREE_DEBUG
   {
     guint i;
-    for (i = 0; i < tree->roots[0].version; ++i)
+    for (i = 0; i <= tree->version; ++i)
       g_tree_node_check (g_tree_root_find_version (tree, i)->root, i);
   }
 #endif
@@ -1200,7 +1178,7 @@ g_tree_steal (GTree         *tree,
 #ifdef G_TREE_DEBUG
   {
     guint i;
-    for (i = 0; i < tree->roots[0].version; ++i)
+    for (i = 0; i <= tree->version; ++i)
       g_tree_node_check (g_tree_root_find_version (tree, i)->root, i);
   }
 #endif
@@ -1260,6 +1238,9 @@ g_tree_remove_internal (GTree         *tree,
       /* we're changing this node, make sure our pointer will stay valid
          when we rotate it */
       node = g_tree_node_next_version (tree, node);
+      /* getting the next version for the node may change where its parent
+         lies, so get a new pointer for it, from the node */
+      parent = node->parent(NOW);
 
       if (!node->left(NOW) ||
           (node->right(NOW) &&
@@ -1325,9 +1306,16 @@ g_tree_remove_internal (GTree         *tree,
 
   tree->nnodes--;
 
-  /* only really delete the node if it's in the current version, otherwise
-     it needs to be remembered */
-  if (node->nv == 1 && node->version(NOW) == tree->version)
+  if (node->version(NOW) == tree->version)
+    {
+      /* remove the entry from the node's pointer table */
+      node->v[0] = node->v[node->nv-1];
+      --node->nv;
+    }
+
+  /* only really delete the node if it was only in the current version,
+     otherwise it needs to be remembered */
+  if (node->nv == 0)
     {
       /* free the node's data if it's the last node which is using it. just
          because the version of the node was created in the latest version 
@@ -1349,9 +1337,11 @@ g_tree_remove_internal (GTree         *tree,
       g_slice_free1 (sizeof(GTreeNodeVersion) *
                      (node->version(NOW) == 0 ? 1 : TABLE_SIZE),
                      node->v);
+      node->v = NULL;
+      node->data = NULL;
       g_slice_free (GTreeNode, node);
     }
-  else
+  else if (steal)
     node->data->stolen = TRUE;
 
   return key_removed;

diff --git a/glib/gtree.h b/glib/gtree.h
index b723aebb8f7a0530c44ee3149e8a351595665eee..5b1a0f30531f9292050d9768a002f710330ea745 100644 (file)
--- a/glib/gtree.h
+++ b/glib/gtree.h
@@ -63,7 +63,7 @@ void     g_tree_unref           (GTree            *tree);
 void     g_tree_destroy         (GTree            *tree);
 guint    g_tree_current_version (GTree            *tree);
 guint    g_tree_next_version    (GTree            *tree);
-void     g_tree_delete_version  (GTree            *tree,
+void     g_tree_delete_versions (GTree            *tree,
                                  guint             version);
 void     g_tree_insert          (GTree            *tree,
                                  gpointer          key,

diff --git a/tests/tree-test.c b/tests/tree-test.c
index 20732598db1b2933106673b2df3810b9de1cd93f..70041ac0f98f0e1b95c87ef5237b85af6cf663e0 100644 (file)
--- a/tests/tree-test.c
+++ b/tests/tree-test.c
@@ -31,6 +31,7 @@
 #include <string.h>
 #include "glib.h"
 
+char wada[123813];
 
 static gint
 my_compare (gconstpointer a,
@@ -122,7 +123,7 @@ my_traverse (gpointer key,
 char chars[] = 
   "0123456789"
   "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
-  "abcdefghijklmnopqrstuvwxyz";
+  "abcdefghijklmnopqrstuvwxyz\0";
 
 char chars2[] = 
   "0123456789"
@@ -363,19 +364,19 @@ main (int   argc,
         }
     }
 
-  g_tree_destroy (tree);
+  g_tree_unref (tree);
 
   /* test removing versions from a tree */
   tree = g_tree_new (my_compare);
 
-  for (i = 0; i < 10; i++) {
+  for (i = 0; i < chars[i]; i++) {
     g_tree_insert (tree, &chars[i], &chars[i]);
     g_tree_next_version (tree);
   }
   g_assert(i == g_tree_nnodes(tree));
 
   /* remove everything from the last version */
-  for (i = 0; i < 10; i++)
+  for (i = 0; chars[i]; i++)
     g_tree_remove (tree, &chars[i]);
   g_assert(g_tree_nnodes(tree) == 0);
 
@@ -386,7 +387,7 @@ main (int   argc,
       char *r;
       int j;
 
-      for (j = 0; j < 10; j++)
+      for (j = 0; chars[j]; j++)
         {
           r = g_tree_lookup_related (tree, &chars[j],
                                      G_TREE_SEARCH_PREDECESSOR, i);
@@ -394,14 +395,29 @@ main (int   argc,
         }
     }
 
-  for (i = 0; i < 5; i++)
-    if (i%2==0)
-      g_tree_delete_version (tree, i);
-  
-  for (i = 0; i < 8; i++)
-    if (i%2!=0)
-      g_tree_delete_version (tree, i);
+  for (i = 0; i < g_tree_current_version (tree); ++i)
+    {
+      guint k;
+      if (i%2==0)
+        g_tree_delete_versions (tree, i);
+
+      for (k = 0; k < g_tree_current_version (tree); ++k)
+        {
+          char *r;
+          int j;
 
+          for (j = 0; chars[j]; j++)
+            {
+              r = g_tree_lookup_related (tree, &chars[j],
+                                         G_TREE_SEARCH_PREDECESSOR, k);
+              g_assert ((k <= i/2*2 && r == NULL) ||
+                        (k > i/2*2 && r && *r == chars[MIN(j,k)]));
+            }
+        }
+    }
+
+  g_tree_delete_versions (tree, g_tree_current_version (tree)-1);
+  
   for (i = 0; i < g_tree_current_version (tree); ++i)
     {
       char *r;
@@ -411,7 +427,7 @@ main (int   argc,
         {
           r = g_tree_lookup_related (tree, &chars[j],
                                      G_TREE_SEARCH_PREDECESSOR, i);
-          //g_assert (r == NULL);
+          g_assert (r == NULL);
         }
     }
 
@@ -452,6 +468,8 @@ main (int   argc,
   g_assert (destroyed_key == NULL);
   g_assert (destroyed_value == NULL);
 
+  g_tree_unref (tree);
+
   return 0;
 }