Skip to content

Commit

Permalink
AVL: Remove obsolete branching optimizations
Browse files Browse the repository at this point in the history
Modern Clang and GCC can successfully implement simple conditions
without branching with math and flag operations.  Use of arrays for
translation no longer helps as much as it was 14+ years ago.

Disassemble of the code generated by Clang 13.0.0 on FreeBSD 13.1,
Clang 14.0.4 on FreeBSD 14 and GCC 10.2.1 on Debian 11 with this
change still shows no branching instructions.

Profiling of CPU-bound scan stage of sorted scrub shows reproducible
reduction of time spent inside avl_find() from 6.52% to 4.58%.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored-By: iXsystems, Inc.
Closes openzfs#13540
  • Loading branch information
amotin authored and andrewc12 committed Sep 23, 2022
1 parent a672b91 commit 74da627
Showing 1 changed file with 4 additions and 20 deletions.
24 changes: 4 additions & 20 deletions module/avl/avl.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,21 +108,6 @@
#include <sys/cmn_err.h>
#include <sys/mod.h>

/*
* Small arrays to translate between balance (or diff) values and child indices.
*
* Code that deals with binary tree data structures will randomly use
* left and right children when examining a tree. C "if()" statements
* which evaluate randomly suffer from very poor hardware branch prediction.
* In this code we avoid some of the branch mispredictions by using the
* following translation arrays. They replace random branches with an
* additional memory reference. Since the translation arrays are both very
* small the data should remain efficiently in cache.
*/
static const int avl_child2balance[] = {-1, 1};
static const int avl_balance2child[] = {0, 0, 1};


/*
* Walk from one node to the previous valued node (ie. an infix walk
* towards the left). At any given node we do one of 2 things:
Expand Down Expand Up @@ -278,8 +263,7 @@ avl_find(avl_tree_t *tree, const void *value, avl_index_t *where)
#endif
return (AVL_NODE2DATA(node, off));
}
child = avl_balance2child[1 + diff];

child = (diff > 0);
}

if (where != NULL)
Expand Down Expand Up @@ -527,7 +511,7 @@ avl_insert(avl_tree_t *tree, void *new_data, avl_index_t where)
* Compute the new balance
*/
old_balance = AVL_XBALANCE(node);
new_balance = old_balance + avl_child2balance[which_child];
new_balance = old_balance + (which_child ? 1 : -1);

/*
* If we introduced equal balance, then we are done immediately
Expand Down Expand Up @@ -693,7 +677,7 @@ avl_remove(avl_tree_t *tree, void *data)
* choose node to swap from whichever side is taller
*/
old_balance = AVL_XBALANCE(delete);
left = avl_balance2child[old_balance + 1];
left = (old_balance > 0);
right = 1 - left;

/*
Expand Down Expand Up @@ -777,7 +761,7 @@ avl_remove(avl_tree_t *tree, void *data)
*/
node = parent;
old_balance = AVL_XBALANCE(node);
new_balance = old_balance - avl_child2balance[which_child];
new_balance = old_balance - (which_child ? 1 : -1);
parent = AVL_XPARENT(node);
which_child = AVL_XCHILD(node);

Expand Down

0 comments on commit 74da627

Please sign in to comment.