All of lore.kernel.org
 help / color / mirror / Atom feed
From: Praveen Kumar <kpraveen.lkml@gmail.com>
To: xen-devel@lists.xen.org
Cc: sstabellini@kernel.org, wei.liu2@citrix.com,
	George.Dunlap@eu.citrix.com, andrew.cooper3@citrix.com,
	dario.faggioli@citrix.com, ian.jackson@eu.citrix.com,
	tim@xen.org, Praveen Kumar <kpraveen.lkml@gmail.com>,
	jbeulich@suse.com
Subject: [Resend][PATCH 08/17] rbtree: low level optimizations in rb_insert_color()
Date: Thu,  1 Jun 2017 02:50:47 +0530	[thread overview]
Message-ID: <20170531212056.10583-9-kpraveen.lkml@gmail.com> (raw)
In-Reply-To: <20170531212056.10583-1-kpraveen.lkml@gmail.com>

- Use the newly introduced rb_set_parent_color() function to flip the color
  of nodes whose parent is already known.
- Optimize rb_parent() when the node is known to be red - there is no need
  to mask out the color in that case.
- Flipping gparent's color to red requires us to fetch its rb_parent_color
  field, so we can reuse it as the parent value for the next loop iteration.
- Do not use __rb_rotate_left() and __rb_rotate_right() to handle tree
  rotations: we already have pointers to all relevant nodes, and know their
  colors (either because we want to adjust it, or because we've tested it,
  or we can deduce it as black due to the node proximity to a known red node).
  So we can generate more efficient code by making use of the node pointers
  we already have, and setting both the parent and color attributes for
  nodes all at once. Also in Case 2, some node attributes don't have to
  be set because we know another tree rotation (Case 3) will always follow
  and override them.

commit 5bc9188aa207dafd47eab57df7c4fe5b3d3f636a from linux tree

Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com>
---
 xen/common/rbtree.c | 160 ++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 129 insertions(+), 31 deletions(-)

diff --git a/xen/common/rbtree.c b/xen/common/rbtree.c
index ccf905e35c..8db7a5b4ca 100644
--- a/xen/common/rbtree.c
+++ b/xen/common/rbtree.c
@@ -22,6 +22,25 @@
 #include <xen/types.h>
 #include <xen/rbtree.h>
 
+/*
+ * red-black trees properties:  http://en.wikipedia.org/wiki/Rbtree
+ *
+ *  1) A node is either red or black
+ *  2) The root is black
+ *  3) All leaves (NULL) are black
+ *  4) Both children of every red node are black
+ *  5) Every simple path from root to leaves contains the same number
+ *     of black nodes.
+ *
+ *  4 and 5 give the O(log n) guarantee, since 4 implies you cannot have two
+ *  consecutive red nodes in a path and every red node is therefore followed by
+ *  a black. So if B is the number of black nodes on every simple path (as per
+ *  5), then the longest possible path due to 4 is 2B.
+ *
+ *  We shall indicate color with case, where black nodes are uppercase and red
+ *  nodes will be lowercase.
+ */
+
 #define    RB_RED    0
 #define    RB_BLACK  1
 
@@ -40,6 +59,17 @@ static inline void rb_set_color(struct rb_node *rb, int color)
     rb->__rb_parent_color = (rb->__rb_parent_color & ~1) | color;
 }
 
+static inline void rb_set_parent_color(struct rb_node *rb,
+                                      struct rb_node *p, int color)
+{
+    rb->__rb_parent_color = (unsigned long)p | color;
+}
+
+static inline struct rb_node *rb_red_parent(struct rb_node *red)
+{
+    return (struct rb_node *)red->__rb_parent_color;
+}
+
 static void __rb_rotate_left(struct rb_node *node, struct rb_root *root)
 {
     struct rb_node *right = node->rb_right;
@@ -86,9 +116,30 @@ static void __rb_rotate_right(struct rb_node *node, struct rb_root *root)
     rb_set_parent(node, left);
 }
 
+/*
+ * Helper function for rotations:
+ * - old's parent and color get assigned to new
+ * - old gets assigned new as a parent and 'color' as a color.
+ */
+static inline void
+__rb_rotate_set_parents(struct rb_node *old, struct rb_node *new,
+                        struct rb_root *root, int color)
+{
+    struct rb_node *parent = rb_parent(old);
+    new->__rb_parent_color = old->__rb_parent_color;
+    rb_set_parent_color(old, new, color);
+    if (parent) {
+        if (parent->rb_left == old)
+            parent->rb_left = new;
+        else
+            parent->rb_right = new;
+    } else
+        root->rb_node = new;
+}
+
 void rb_insert_color(struct rb_node *node, struct rb_root *root)
 {
-    struct rb_node *parent, *gparent;
+    struct rb_node *parent = rb_red_parent(node), *gparent, *tmp;
 
     while (true)
     {
@@ -99,61 +150,108 @@ void rb_insert_color(struct rb_node *node, struct rb_root *root)
          * Otherwise, take some corrective action as we don't
          * want a red root or two consecutive red nodes.
          */
-        parent = rb_parent(node);
         if (!parent)
         {
-            rb_set_black(node);
+            rb_set_parent_color(node, NULL, RB_BLACK);
             break;
         } else if (rb_is_black(parent))
             break;
 
-        gparent = rb_parent(parent);
+        gparent = rb_red_parent(parent);
 
         if (parent == gparent->rb_left)
         {
+            tmp = gparent->rb_right;
+            if (tmp && rb_is_red(tmp))
             {
-                register struct rb_node *uncle = gparent->rb_right;
-                if (uncle && rb_is_red(uncle))
-                {
-                    rb_set_black(uncle);
-                    rb_set_black(parent);
-                    rb_set_red(gparent);
-                    node = gparent;
-                    continue;
-                }
+                /*
+                 * Case 1 - color flips
+                 *
+                 *       G            g
+                 *      / \          / \
+                 *     p   u  -->   P   U
+                 *    /            /
+                 *   n            N
+                 *
+                 * However, since g's parent might be red, and
+                 * 4) does not allow this, we need to recurse
+                 * at g.
+                 */
+                rb_set_parent_color(tmp, gparent, RB_BLACK);
+                rb_set_parent_color(parent, gparent, RB_BLACK);
+                node = gparent;
+                parent = rb_parent(node);
+                rb_set_parent_color(node, parent, RB_RED);
+                continue;
             }
 
             if (parent->rb_right == node)
             {
-                __rb_rotate_left(parent, root);
+                /*
+                 * Case 2 - left rotate at parent
+                 *
+                 *      G             G
+                 *     / \           / \
+                 *    p   U  -->    n   U
+                 *     \           /
+                 *      n         p
+                 *
+                 * This still leaves us in violation of 4), the
+                 * continuation into Case 3 will fix that.
+                 */
+                parent->rb_right = tmp = node->rb_left;
+                node->rb_left = parent;
+                if (tmp)
+                    rb_set_parent_color(tmp, parent, RB_BLACK);
+                rb_set_parent_color(parent, node, RB_RED);
                 parent = node;
             }
-
-            rb_set_black(parent);
-            rb_set_red(gparent);
-            __rb_rotate_right(gparent, root);
+            /*
+             * Case 3 - right rotate at gparent
+             *
+             *        G           P
+             *       / \         / \
+             *      p   U  -->  n   g
+             *     /                 \
+             *    n                   U
+             */
+            gparent->rb_left = tmp = parent->rb_right;
+            parent->rb_right = gparent;
+            if (tmp)
+                rb_set_parent_color(tmp, gparent, RB_BLACK);
+            __rb_rotate_set_parents(gparent, parent, root, RB_RED);
+            break;
         } else {
+            tmp = gparent->rb_left;
+            if (tmp && rb_is_red(tmp))
             {
-                register struct rb_node *uncle = gparent->rb_left;
-                if (uncle && rb_is_red(uncle))
-                {
-                    rb_set_black(uncle);
-                    rb_set_black(parent);
-                    rb_set_red(gparent);
-                    node = gparent;
-                    continue;
-                }
+                /* Case 1 - color flips */
+                rb_set_parent_color(tmp, gparent, RB_BLACK);
+                rb_set_parent_color(parent, gparent, RB_BLACK);
+                node = gparent;
+                parent = rb_parent(node);
+                rb_set_parent_color(node, parent, RB_RED);
+                continue;
             }
 
             if (parent->rb_left == node)
             {
-                __rb_rotate_right(parent, root);
+                /* Case 2 - right rotate at parent */
+                parent->rb_left = tmp = node->rb_right;
+                node->rb_right = parent;
+                if (tmp)
+                    rb_set_parent_color(tmp, parent, RB_BLACK);
+                rb_set_parent_color(parent, node, RB_RED);
                 parent = node;
             }
 
-            rb_set_black(parent);
-            rb_set_red(gparent);
-            __rb_rotate_left(gparent, root);
+            /* Case 3 - left rotate at gparent */
+            gparent->rb_right = tmp = parent->rb_left;
+            parent->rb_left = gparent;
+            if (tmp)
+                rb_set_parent_color(tmp, gparent, RB_BLACK);
+            __rb_rotate_set_parents(gparent, parent, root, RB_RED);
+            break;
         }
     }
 }
-- 
2.12.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

  parent reply	other threads:[~2017-05-31 21:20 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-31 21:20 [Xen-devel[PATCH Resend v2] xen: common: rbtree: ported updates from linux tree Praveen Kumar
2017-05-31 21:20 ` [Resend][PATCH 01/17] rb_tree: reorganize code in rb_erase() for additional changes Praveen Kumar
2017-05-31 22:40   ` Dario Faggioli
2017-05-31 22:56     ` Andrew Cooper
2017-06-01  8:01       ` Dario Faggioli
2017-06-02 16:28         ` Praveen Kumar
2017-06-08 15:57   ` Jan Beulich
2017-05-31 21:20 ` [Resend][PATCH 02/17] rb_tree: make clear distinction between two different cases in rb_erase() Praveen Kumar
2017-05-31 21:20 ` [Resend][PATCH 03/17] rb_tree: remove redundant if()-condition " Praveen Kumar
2017-05-31 21:20 ` [Resend][PATCH 04/17] rbtree: empty nodes have no color Praveen Kumar
2017-06-12 16:42   ` Dario Faggioli
2017-06-13  8:21     ` Jan Beulich
2017-05-31 21:20 ` [Resend][PATCH 05/17] rbtree: move some implementation details from rbtree.h to rbtree.c Praveen Kumar
2017-05-31 21:20 ` [Resend][PATCH 06/17] rbtree: break out of rb_insert_color loop after tree rotation Praveen Kumar
2017-06-12 16:41   ` Dario Faggioli
2017-05-31 21:20 ` [Resend][PATCH 07/17] rbtree: adjust root color in rb_insert_color() only when necessary Praveen Kumar
2017-05-31 21:20 ` Praveen Kumar [this message]
2017-05-31 21:20 ` [Resend][PATCH 09/17] rbtree: adjust node color in __rb_erase_color() " Praveen Kumar
2017-05-31 21:20 ` [Resend][PATCH 10/17] rbtree: optimize case selection logic in __rb_erase_color() Praveen Kumar
2017-05-31 21:20 ` [Resend][PATCH 11/17] rbtree: low level optimizations " Praveen Kumar
2017-05-31 21:20 ` [Resend][PATCH 12/17] rbtree: optimize fetching of sibling node Praveen Kumar
2017-05-31 21:20 ` [Resend][PATCH 13/17] rbtree: add __rb_change_child() helper function Praveen Kumar
2017-06-12 16:14   ` Dario Faggioli
2017-05-31 21:20 ` [Resend][PATCH 14/17] rbtree: place easiest case first in rb_erase() Praveen Kumar
2017-06-12 16:19   ` Dario Faggioli
2017-05-31 21:20 ` [Resend][PATCH 15/17] rbtree: handle 1-child recoloring in rb_erase() instead of rb_erase_color() Praveen Kumar
2017-05-31 21:20 ` [Resend][PATCH 16/17] rbtree: low level optimizations in rb_erase() Praveen Kumar
2017-05-31 21:20 ` [Resend][PATCH 17/17] rbtree: add postorder iteration functions Praveen Kumar
2017-05-31 22:34 ` [Xen-devel[PATCH Resend v2] xen: common: rbtree: ported updates from linux tree Dario Faggioli
2017-06-01  7:26 ` Jan Beulich
2017-06-01  7:43   ` Dario Faggioli
2017-06-02 16:35     ` Praveen Kumar
2017-06-02 16:31   ` Praveen Kumar
2017-06-12 16:44 ` Dario Faggioli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170531212056.10583-9-kpraveen.lkml@gmail.com \
    --to=kpraveen.lkml@gmail.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=dario.faggioli@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.