Force an arbitrary order on otherwise identical items.

2007-02-07  Soren Sandmann <sandmann@daimi.au.dk>

	* tests/sequence-test.c (compare_items): Force an arbitrary order
	on otherwise identical items. 

	* glib/gsequence.c: Add comment discussing splay trees vs. other trees. 
	* glib/gsequence.c (is_end): Add fast path for the common case
	when the node is not actually the end node.


svn path=/trunk/; revision=5328

ChangeLog

@@ -1,3 +1,12 @@
+2007-02-07  Soren Sandmann <sandmann@daimi.au.dk>
+
+	* tests/sequence-test.c (compare_items): Force an arbitrary order
+	on otherwise identical items. 
+
+	* glib/gsequence.c: Add comment discussing splay trees vs. other trees. 
+	* glib/gsequence.c (is_end): Add fast path for the common case
+	when the node is not actually the end node.
+
 2007-02-05  Soren Sandmann <sandmann@daimi.au.dk>
 
 	* glib/gsequence.c (g_sequence_sort_iter): Don't prohibit access

glib/gsequence.c

@@ -32,7 +32,7 @@ struct _GSequence
   gboolean              access_prohibited;
 
   /* The 'real_sequence' is used when temporary sequences are created
-   * to hold nodes that being rearranged. The 'real_sequence' of such
+   * to hold nodes that are being rearranged. The 'real_sequence' of such
    * a temporary sequence points to the sequence that is actually being
    * manipulated. The only reason we need this is so that when the
    * sort/sort_changed/search_iter() functions call out to the application
@@ -111,8 +111,16 @@ check_iter_access (GSequenceIter *iter)
 static gboolean
 is_end (GSequenceIter *iter)
 {
-  GSequence *seq = get_sequence (iter);
+  GSequence *seq;
+
+  if (iter->right)
+    return FALSE;
+
+  if (iter->parent && iter->parent->right != iter)
+    return FALSE;
   
+  seq = get_sequence (iter);
+
   return seq->end_node == iter;
 }
 
@@ -1289,6 +1297,49 @@ g_sequence_swap (GSequenceIter *a,
 /*
  * Implementation of the splay tree. 
  */
+
+/* Splay Tree vs. Other Kinds of Trees
+ *
+ * There are both advantages and disadvantages to using a splay tree vs. some other
+ * kind of tree such as a red/black tree or a btree.
+ *
+ * Advantages of splay trees
+ *
+ * - They are very simple to implement, especially things like move_range() or concatenate()
+ *   are very easy to do for splay trees. The algorithm to split a red/black tree, while still,
+ *   O(log n) is much more involved.
+ *
+ * - If we add aggregates at one point, splay trees make it really easy to compute the aggregate
+ *   for an arbitrary range of the tree. In a red/black tree you would have to pick out the correct
+ *   subtrees, then call out to the aggregator function to compute them.
+ *      On the other hand, for a splay tree, aggregates would be invalidated on lookups, so you
+ *   would call the aggregator much more often. In both cases, the aggregator function would be
+ *   called O(log n) times as a side-effect of asking for the aggregate of a range.
+ *
+ * - If you are only using the list API and never the insert_sorted(), the operations on a
+ *   splay tree will actually be O(1) rather than O(log n). But this is most likely one
+ *   for the "who cares" department, since the O(log n) of a red/black tree really is quite
+ *   fast and if what you need is a queue you can just use GQueue.
+ *
+ * The disadvantages
+ *
+ * - Splay trees are only amortized O(log n) which means individual operations could take a long
+ *   time, which is undesirable in GUI applications
+ *
+ * - Red/black trees are mode widely known since they are tought in CS101 courses.
+ *
+ * - Red/black trees or btrees are more efficient. In particular, splay trees write to the 
+ *   nodes on lookup, which causes dirty pages that the VM system will have to launder.
+ *
+ * - Splay trees are not necessarily balanced at all which means straight-forward recursive
+ *   algorithms can use lots of stack.
+ *
+ * It is likely worth investigating whether a BTree would be a better choice, in particular the
+ * algorithm to split a BTree may not be all that complicated given that split/join for nodes
+ * will have to be implemented anyway.
+ * 
+ */
+
 static void
 node_update_fields (GSequenceNode *node)
 {

tests/sequence-test.c

@@ -30,13 +30,32 @@ typedef struct SequenceInfo
   int		n_items;
 } SequenceInfo;
 
+typedef struct
+{
+  SequenceInfo *seq;
+  int		  number;
+} Item;
+
 void g_sequence_self_test_internal_to_glib_dont_use (GSequence *sequence);
 
+static Item *
+fix_pointer (gconstpointer data)
+{
+  return (Item *)((char *)data - 1);
+}
+
+static Item *
+get_item (GSequenceIter *iter)
+{
+  return fix_pointer (g_sequence_get (iter));
+}
+
 static void
 check_integrity (SequenceInfo *info)
 {
   GList *list;
   GSequenceIter *iter;
+  int i;
   
   g_sequence_self_test_internal_to_glib_dont_use (info->sequence);
   
@@ -45,27 +64,23 @@ check_integrity (SequenceInfo *info)
 	     g_sequence_get_length (info->sequence), info->n_items);
   g_assert (info->n_items == g_queue_get_length (info->queue));
   g_assert (g_sequence_get_length (info->sequence) == info->n_items);
-  
+
   iter = g_sequence_get_begin_iter (info->sequence);
   list = info->queue->head;
+  i = 0;
   while (iter != g_sequence_get_end_iter (info->sequence))
     {
       g_assert (list->data == iter);
       
       iter = g_sequence_iter_next (iter);
       list = list->next;
+      i++;
     }
   
   g_assert (info->n_items == g_queue_get_length (info->queue));
   g_assert (g_sequence_get_length (info->sequence) == info->n_items);
 }
 
-typedef struct
-{
-  SequenceInfo *seq;
-  int		  number;
-} Item;
-
 static gpointer
 new_item (SequenceInfo *seq)
 {
@@ -81,18 +96,6 @@ new_item (SequenceInfo *seq)
   return ((char *)item + 1);
 }
 
-static Item *
-fix_pointer (gconstpointer data)
-{
-  return (Item *)((char *)data - 1);
-}
-
-static Item *
-get_item (GSequenceIter *iter)
-{
-  return fix_pointer (g_sequence_get (iter));
-}
-
 static void
 free_item (gpointer data)
 {
@@ -129,11 +132,28 @@ compare_items (gconstpointer a,
   const Item *item_b = fix_pointer (b);
 
   if (item_a->number < item_b->number)
-    return -1;
+    {
+      return -1;
+    }
   else if (item_a->number == item_b->number)
-    return 0;
+    {
+      /* Force an arbitrary order on the items
+       * We have to do this, since g_queue_insert_sorted() and
+       * g_sequence_insert_sorted() do not agree on the exact
+       * position the item is inserted if the new item is
+       * equal to an existing one.
+       */
+      if (item_a < item_b)
+	return -1;
+      else if (item_a == item_b)
+	return 0;
+      else
+	return 1;
+    }
   else
-    return 1;
+    {
+      return 1;
+    }
 }
 
 static void
@@ -314,9 +334,9 @@ run_random_tests (guint32 seed)
       int i;
       SequenceInfo *seq = RANDOM_SEQUENCE();
       int op = g_random_int_range (0, N_OPS);
-      
+
 #if 0
-      g_print ("%d\n", op);
+      g_print ("%d on %p\n", op, seq);
 #endif
       
       switch (op)
@@ -378,6 +398,7 @@ run_random_tests (guint32 seed)
 	  break;
 	case SORT_ITER:
 	  {
+	    check_integrity (seq);
 	    g_sequence_sort_iter (seq->sequence,
 				  (GSequenceIterCompareFunc)compare_iters, seq->sequence);
 	    g_queue_sort (seq->queue, compare_iters, NULL);
@@ -978,6 +999,7 @@ run_random_tests (guint32 seed)
  */
 static gulong seeds[] =
   {
+    825541564u,
     801678400u,
     1477639090u,
     3369132895u,