[PATCH 2/2] Optimization of function rb_erase() in lib/rbtree.c

From: Wolfram Strepp
Date: Tue Jan 20 2009 - 16:58:52 EST


This patch reorganizes some lines, and therefore one if-condition
can be omitted. Here an explanation.

There are two cases when a node is erased:
'normal case': the successor is not the right-hand-child of the node to be erased
'special case': the successor is the right-hand child of the node to be erased

In the normal case, the pointers from the nodes to its parents and vice-versa
have all to be reconnected to its new parents/childs.
The current code basically ignores this special case, doing the reconnections
always in the way as if it were the normal case. This is ok, but not optimal.
The patch re-arranges the lines, such that it makes use of one particular
feature of the special case: when the node to be deleted is replaced by its successor,
it pulls it child behind - so no reconnection is necessary. As a result, the
if-condition, which checks if this child is a NIL-leave, can be omitted.

Here some ascii-art, with following symbols (referring to the code):
O: node to be deleted
N: the successor of O
P: parent of N
C: child of N
L: some other node

normal case:

O N
/ \ / \
/ \ / \
L \ L \
/ \ P ----> / \ P
/ \ / \
/ /
N C
\ / \
\
C
/ \


special case:
O|P N
/ \ / \
/ \ / \
L \ L \
/ \ N ----> / C
\ / \
\
C
/ \


Signed-off-by: Wolfram Strepp <wstrepp@xxxxxx>

======================================

--- a/lib/rbtree.c
+++ b/lib/rbtree.c
@@ -231,22 +231,13 @@ void rb_erase(struct rb_node *node, struct rb_root *root)
node = node->rb_right;
while ((left = node->rb_left) != NULL)
node = left;
+
child = node->rb_right;
parent = rb_parent(node);
color = rb_color(node);

- if (child)
- rb_set_parent(child, parent);
- if (parent == old) {
- parent->rb_right = child;
- parent = node;
- } else
- parent->rb_left = child;
-
+ /* connect 'node' with its new parent */
node->rb_parent_color = old->rb_parent_color;
- node->rb_right = old->rb_right;
- node->rb_left = old->rb_left;
-
if (rb_parent(old))
{
if (rb_parent(old)->rb_left == old)
@@ -256,9 +247,26 @@ void rb_erase(struct rb_node *node, struct rb_root *root)
} else
root->rb_node = node;

- rb_set_parent(old->rb_left, node);
- if (old->rb_right)
+
+ if (parent == old) {
+ /* special case: 'node' becomes parent, pulling its
+ child up behind --> no re-connecting necessary ! */
+ parent = node;
+ } else {
+ /* replace 'node' with its former child */
+ parent->rb_left = child;
+ if (child)
+ rb_set_parent(child, parent);
+
+ /* connect 'node' to its new right-hand child */
+ node->rb_right = old->rb_right;
rb_set_parent(old->rb_right, node);
+ }
+
+ /* connect 'node' to its new left-hand child */
+ node->rb_left = old->rb_left;
+ rb_set_parent(old->rb_left, node);
+
goto color;
}


--
Psssst! Schon vom neuen GMX MultiMessenger gehört? Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/