- Fixed rb-tree violations

include/fennec/containers/sequence.h

@@ -41,23 +41,11 @@
 
 // https://en.wikipedia.org/wiki/Red%E2%80%93black_tree
 // https://www.geeksforgeeks.org/dsa/insertion-in-red-black-tree/
+// https://github.com/anandarao/Red-Black-Tree/blob/master/RBTree.cpp
 
-// Uncertain how I managed to do this, but this data structure has
+// After rewriting the _fix_insert and _fix_erase functions the performance decreased significantly in the lower end
-// A 33%-100% performance increase over std::set when running Dijkstra's
+// but now in the higher end it remains consistent. Something I was doing was disturbing both the rb-tree and bst tree
-//
+// properties, now that is fixed. I'll see about optimizing more in the future.
-// Guesses:
-// -> I likely make some assumptions that std::set doesn't
-// -> Cache locality
-// -> Simplified rotation and coloring logic
-//
-// Some of the implementations I have seen have multiple levels
-// of if statements based on directionality which causes branching.
-// I use const-expressions that reduce down to cmov instructions
-//
-//
-// I ran some more performance tests, and it does not hold up well at the larger end of things.
-// I think something with the balancing on insertion is a tad messed up.
-//
 
 namespace fennec
 {
@@ -376,19 +364,12 @@ protected:
 	// Then we just need to handle splitting a 4-node
 	constexpr void _fix_insert(size_t n) {
 		size_t p = parent(n);
-		while (color(p) != black) {
+		while (n != _root && color(n) == red && color(p) == red) {
 			size_t g = parent(p);
-			size_t u = sibling(p);
+			bool   d = n == right(p);
-			size_t d = direction(n);
+			bool   r = p == right(g);
-			size_t r = direction(p);
+			size_t u = child(g, !r);
 
-			// Case 4
-			if (g == npos) {
-				_color(p) = black;
-				return;
-			}
-
-			// Split 4 node
 			if (color(u) == red) {
 				_recolor(g);
 				n = g;
@@ -396,16 +377,18 @@ protected:
 				continue;
 			}
 
-			// LR & RL case
 			if (d != r) {
 				rotate(p, r);
+				n = p;
+				p = parent(n);
 			}
 
-			// LL & RR case
 			rotate(g, not r);
-			n = grandparent(n);
+			fennec::swap(_color(p), _color(g));
+			n = p;
 			p = parent(n);
 		}
+		_color(_root) = black;
 	}
 
 	constexpr void _transplant(size_t u, size_t v) {
@@ -443,8 +426,8 @@ protected:
 	}
 
 	constexpr size_t _red_child(size_t x) {
-		size_t l = left(x);
+		size_t l = _left(x);
-		size_t r = right(x);
+		size_t r = _right(x);
 
 		if (color(l) == red) {
 			return l;
@@ -457,96 +440,89 @@ protected:
 		return npos;
 	}
 
-	// This is an implementation based on the C code in
-	// the wikipedia article adapted to this framework
 	constexpr void _fix_erase(size_t n) {
+		if (n == npos) {
+			return;
+		}
+
+		if (n == _root) {
+			_root = npos;
+			return;
+		}
+
+		size_t o = n;
 		size_t p = parent(n);
-		size_t s, sc, sf;
+		if (p == npos) {
+			_root = npos;
+			return;
+		}
+
 		bool   d = n == right(p);
+		size_t c = _red_child(n);
+		size_t s = npos;
+		if (_color(n) == red || c != npos) {
+			_child(p, d) = c;
+			if (c != npos) {
+				_parent(c) = p;
+			}
+			_color(c) = black;
+			return;
+		}
 
-		_child(p, d) = npos;
+		while (n != _root) {
+			p = _parent(n);
+			d = n == _right(p);
+			s = _child(p, !d);
 
-		goto start_balance;
+			if (s == npos) {
+				break;
+			}
 
-		do {
+			if (_color(s) == red) {
-			d = n == right(p);
-	start_balance:
-			s  = child(p, !d);
-			sf = child(s, !d);
-			sc = child(s, d);
-
-			if (color(s) == red) {
-				// Case 3
-				rotate(p, d);
-				_color(p) = red;
 				_color(s) = black;
-
+				_color(p) = red;
-				// Fix pointers
+				rotate(p, d);
-				s = sc;
+				continue;
-				sf = child(s, !d);
-				sc = child(s, d);
-
-				if (color(sf) == red) {
-					goto case_6;
-				}
-
-				if (color(sc) == red) {
-					goto case_5;
-				}
-
-				// Case 4
-				if (color(p) == red) {
-					if (s != npos) {
-						_color(s) = red;
-					}
-					_color(p) = black;
-					return;
-				}
 			}
 
-			if (color(sf) == red) {
+			size_t nc = _child(s, d);
-				goto case_6;
+			size_t nf = _child(s, !d);
-			}
 
-			if (color(sc) == red) {
+			if (color(nc) == black && color(nf) == black) {
-				goto case_5;
-			}
-
-			// Case 4
-			if (color(p) == red) {
-				if (s != npos) {
-					_color(s) = red;
-				}
-				_color(p) = black;
-				return;
-			}
-
-			// Case 1
-			if (p == npos) {
-				return;
-			}
-
-			// Case 2
-			if (s != npos) {
 				_color(s) = red;
+				if (_color(p) == red) {
+					_color(p) = black;
+					break;
+				}
+				n = p;
+				continue;
 			}
-			n = p;
-		} while ((p = parent(n)) != npos);
 
-		return; //
+			if (color(nf) == black) {
+				_color(nc) = black;
+				_color(s) = red;
+				rotate(s, !d);
+				s  = nc;
+				nf = s;
+			}
+			_color(s) = _color(p);
+			_color(p) = black;
+			_color(nf) = black;
+			rotate(p, d);
+			break;
+		}
 
-	case_5:
+		p = parent(o);
-		rotate(s, !d);
+		if (p != npos) {
-		_color(s) = red;
+			if (o == _left(p)) {
-		_color(sc) = black;
+				_left(p) = npos;
-		sf = s;
+			} else {
-		s = sc;
+				_right(p) = npos;
-
+			}
-	case_6:
+			_color(_root) = black;
-		rotate(p, d);
+		} else {
-		_color(s) = color(p);
+			_root = npos;
-		_color(p) = black;
+		}
-		_color(sf) = black;
 	}
 
 	constexpr void _erase(size_t n) {
@@ -554,19 +530,19 @@ protected:
 			return;
 		}
 
-		size_t l = left(n);
+		size_t l = _left(n);
-		size_t r = right(n);
+		size_t r = _right(n);
 
 		// 2 children
 		if (l != npos && r != npos) {
 			size_t s = left_most(r);
 			_swap_val(n, s);
 			n = s;
-			l = left(n);
+			l = _left(n);
-			r = right(n);
+			r = _right(n);
 		}
 
-		size_t p = parent(n);
+		size_t p = _parent(n);
 		bool   d = n == right(p);
 		size_t c = l != npos ? l : r;
 
@@ -589,7 +565,7 @@ protected:
 		}
 
 		// Single Child, Red, and Root cases
-		if (p == npos || c != npos || color(n) == red) {
+		if (p == npos || c != npos || _color(n) == red) {
 			if (p != npos) {
 				_child(p, d) = c;
 			}