From 13852df0713342273e2fb5a13aad192b02608407 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 2 Apr 2023 03:02:45 -0500 Subject: [PATCH] Switched back to altgt 0 for unreachable tags, made btree tests pass again This fixed two notable bugs: 1. Using "altle 0xfff0" to terminate unreachable rbyd trunks threw off id calculations in lfsr_rbyd_fetch searches. We derive the tag's id+weight from the lower bound calculated as the sum of all "altle"s and an always-followed "altle 0xfff0" throws this off. We _could_ derive the tag's id+weight from the upper bound, inverting this relationship, but decided to revert back to using "altgt 0" to terminate unreachable rbyd trunks. Using the lower bound is more intuitive, and "altgt 0" has the benifit of supporting variable-length tags if we ever need to adopt those. To avoid the previous issues around 0-tag holes (which was the original motivation for altle 0xfff0), 0-tags are now automatically adjusted in lfsr_rbyd_lookup, and avoided in lfsr_rbyd_append. But note! if any implemention tries to look up 0-tags, this will eventually break! See previous commits for more info. 2. Unfortunately, we can't combine branch updates and weight updates in lfsr_btree_commit in the general case. If our btree contains bname tags, the weight is attached to the bname tag, separately from the branch tag. Branch updates in lfsr_btree_commit need two separate attrs for the weight and branch struct for this reason, which is unfortunate. The amount of extra conditions to make bname+branch pairs work makes me want to redesign the inner-nodes of the btrees, but I can't think of a better way to approach the problem. --- lfs.c | 46 +++++++++++++++++++++++++--------------------- lfs_util.h | 8 ++++++++ scripts/dbgrbyd.py | 9 +++------ 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/lfs.c b/lfs.c index c9b728e3..96924c83 100644 --- a/lfs.c +++ b/lfs.c @@ -1403,21 +1403,18 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd, // note we need to include our id's weight in our weight // calculation, but only when our id is included in the tree lfs_ssize_t delta = (lower+upper) - weight; - if (!lfsr_tag_isrm(tag) - && tag != LFSR_TAG_GROW - && tag != LFSR_TAG_SHRINK) { + if (!lfsr_tag_isrm(tag)) { delta += id+1-lower; } // adjust any pending finds - if (find && find->predicted_id >= id-lfs_smax32(delta-1, 0)+(delta < 0 ? 1 : 0)) { + if (find && find->predicted_id >= (lfs_ssize_t)lower) { // pending find removed? - if (delta < 0 && find->predicted_id <= id+lfs_smin32(delta+1, 0)) { - printf("%x: a %d v %d+%d\n", off, find->predicted_id, id-(delta-1), delta); + if (delta < 0 + && find->predicted_id < (lfs_ssize_t)lower + -delta) { find->predicted_tag = 0; - find->predicted_id = id-1; + find->predicted_id = lower-1; // TODO id-1? } else { - printf("%x: b %d v %d+%d\n", off, find->predicted_id, id-(delta-1), delta); find->predicted_id += delta; } } @@ -1580,6 +1577,10 @@ static int lfsr_rbyd_lookup(lfs_t *lfs, const lfsr_rbyd_t *rbyd, lfs_ssize_t lower = -1; lfs_ssize_t upper = rbyd->weight; + // make sure we never look up zero tags, the way we create + // unreachable tags has a hole here + tag = lfs_max16(tag, 0x10); + // no trunk yet? if (!branch) { return LFS_ERR_NOENT; @@ -1963,7 +1964,7 @@ static void lfsr_rbyd_p_red( static int lfsr_rbyd_append(lfs_t *lfs, lfsr_rbyd_t *rbyd, lfsr_tag_t tag, lfs_ssize_t id, lfs_ssize_t delta, lfsr_data_t data) { - // 0 tags shouldn't be written to disk, use unr if tag is unreachable + // never write zero tags to disk, use unr if tag contains no data LFS_ASSERT(tag != 0); // ignore noops @@ -2001,8 +2002,9 @@ static int lfsr_rbyd_append(lfs_t *lfs, lfsr_rbyd_t *rbyd, id -= 1; id_ = id + 1; other_id_ = id + 1; - tag_ = 0; - other_tag_ = 0; + // also note these tags MUST NOT be zero, due to unreachable tag holes + tag_ = 0x10; + other_tag_ = 0x10; } else if (lfsr_tag_ismk(tag) && delta < 0) { LFS_ASSERT(id < rbyd->weight); @@ -2011,8 +2013,9 @@ static int lfsr_rbyd_append(lfs_t *lfs, lfsr_rbyd_t *rbyd, id += 1; id_ = id - lfs_smax32(-delta, 0); other_id_ = id; - tag_ = 0; - other_tag_ = 0; + // also note these tags MUST NOT be zero, due to unreachable tag holes + tag_ = 0x10; + other_tag_ = 0x10; } else if (lfsr_tag_isrm(tag)) { LFS_ASSERT(id < rbyd->weight); @@ -2347,7 +2350,7 @@ static int lfsr_rbyd_append(lfs_t *lfs, lfsr_rbyd_t *rbyd, || lfsr_tag_key(tag_) < lfsr_tag_key(tag)))) { if (lfsr_tag_isrm(tag)) { // if removed make our tag unreachable - alt = LFSR_TAG_ALT(B, LE, 0xfff0); + alt = LFSR_TAG_ALT(B, GT, 0); weight = upper_id - lower_id - 1 + delta; } else { // split less than @@ -2361,7 +2364,7 @@ static int lfsr_rbyd_append(lfs_t *lfs, lfsr_rbyd_t *rbyd, || lfsr_tag_key(tag_) > lfsr_tag_key(tag)))) { if (lfsr_tag_isrm(tag)) { // if removed make our tag unreachable - alt = LFSR_TAG_ALT(B, LE, 0xfff0); + alt = LFSR_TAG_ALT(B, GT, 0); weight = upper_id - lower_id - 1 + delta; } else { // split greater than @@ -3241,7 +3244,6 @@ static int lfsr_btree_commit(lfs_t *lfs, continue; split:; - printf("B SPLIT %x %d\n", rbyd_.block, rbyd_.weight); // find out which id we need to split around lfs_ssize_t bisect = lfsr_rbyd_bisect(lfs, rbyd); if (bisect < 0) { @@ -3480,7 +3482,6 @@ static int lfsr_btree_commit(lfs_t *lfs, continue; merge:; - printf("B MERGE %x %d\n", rbyd_.block, rbyd_.weight); // last child? try the left sibling // lfs_ssize_t sid; lfs_ssize_t sdelta; @@ -3657,7 +3658,10 @@ static int lfsr_btree_commit(lfs_t *lfs, MKUNR, sid, -sweight, NULL, 0, &scratch_attrs[1]); scratch_attrs[1] = *LFSD_ATTR( - BRANCH, rid, +rbyd_.weight-rweight, scratch_buf1, delta1, + BRANCH, rid, 0, scratch_buf1, delta1, + &scratch_attrs[2]); + scratch_attrs[2] = *LFSD_ATTR( + UNR, rid, +rbyd_.weight-rweight, NULL, 0, NULL); } @@ -3781,9 +3785,9 @@ static int lfsr_btree_update(lfs_t *lfs, lfsr_btree_t *btree, return lfsr_btree_commit(lfs, btree, id, &rbyd, LFSD_ATTR_IF_(tag != rtag, lfsr_tag_setrm(rtag), rid, 0, NULL, 0, - LFSD_ATTR_(tag, rid, +weight-rweight, - buffer, size, - NULL))); + LFSD_ATTR_(tag, rid, 0, buffer, size, + LFSD_ATTR(UNR, rid, +weight-rweight, NULL, 0, + NULL)))); } } diff --git a/lfs_util.h b/lfs_util.h index 5ed1eba8..5879acdc 100644 --- a/lfs_util.h +++ b/lfs_util.h @@ -138,6 +138,14 @@ static inline int32_t lfs_smin32(int32_t a, int32_t b) { return (a < b) ? a : b; } +static inline uint16_t lfs_max16(uint16_t a, uint16_t b) { + return (a > b) ? a : b; +} + +static inline uint16_t lfs_min16(uint16_t a, uint16_t b) { + return (a < b) ? a : b; +} + // TODO how many of these do we actually need // Swap two 16-bit numbers static inline void lfs_swap16(uint16_t *a, uint16_t *b) { diff --git a/scripts/dbgrbyd.py b/scripts/dbgrbyd.py index 7ece3198..007f917e 100755 --- a/scripts/dbgrbyd.py +++ b/scripts/dbgrbyd.py @@ -275,7 +275,7 @@ def show_log(block_size, data, rev, off, *, # note we ignore out-of-bounds here for debugging if delta > 0: # grow lifetimes - i, id_ = index(weights, id-(delta-1)) + i, id_ = index(weights, lower_) if id_ > 0: weights[i:i+1] = [id_, delta, weights[i]-id_] lifetimes[i:i+1] = [ @@ -288,21 +288,18 @@ def show_log(block_size, data, rev, off, *, elif delta < 0: # shrink lifetimes - i, id_ = index(weights, id+1) + i, id_ = index(weights, lower_) delta_ = -delta weights_ = weights.copy() lifetimes_ = lifetimes.copy() shrinks = set() while delta_ > 0 and i < len(weights_): - if id_ > 0: + if weights_[i] > delta_: delta__ = min(delta_, weights_[i]-id_) delta_ -= delta__ weights_[i] -= delta__ i += 1 id_ = 0 - elif weights_[i] > delta_: - weights_[i] -= delta_ - size_ = 0 else: delta_ -= weights_[i] weights_[i:i+1] = []