From 51a874e5849d499051b75ab77bb174c40785df19 Mon Sep 17 00:00:00 2001
From: Christopher Haster <geky@geky.net>
Date: Thu, 17 Aug 2023 00:30:36 -0500
Subject: [PATCH] Avoid aborted btree merges by deriving an upper bound on
 uncompacted size

Previously, we couldn't accurately predict if a sibling would fit in our
current rbyd because of the overhead of calculating how much space each
of our O(log(n)) alt trunks would take up.

The best we could do is make a rough estimate, and try to merge,
aborting and cleaning up any written tags if it turns out our merge
didn't end up fitting.

But I've recently found a way to calculate an upper bound without too
much overhead, relying only on the compacted estimate:

---

Consider a compacted estimate, e_c. When does our uncompacted estimate
deviate the most? When e_c is packed full of the smallest possible tag
encoding. Since, after compacting t tags, we need and additional 2 alts
and 1 null tag for our compacted rbyd, and since each tag encodes to 4
bytes at minimum, this gives us (1+2+1)*4 bytes, or 16 bytes per tag:

  e_c = 16*t

If we aren't compacting, we rely on rbyd's self-balancing properties,
which guarantees a height strictly less than 2*log2(n)+1. This gives us
a similar, but aymptotically different uncompacted estimate, e_u:

  e_u = 4*t*(1 + 2*log2(t) + 1)

Or, simplifying:

  e_u = 8*t*(log2(t) + 1)

If we know our compacted estimate, e_c, we can assume worst-case it's
full of small tags, and plug this into our uncompacted estimate e_u:

  e_u <= 8*(e_c/16)*(log2(e_c/16) + 1)

Or, simplifying:

  e_u <= (e_c/2)*(log2(e_c/16) + 1)

Since we're dealing with integers, log2(e_c/16) is strictly >= 1. We can
substitute this in for a slightly simpler equation:

  e_u <= (e_c/2)*(log2(e_c/16) + log2(e_c/16))

Or, simplifying:

  e_u <= e_c * log2(e_c/16)

This gives us a simple upper bound calculation we can do to convert any
compacted estimate into a rough, uncompacted one:

  e_u <= e_c * log2(e_c/16)

---

We can use this estimate in our btree merge code to be sure we won't
overflow our current rbyd before we even try merging.

            code          stack
  before:  20638           1744
  after:   20566 (-0.4%)   1728 (-0.9%)

It's worth noting these numbers are purely from the removal of the merge
abort code. There are likely still opportunities to save code/RAM thanks
to predicting merges more accurately.
---
 lfs.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/lfs.c b/lfs.c
index b56a50d5..44a75fbe 100644
--- a/lfs.c
+++ b/lfs.c
@@ -4011,7 +4011,7 @@ static int lfsr_btree_commit(lfs_t *lfs, lfsr_btree_t *btree,
 
         // is our compacted size too small? try to merge with one of
         // our siblings
-        if (rbyd_.eoff < lfs->cfg->block_size/4) {
+        if (rbyd_.eoff <= lfs->cfg->block_size/4) {
             goto merge;
         }
 
@@ -4266,22 +4266,22 @@ static int lfsr_btree_commit(lfs_t *lfs, lfsr_btree_t *btree,
             }
 
             // estimate if our sibling will fit
-            //
-            // this is imprecise when not compacting, so we may still fail to
-            // merge, but this at least lets us avoid wasting programming cycles
-            // when merge failure is obvious
             lfs_ssize_t estimate = lfsr_rbyd_estimateall(lfs, &sibling, -1, -1,
                     NULL);
             if (estimate < 0) {
                 return estimate;
             }
 
-            // don't fit? can't merge
-            if ((lfs_size_t)estimate > lfs->cfg->block_size/4) {
+            // doesn't fit? can't merge
+            //
+            // note we use our uncompacted estimate here, since we need to
+            // make sure our commit that merges the sibling doesn't fail
+            if (estimate * lfs_nlog2((rbyd_.eoff+estimate)/16)
+                    > lfs->cfg->block_size/4) {
                 continue;
             }
 
-            // found a sibling that can probably be merged
+            // found a sibling that can be merged
             break;
         }
 
@@ -4306,23 +4306,9 @@ static int lfsr_btree_commit(lfs_t *lfs, lfsr_btree_t *btree,
                     sibling_delta+rid_-lfs_smax32(weight_-1, 0), tag_, +weight_,
                     data_);
             if (err) {
+                LFS_ASSERT(err != LFS_ERR_RANGE);
                 return err;
             }
-
-            // if we exceed our compaction threshold our merge has failed,
-            // clean up ids and return to merge_abort
-            if (rbyd__.eoff > lfs->cfg->block_size/2) {
-                err = lfsr_rbyd_append(lfs, &rbyd__,
-                        sibling_delta+(rbyd__.weight-rbyd_.weight)-1,
-                        LFSR_TAG_RM, -(rbyd__.weight-rbyd_.weight),
-                        LFSR_DATA_NULL);
-                if (err) {
-                    return err;
-                }
-
-                rbyd_ = rbyd__;
-                goto merge_abort;
-            }
         }
 
         if (sibling.weight > 0 && rbyd_.weight > 0) {