From 86a8582445211326442aa5582341ba08533d492b Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 23 Apr 2024 15:47:37 -0500 Subject: [PATCH] Tweaked canonical altn to point to itself By definition, altns should never be followed, so it doesn't really matter where they point. But it's not like they can point literally nowhere, so where should they point? A couple options: 1. jump=jump - Wherever the old alt pointed - Easy, literally a noop - Unsafe, bugs could reveal outdated parts of the tree - Encoding size eh 2. jump=0 - Point to offset=0 - Easier, +0 code - Safer, branching to 0 should assert - Worst possible encoding size 3. jump=itself - Point to itself - A bit tricky, +4 code - Safe, should assert, even without asserts worst case infinite loop - Optimal encoding size An infinite loop isn't the best failure state, but we can catch this with an assert, which we would need for jump=0 anyways. And this is only a concern if there are other fs bugs. jump=0 is actually slightly worse if asserts are disabled, since we'd end up reading the revision count as garbage. Adopting jump=itself gives us the optimal 4-byte encoding: altbn w0 = 40 00 00 00 '-+-' ^ ^ '----|--|-- tag = altbn '--|-- weight = 0 '-- jump = itself (branch - 0) This requires tweaking the alt encoder a bit, to avoid relative encoding jump=0s, but this is pretty cheap: code stack jump=jump: 34068 2864 jump=0: 34068 (+0.0%) 2864 (+0.0%) jump=itself: 34072 (+0.0%) 2864 (+0.0%) I thought we may need to also tweak the decoder, so later trunk copies don't accidentally point to the old location, but humorously our pruning kicks in redundantly to reset altbn's jump=itself on every trunk. Note lfsr_rbyd_lookupnext was also rearranged a bit to make it easier to assert on infinite loops and this also added some code. Probably just due to compiler noise: code stack before: 34068 2864 after: 34076 (+0.0%) 2864 (+0.0%) Also note that we still accept all of the above altbn encoding options. This only affects encoding and dbg scripts. --- lfs.c | 27 +++++++++++++++------------ scripts/dbgbtree.py | 4 ++-- scripts/dbglfs.py | 4 ++-- scripts/dbgmtree.py | 4 ++-- scripts/dbgrbyd.py | 7 +++---- scripts/dbgtag.py | 4 ++-- 6 files changed, 26 insertions(+), 24 deletions(-) diff --git a/lfs.c b/lfs.c index fbf214ab..257abd21 100644 --- a/lfs.c +++ b/lfs.c @@ -2458,6 +2458,9 @@ static int lfsr_rbyd_lookupnext(lfs_t *lfs, const lfsr_rbyd_t *rbyd, // found an alt? if (lfsr_tag_isalt(alt)) { + lfs_size_t branch_ = branch + d; + + // take alt? if (lfsr_tag_follow( alt, weight, lower_rid, upper_rid, @@ -2465,19 +2468,16 @@ static int lfsr_rbyd_lookupnext(lfs_t *lfs, const lfsr_rbyd_t *rbyd, lfsr_tag_flip( &alt, &weight, lower_rid, upper_rid); - lfsr_tag_trim( - alt, weight, - &lower_rid, &upper_rid, - NULL, NULL); - branch = branch - jump; - } else { - lfsr_tag_trim( - alt, weight, - &lower_rid, &upper_rid, - NULL, NULL); - branch = branch + d; + branch_ = branch - jump; } + lfsr_tag_trim( + alt, weight, + &lower_rid, &upper_rid, + NULL, NULL); + LFS_ASSERT(branch_ != branch); + branch = branch_; + // found end of tree? } else { // update the tag rid @@ -2683,7 +2683,9 @@ static int lfsr_p_flush(lfs_t *lfs, lfsr_rbyd_t *rbyd, // change to a relative jump at the last minute lfsr_tag_t alt = p[3-1-i].alt; lfsr_rid_t weight = p[3-1-i].weight; - lfs_size_t jump = rbyd->eoff - p[3-1-i].jump; + lfs_size_t jump = (p[3-1-i].jump) + ? rbyd->eoff - p[3-1-i].jump + : 0; int err = lfsr_rbyd_appendtag(lfs, rbyd, alt, weight, jump); if (err) { @@ -3196,6 +3198,7 @@ trunk:; } // continue to next alt + LFS_ASSERT(branch_ != branch); branch = branch_; continue; diff --git a/scripts/dbgbtree.py b/scripts/dbgbtree.py index 2ee3b909..23791ddf 100755 --- a/scripts/dbgbtree.py +++ b/scripts/dbgbtree.py @@ -239,8 +239,8 @@ def tagrepr(tag, w=None, size=None, off=None): ' 0x%x' % (tag & 0x0fff) if tag & 0x0fff != 0 else '', ' w%d' % w if w is not None else '', ' 0x%x' % (0xffffffff & (off-size)) - if size is not None and off is not None - else ' -%d' % size if size is not None + if size and off is not None + else ' -%d' % size if size else '') else: return '0x%04x%s%s' % ( diff --git a/scripts/dbglfs.py b/scripts/dbglfs.py index 4fc6bf6f..8f651016 100755 --- a/scripts/dbglfs.py +++ b/scripts/dbglfs.py @@ -270,8 +270,8 @@ def tagrepr(tag, w=None, size=None, off=None): ' 0x%x' % (tag & 0x0fff) if tag & 0x0fff != 0 else '', ' w%d' % w if w is not None else '', ' 0x%x' % (0xffffffff & (off-size)) - if size is not None and off is not None - else ' -%d' % size if size is not None + if size and off is not None + else ' -%d' % size if size else '') else: return '0x%04x%s%s' % ( diff --git a/scripts/dbgmtree.py b/scripts/dbgmtree.py index 589288ac..062632e6 100755 --- a/scripts/dbgmtree.py +++ b/scripts/dbgmtree.py @@ -254,8 +254,8 @@ def tagrepr(tag, w=None, size=None, off=None): ' 0x%x' % (tag & 0x0fff) if tag & 0x0fff != 0 else '', ' w%d' % w if w is not None else '', ' 0x%x' % (0xffffffff & (off-size)) - if size is not None and off is not None - else ' -%d' % size if size is not None + if size and off is not None + else ' -%d' % size if size else '') else: return '0x%04x%s%s' % ( diff --git a/scripts/dbgrbyd.py b/scripts/dbgrbyd.py index eb858ff2..cc7a8046 100755 --- a/scripts/dbgrbyd.py +++ b/scripts/dbgrbyd.py @@ -241,8 +241,8 @@ def tagrepr(tag, w=None, size=None, off=None): ' 0x%x' % (tag & 0x0fff) if tag & 0x0fff != 0 else '', ' w%d' % w if w is not None else '', ' 0x%x' % (0xffffffff & (off-size)) - if size is not None and off is not None - else ' -%d' % size if size is not None + if size and off is not None + else ' -%d' % size if size else '') else: return '0x%04x%s%s' % ( @@ -267,8 +267,7 @@ def dbg_log(data, block_size, rev, eoff, weight, *, if not tag & TAG_ALT: j_ += size - # skip alt-nevers - if tag & TAG_ALT and tag & ~TAG_R != TAG_ALT: + if tag & TAG_ALT and size: # figure out which alt color if tag & TAG_R: _, ntag, _, _, _ = fromtag(data[j_:]) diff --git a/scripts/dbgtag.py b/scripts/dbgtag.py index cb870bc1..03076500 100755 --- a/scripts/dbgtag.py +++ b/scripts/dbgtag.py @@ -199,8 +199,8 @@ def tagrepr(tag, w=None, size=None, off=None): ' 0x%x' % (tag & 0x0fff) if tag & 0x0fff != 0 else '', ' w%d' % w if w is not None else '', ' 0x%x' % (0xffffffff & (off-size)) - if size is not None and off is not None - else ' -%d' % size if size is not None + if size and off is not None + else ' -%d' % size if size else '') else: return '0x%04x%s%s' % (