From c89c846329ec834e49b38e0e104b90dfd19bbb58 Mon Sep 17 00:00:00 2001
From: Christopher Haster <geky@geky.net>
Date: Sat, 25 Jan 2025 14:25:36 -0600
Subject: [PATCH] rbyd: Potential y-prune disambiguity with more jump >= branch
 trickery?

Idea!

The problem isn't so much that y-pruning recolors and diverged-pruning
doesn't recolor, but that we are unconditionally recoloring based on
the assumed yellow node direction.

Consider all possible yellow prune permutations:

          <y                      <r
  .-------'|              .-------'|
  |       <r  prune y     |        |
  |  .----'|    =>        |     .--'
  |  |    <b              |    <b
  |  |  .-'|              |  .-'|
  1  2  3  4           1  2  3  4  x

                                  <r
                       .----------'|
              prune r  |           |
                =>     |        .--'
                       |       <b
                       |     .-'|
                       1  2  3  4  x

                                  <b
                       .----------'|
              prune b  |           |
                =>     |  .--------'
                       |  |
                       |  |
                       1  2  3  4  x

Note these all maintain the color balance of our tree, but we only need
to recolor the yellow alt when it's the black alt we're pruning.

But how do we know if it's the black alt we're pruning? Well, we can use
the same trick we used in unflipping y-splits. The black alt is the only
alt where jump >= the current branch.

This is true because we always split yellow nodes as soon as we see
them.

---

This idea is promising, but needs a bit of work.

So, uh, please ignore the additional test failures:

  test_rbyd+balance before: 312/385878 failed
  test_rbyd+balance after:  300/385878 failed (-3.8%)

Curiously, these are now actual test failures and not just balance
failures. I can't decide if that's a good sign or not.

Code changes:

           code          stack          ctx
  before: 38860           2624          640
  after:  38880 (+0.1%)   2624 (+0.0%)  640 (+0.0%)
---
 lfs.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/lfs.c b/lfs.c
index cdb1a5f7..5a853d32 100644
--- a/lfs.c
+++ b/lfs.c
@@ -3504,6 +3504,11 @@ static int lfsr_rbyd_appendrat(lfs_t *lfs, lfsr_rbyd_t *rbyd,
     a_tag = lfs_max(a_tag, 0x1);
     b_tag = lfs_max(b_tag, 0x1);
 
+    LFS_DEBUG("%04x: rbyd append %d %04x %d %04x",
+            lfsr_rbyd_eoff(rbyd),
+            a_rid, a_tag,
+            b_rid, b_tag);
+
     // keep track of diverged state
     //
     // this is only used if we operate on a range of tags, in which case
@@ -3635,7 +3640,8 @@ trunk:;
                 if (lfsr_tag_unreachable(
                             p[0].alt, p[0].weight,
                             lower_rid, upper_rid,
-                            lower_tag, upper_tag)) {
+                            lower_tag, upper_tag)
+                        && p[0].jump >= branch_) {
                     LFS_DEBUG("%04x->%04x: yprune",
                             branch, lfsr_rbyd_eoff(rbyd));
                     alt &= ~LFSR_TAG_R;
@@ -3652,10 +3658,11 @@ trunk:;
                 // |  |  .----'|      |     .----'|  |
                 // 1  2  3  4  4      1  2  3  4  4  2
                 } else if (lfsr_tag_unreachable2(
-                        alt, weight,
-                        p[0].alt, p[0].weight,
-                        lower_rid, upper_rid,
-                        lower_tag, upper_tag)) {
+                            alt, weight,
+                            p[0].alt, p[0].weight,
+                            lower_rid, upper_rid,
+                            lower_tag, upper_tag)
+                        && jump >= branch_) {
                     LFS_DEBUG("%04x->%04x: rprune",
                             branch, lfsr_rbyd_eoff(rbyd));
                     alt = p[0].alt & ~LFSR_TAG_R;
@@ -3878,8 +3885,9 @@ trunk:;
                             lower_rid, upper_rid,
                             a_rid, a_tag,
                             b_rid, b_tag);
+                // TODO I think this logic is wrong, what's correct here?
                 if (diverging_red) {
-                    LFS_DEBUG("%04x->%04x: div r pruning",
+                    LFS_DEBUG("%04x->%04x: div r trimming",
                             branch, lfsr_rbyd_eoff(rbyd));
                     // trim so alt is pruned
                     lfsr_tag_trim(
@@ -3902,7 +3910,7 @@ trunk:;
                 if (diverging
                         && (!lfsr_tag_isred(alt)
                             || lfsr_tag_isred(p[0].alt))) {
-                    LFS_DEBUG("%04x->%04x: div trimming",
+                    LFS_DEBUG("%04x->%04x: div b trimming",
                             branch, lfsr_rbyd_eoff(rbyd));
                     // trim so alt is pruned
                     lfsr_tag_trim(
@@ -4000,7 +4008,7 @@ trunk:;
                 // |  |    <b         |    <b  |
                 // |  |  .-'|         |  .-'|  |
                 // 1  2  3  4      1  2  3  4  1
-                if (branch_ <= branch) {
+                if (branch_ < branch) {
                     LFS_DEBUG("%04x->%04x: ysplit b",
                             branch, lfsr_rbyd_eoff(rbyd));
                     // TODO hwat, >= solves this??