Github/littlefs
1
Fork 0
Code Actions Activity

rbyd-rr: Made remove leaf-splits red

Browse Source 
This avoids the alta prune cludge, where we prune altas unconditionally
knowing we only emit these to make removes work.

The alta prune cludge was a bit concerning forward-compatibility-wise,
since it technically violates the rby structure of the tree, but
necessary to prevent unbalancing when we terminate remove leaves with
black altas. Terminating with a black alta technically also violates the
rby structure, and two wrongs make a right, right?

But why are these altas black? To be honest it's just what made the code
work in the moment. These should be red, but terminating with red altas
turned out to be surprisingly tricky.

The problem is when we terminate with a red alta, the alta is subject to
recoloring, and may be reordered as a part of a yellow node to preserve
the yellow-alts-point-same-dir invariant. But if you reorder the alta,
anything after it becomes unreachable! Not good!

  altrgt 0x200           altrgt 0x200
  altrle 0x100    =>     altra
  altba         yellow   altble 0x100 <-- unreachable!
  null          reorder  null

The solution here turned out to just not use altas at all. If we're
careful with our tag bounds, we can create an alt that is _implicitly_
alta without a special encoding. Such an alt can be reordered without
issue:

  altrgt 0x200           altrgt 0x200
  altrle 0x100    =>     altrgt 0x100
  altbgt 0x100  yellow   altble 0x100 <-- reachable
  null          reorder  null         <-- unreachable

The other option would have been to make lfsr_rbyd_p_recolor alta aware,
but this would have been quite complicated and fully of special cases...

The "if we're careful with our tag bounds" is the tricky bit, since we
didn't really need to be that careful before. But the end result is
tracking diverged tag bounds the same way we track diverged rid bounds,
which is a nice bit of consistency. This also avoids annoying yellow
terminating d_state corner cases. It's a nice improvement.

As a part of these changes I also tweaked to the lower_tag bound to
track last seen alt instead of alt+1. To be honest I'm not really sure
how alt+1 got there. I guess to be consistent with the upper_tag bound?
Bound the upper_tag bound is exclusive, so this ends up weird and
difficult to reason about...

This change also makes it so _all_ leaf splits end up red, which is
unexpected but nice for consistency. We can probably make leaf-split
recoloring unconditional eventually.
This commit is contained in:
Christopher Haster
2024-03-27 17:11:33 -05:00
parent 5269f79431
commit a5999c892b
1 changed files with 40 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+40 -40
View File
@@ -924,7 +924,7 @@ static inline bool lfsr_tag_prune2(
alt, weight,
alt2, weight2,
lower_rid, upper_rid,
lower_rid-1, lower_tag);
lower_rid-1, lower_tag+1);
} else {
return lfsr_tag_follow2(
alt, weight,
@@ -965,7 +965,7 @@ static inline void lfsr_tag_trim(
} else {
*lower_rid += weight;
if (lower_tag && !lfsr_tag_isn(alt)) {
*lower_tag = alt + 1;
*lower_tag = alt;
}
}
}
@@ -2792,6 +2792,7 @@ static int lfsr_rbyd_appendattr(lfs_t *lfs, lfsr_rbyd_t *rbyd,
// note both normal and rm wide-tags have the same bounds, really it's
// the normal non-wide-tags that are an outlier here
if (lfsr_tag_issup(tag)) {
// TODO why can't this be rid+1?
a_tag = 0x000;
b_tag = 0x800;
} else if (lfsr_tag_issub(tag)) {
@@ -2799,7 +2800,7 @@ static int lfsr_rbyd_appendattr(lfs_t *lfs, lfsr_rbyd_t *rbyd,
b_tag = lfsr_tag_supkey(tag) + 0x100;
} else if (lfsr_tag_isrm(tag) || !lfsr_tag_key(tag)) {
a_tag = lfsr_tag_key(tag);
b_tag = lfsr_tag_key(tag) + 0x1;
b_tag = lfsr_tag_key(tag) + 1;
} else {
a_tag = lfsr_tag_key(tag);
b_tag = lfsr_tag_key(tag);
@@ -2941,11 +2942,11 @@ again:;
// alt &= ~LFSR_TAG_R;
// }
// TODO better solution?
// always prune alt-always tags
if (lfsr_tag_isa(alt)) {
goto prune;
}
// // TODO better solution?
// // always prune alt-always tags
// if (lfsr_tag_isa(alt)) {
// goto prune;
// }
// prune?
// <b >b
@@ -3077,13 +3078,15 @@ again:;
p_alts[0] &= ~LFSR_TAG_R;
lfsr_rbyd_p_recolor(p_alts, p_weights, p_jumps);
// keep track of last alt on diverged trunk to stitch the
// trunks together with
if (d_state == LFSR_D_DIVERGEDLOWER
&& !lfsr_tag_isn(p_alts[0])) {
d_tag = p_alts[0];
// d_rid = lower_rid;
}
// // keep track of last alt on diverged trunk to stitch the
// // trunks together with
// if ((d_state == LFSR_D_DIVERGEDLOWER
// || (d_state == LFSR_D_NOTDIVERGING
// && lfsr_tag_isle(p_alts[0])))
// && !lfsr_tag_isn(p_alts[0])) {
// d_tag = p_alts[0];
// // d_rid = lower_rid;
// }
branch = branch_;
continue;
@@ -3136,13 +3139,15 @@ again:;
&lower_rid, &upper_rid,
&lower_tag, &upper_tag);
// keep track of last alt on diverged trunk to stitch the
// trunks together with
if (d_state == LFSR_D_DIVERGEDLOWER
&& !lfsr_tag_isn(alt)) {
d_tag = alt;
// d_rid = lower_rid;
}
// // keep track of last alt on diverged trunk to stitch the
// // trunks together with
// if ((d_state == LFSR_D_DIVERGEDLOWER
// || (d_state == LFSR_D_NOTDIVERGING
// && lfsr_tag_isle(alt)))
// && !lfsr_tag_isn(alt)) {
// d_tag = alt;
// // d_rid = lower_rid;
// }
}
// push alt onto our queue
@@ -3206,16 +3211,22 @@ again:;
// no divergence? guess we only need one trunk then, actually write
// it out this time
if (d_state == LFSR_D_DIVERGINGLOWER) {
printf("%04x->%04x: not diverging\n",
branch,
rbyd->eoff);
d_state = LFSR_D_NOTDIVERGING;
goto again;
// diverged lower trunk? we need an upper trunk too
} else if (d_state == LFSR_D_DIVERGEDLOWER) {
printf("%04x->%04x: diverging switch\n",
branch,
rbyd->eoff);
// keep track of last alt on diverged trunk to stitch the trunks
// together with
d_state = LFSR_D_DIVERGINGUPPER;
// d_tag = p_alts[0];
d_rid = lower_rid;
d_tag = lower_tag;
// flush any pending alts
err = lfsr_rbyd_p_flush(lfs, rbyd,
@@ -3243,8 +3254,9 @@ again:;
// diverged upper trunk? done diverging
} else if (d_state == LFSR_D_DIVERGEDUPPER) {
// use the diverged rid bound for leaf weight calculation
// use the lower diverged bound for leaf weight calculation
lower_rid = d_rid;
lower_tag = d_tag;
}
// split leaf nodes?
@@ -3275,18 +3287,12 @@ again:;
&& lfsr_tag_key(tag_) < lfsr_tag_key(tag)))))) {
if (lfsr_tag_isrm(tag) || !lfsr_tag_key(tag)) {
// if removed, make our tag unreachable
alt = LFSR_TAG_ALT(LFSR_TAG_GT, LFSR_TAG_B, 0);
alt = LFSR_TAG_ALT(LFSR_TAG_GT, LFSR_TAG_R, lower_tag);
weight = upper_rid - lower_rid + delta;
upper_rid -= weight;
} else {
// split less than
alt = LFSR_TAG_ALT(
LFSR_TAG_LE,
LFSR_TAG_R,
// (!lfsr_d_isdiverged(d_state))
// ? LFSR_TAG_R
// : LFSR_TAG_B,
tag_);
alt = LFSR_TAG_ALT(LFSR_TAG_LE, LFSR_TAG_R, tag_);
weight = upper_rid - lower_rid;
lower_rid += weight;
}
@@ -3302,18 +3308,12 @@ again:;
&& lfsr_tag_key(tag_) > lfsr_tag_key(tag)))))) {
if (lfsr_tag_isrm(tag) || !lfsr_tag_key(tag)) {
// if removed, make our tag unreachable
alt = LFSR_TAG_ALT(LFSR_TAG_GT, LFSR_TAG_B, 0);
alt = LFSR_TAG_ALT(LFSR_TAG_GT, LFSR_TAG_R, lower_tag);
weight = upper_rid - lower_rid + delta;
upper_rid -= weight;
} else {
// split greater than
alt = LFSR_TAG_ALT(
LFSR_TAG_GT,
LFSR_TAG_R,
// (!lfsr_d_isdiverged(d_state))
// ? LFSR_TAG_R
// : LFSR_TAG_B,
tag);
alt = LFSR_TAG_ALT(LFSR_TAG_GT, LFSR_TAG_R, tag);
weight = upper_rid - (rid+1);
upper_rid -= weight;
}