From c370fbec1a3b7a0b02caffa1b8ff27f6a371fc9b Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 5 Apr 2024 13:45:31 -0500 Subject: [PATCH] rbyd-rr: Limping along, fixed test_files_many, all tests are passing now The issue, found in test_files_many:h1g4j10l18, occurs when a SUBWIDE tag follows a compaction. When this happens, it's possible for our stitched diverging alt to be followed/flipped when it shouldn't be. This is because the new lower_rid/upper_rid window can make the stitched alt ambiguous. I don't think this is strictly an issue with compaction, as much as compaction is giving us a tree structure that's not reachable through only appendattrs. Here are the three culprit trunks: altrle 0x300 w8 0x2c8 altble 0x203 w6 0x2d4 <- diverge null altrle 0x300 w8 0x2c8 altbgt 0x203 w0 0x2e8 <- diverge altble 0x300 w4 0x2b4 altbn w0 0x0 altble 0x300 w1 0x228 altbn w0 0x0 null altrle 0x300 w8 0x2c8 altbgt 0x300 w0 0x2e8 <- stitch altbn w0 0x0 altbn w0 0x0 altbn w0 0x0 altbgt 0x201 w0 0x164 reg w1 And here is a simplified view, after compaction, before we do a subwide append/replace: .-> reg w1 a .-------b-> data | .-> reg w1 b | .---b-> data r-b-r-b-b-> orphan w1 <- removed as a part of our subwide op ^ diverging '-+-' weight=3 altrle data w1 altble orphan w1 First, as a part of our subwide append, we're going to write out the lower trunk. We diverge on the first altble since the entire orphan is inside our subwide range. It may seem a bit strange to diverge on a null tag, but this isn't actually an issue, we're allowed a single null tag to terminate our tree: .-> reg w1 a .-------b-> data | .-> reg w1 b r-b-----b-> data ^ diverging '-+-' weight=3 altrle data w1 altbgt data w0 Nothing wrong so far. The weight of our leaves (2) don't match our tree's weight (3), but this is normal for the lower trunk. We fix this when we stitch the diverging alt on the upper trunk. Speaking of the upper trunk, let's start writing it out, but pause at the stitching alt: .-> reg w1 a .-------b-> data | .-> reg w1 b | .-----b-> data r-b-? ^ stitching '-+-' weight=3 altrle data w1 altble data w1 Note we've flipped the altbgt data into an altble data, since we're going down the other diverged path now. But before we continue, as a part of stitching, we need to adjust our tree weight to account for the weight of the orphan we deleted as a part of our range operation: .-> reg w1 a .-------b-> data | .-> reg w1 b | .-----b-> data r-b-? ^ stitching '-+-' weight=2 altrle data w1 altble data w1 Uh oh. Weight is 2 and both our alts add up to 2? All of a sudden it looks like we should follow the stitched alt. Our follow/flip logic kicks in, and disaster! .-> reg w1 a .-------b-> data r-b-----b-> reg w1 c <- added as a part of our subwide op '-> data <- somehow data survives '-+-' but where did b go? weight=2 altrle data w1 altbgt data w0 We go down the wrong path, and because our state machine thinks we've diverged, we prune all le alts, destroying our tree. --- So what's is going wrong? The problem is that when we update our window, the stitched diverging alt can become ambiguous. Which sort of makes sense. The reason we update our window is so we can continue down the tree veiwing it as it was _before_ the range operation. But the stitched alt belongs to the tree _after_ the range operation. The solution here is to just make sure we never follow the stitched alt. This is a bit annoying, as it makes the stitched alt a rather special case, but as far as I can tell it's necessary to avoid ambiguity. --- lfs.c | 97 +++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 61 insertions(+), 36 deletions(-) diff --git a/lfs.c b/lfs.c index f6cde566..87c52c47 100644 --- a/lfs.c +++ b/lfs.c @@ -2980,7 +2980,7 @@ again:; p_alts[0], p_weights[0]); if (d_upper) { - //alt &= ~LFSR_TAG_R; + alt &= ~LFSR_TAG_R; diverged = true; // alt = LFSR_TAG_ALT( @@ -2992,11 +2992,20 @@ again:; // } else { // weight = (upper_rid - lower_rid) - weight; // } + + if (lfsr_tag_follow2( + alt, weight, + p_alts[0], p_weights[0], + lower_rid, upper_rid, + a_rid, a_tag)) { + lfsr_tag_flip2( + &alt, &weight, + p_alts[0], p_weights[0], + lower_rid, upper_rid); + lfs_swap32(&jump, &branch_); + } + if (lfsr_tag_isle(alt)) { - alt = LFSR_TAG_ALT( - LFSR_TAG_LE, - alt & LFSR_TAG_R, - d_tag); printf("%04x->%04x: dle 0x%x %d w%d (%d %d)\n", branch, rbyd->eoff, @@ -3005,43 +3014,55 @@ again:; weight, lower_rid, upper_rid); - lower_rid += weight; - weight = d_rid - lower_rid + weight; - if (lfsr_tag_isred(p_alts[0]) - && lfsr_tag_isle(p_alts[0])) { - weight -= p_weights[0]; - } - lower_rid -= weight; -// jump = d_branch; - } else { - lfsr_tag_flip2( - &alt, &weight, - p_alts[0], p_weights[0], - lower_rid, upper_rid); alt = LFSR_TAG_ALT( LFSR_TAG_LE, alt & LFSR_TAG_R, d_tag); - printf("%04x->%04x: dgt 0x%x %d w%d (%d %d)\n", - branch, - rbyd->eoff, - d_tag, - d_rid, - weight, - lower_rid, - upper_rid); - lower_rid += weight; - weight = d_rid - lower_rid + weight; + lfsr_rid_t weight_ = d_rid - lower_rid; if (lfsr_tag_isred(p_alts[0]) && lfsr_tag_isle(p_alts[0])) { - weight -= p_weights[0]; + weight_ -= p_weights[0]; } - lower_rid -= weight; - lfsr_tag_flip2( - &alt, &weight, - p_alts[0], p_weights[0], - lower_rid, upper_rid); -// branch_ = d_branch; + lower_rid += weight - weight_; + weight = weight_; +// lower_rid += weight; +// weight = d_rid - lower_rid + weight; +// if (lfsr_tag_isred(p_alts[0]) +// && lfsr_tag_isle(p_alts[0])) { +// weight -= p_weights[0]; +// } +// lower_rid -= weight; +// jump = d_branch; + } else { + LFS_UNREACHABLE(); +// printf("%04x->%04x: dgt 0x%x %d w%d (%d %d)\n", +// branch, +// rbyd->eoff, +// d_tag, +// d_rid, +// weight, +// lower_rid, +// upper_rid); +// lfsr_tag_flip2( +// &alt, &weight, +// p_alts[0], p_weights[0], +// lower_rid, upper_rid); +// alt = LFSR_TAG_ALT( +// LFSR_TAG_LE, +// alt & LFSR_TAG_R, +// d_tag); +// lower_rid += weight; +// weight = d_rid - lower_rid + weight; +// if (lfsr_tag_isred(p_alts[0]) +// && lfsr_tag_isle(p_alts[0])) { +// weight -= p_weights[0]; +// } +// lower_rid -= weight; +// lfsr_tag_flip2( +// &alt, &weight, +// p_alts[0], p_weights[0], +// lower_rid, upper_rid); +//// branch_ = d_branch; } printf("%04x->%04x: dtag 0x%x w%d (%d %d)\n", @@ -3051,8 +3072,11 @@ again:; weight, lower_rid, upper_rid); + + goto push; + } else { - //alt &= ~LFSR_TAG_R; + alt &= ~LFSR_TAG_R; //d_will_diverge = true; diverged = true; } @@ -3526,6 +3550,7 @@ again:; } } + push:; // trim alts from our current bounds lfsr_tag_trim2( alt, weight,