Github/littlefs
1
Fork 0
Code Actions Activity

rbyd: Terminate rbyd compaction layers with altgts

Browse Source 
This fixes a minor balance issue with rbyd commits after compaction,
reducing the worst case rbyd height by ~1/2.

I've been noticing some strange balance issues in our rbyds for a while,
it turns out our algorithms for compaction and commit weren't playing
nicely with each other.

The problem is always terminating with altles, and subtleties around
incomplete trees (non-powers-of-two).

When we terminate with altles, we end up with a dangling null tag:

  rbyd log:             rbyd tree:
  data a   <.                   .---> a
  data b   <--.             .---r-b-> b
  data c   <----.           |   .---> c
  data d   <------.     .---r-b-r-b-> d
  data e   <--------.   | .---b---b-> e
  altrle a <. | | | |   r-b---------> null
  altble b -|-' | | |
  null      |   | | |
  altrle c <--.-' | |
  altble d -|-|---' |
  null      | |     |
  altble e <----.---'
  null      | | |
  altrle b <. | |
  altble d -|-' |
  null      |   |
  altble e <--.-'
  null      | |
  altrle d -' |
  altble e ---'
  null

Which is fine, we're allowed to terminate rbyds with a null tag.

The problem is that from the rbyd's view of the world, that null tag has
the same balance as the rest of the entire tree. If we continue to
append tags (which is a very common thing to do), we end up with a
lopsided tree:

  rbyd log:             rbyd tree:
  data a   <.                   .---> a
  data b   <--.             .---r-b-> b
  data c   <----.           |   .---> c
  data d   <------.     .---r-b-r-b-> d
  data e   <--------.   | .---b---b-> e
  altrle a <. | | | |   r-b---------> null
  altble b -|-' | | |          v
  null      |   | | |           .---> a
  altrle c <--.-' | |       .---r-b-> b
  altble d -|-|---' |       |   .---> c
  null      | |     |   .---r-b-r-b-> d
  altble e <----.---'   | .---b---b-> e
  null      | | |       r-b---------> f
  altrle b <. | |              v
  altble d -|-' |                 .---> a
  null      |   |             .---r-b-> b
  altble e <--.-'             |   .---> c
  null      | |         .-----r-b-r-b-> d
  altrle d  | |         | .-----b---b-> e
  altble e  | |         | | .---------> f
  null      | |         y-r-b---------> g
  altrle d  | |                v
  altble e  | |                   .---> a
  data f   <--.               .---r-b-> b
  altrle d <. |               |   .---> c
  altrle e -|-|           .---r-b-r-b-> d
  altble f  | |         .-y-r---b---b-> e
  data g   <----.       | .-----------> f
  altble e  | | |       | | .---------> g
  altrle f  | | |       b-r-b---------> h
  altble g  | | |              v
  data h   <------.                 .---> a
  altble e -' | | |             .---r-b-> b
  altrle f ---' | |             |   .---> c
  altrle g -----' |         .---r-b-r-b-> d
  altble h -------'     .---y-r---b---b-> e
  data i                | .-------------> f
                        | | .-----------> g
                        | | | .---------> h
                        b-y-r-b---------> i

                        '---- h=4 --------'

Fortunately the solution is relatively simple. If we terminate each
layer of the compaction with an altgt, instead of an altle, the null
tag becomes unreachable:

  rbyd log:             rbyd tree:
  data a   <.                   .---> a
  data b   <--.             .---r-b-> b
  data c   <----.           |   .---> c
  data d   <------.     .---r-b-r-b-> d
  data e   <--------.   r-b---b---b-> e
  altrle a <. | | | |
  altble b -|-' | | |
  null      |   | | |
  altrle c <--.-' | |
  altble d -|-|---' |
  null      | |     |
  altbgt d <----.---'
  null      | | |
  altrle b <. | |
  altble d -|-' |
  null      |   |
  altbgt d <--.-'
  null      | |
  altrle d -' |
  altbgt d ---'
  null

This preserves the balance of the tree:

  rbyd log:             rbyd tree:
  data a   <.                   .---> a
  data b   <--.             .---r-b-> b
  data c   <----.           |   .---> c
  data d   <------.     .---r-b-r-b-> d
  data e   <. | | |     r-b---b---b-> e
  altrle a -| | | |            v
  altble b -|-' | |           .---> a
  null      |   | |       .---r-b-> b
  altrle c <--.-' |       |   .---> c
  altble d -|-|---'     .-r-b-r-b-> d
  null      | |         |       .-> e
  altbgt d  | |         b---b---b-> f
  null      | |                v
  altrle b <. |               .---> a
  altble d -|-'           .---r-b-> b
  null      |             |   .---> c
  altbgt d  |           .-r-b-r-b-> d
  null      |           |     .---> e
  altrle d  |           |     | .-> f
  altbgt d  |           b---b-r-b-> g
  null      |                  v
  altble d  |                   .---> a
  altbn  d  |             .-----r-b-> b
  altble e  |             |     .---> c
  data f   <|-.         .-r-b---r-b-> d
  altble d  | |         |     .-----> e
  altbn     | |         |     | .---> f
  altrle e  | |         |     | | .-> g
  altble f  | |         b---b-y-r-b-> h
  data g   <----.              v
  altble d  | | |             .---> a
  altbn     | | |         .---r-b-> b
  altrle e <--. |         |   .---> c
  altrle f -|-| |       .-r-b-r-b-> d
  altble g  | | |       |     .---> e
  data h   <------.     |   .-y-r-> f
  altble d -' | | |     |   | .---> g
  altble f ---' | |     |   | | .-> h
  altrle g -----' |     b---b-r-b-> i
  altble h -------'
  data i                '---- h=3 --'

As a plus, this also makes it a bit easier to see compaction layers
without decoding jumps, which is nice for debugging.

---

It's worth noting the subtlety around when use use altgts vs altles
here. We need to use altles in all but the last tag, in order to know
what the largest tag was in each subtree when building the next layer.
Fortunately, the last tag is the only tag where we need an altgt in
order to make the null tag unreachable.

This adds a bit of code, but preserving rbyd balance after compaction is
well worth it:

           code          stack          ctx
  before: 38500           2624          640
  after:  38512 (+0.0%)   2624 (+0.0%)  640 (+0.0%)
This commit is contained in:
Christopher Haster
2025-01-21 14:30:43 -06:00
parent 109bd4e0ab
commit f90906458a
1 changed files with 19 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+19 -11
View File
@@ -1332,7 +1332,7 @@ static inline void lfsr_tag_trim(
LFS_ASSERT((lfsr_srid_t)weight >= 0);
if (lfsr_tag_isgt(alt)) {
*upper_rid -= weight;
if (upper_tag && !lfsr_tag_isn(alt)) {
if (upper_tag && !lfsr_tag_isa(alt)) {
*upper_tag = alt + 1;
}
} else {
@@ -4345,6 +4345,7 @@ static int lfsr_rbyd_appendcompaction(lfs_t *lfs, lfsr_rbyd_t *rbyd,
// balanced binary tree upwards until we have a single trunk
lfs_size_t layer = off;
lfsr_rid_t weight = 0;
lfsr_tag_t tag_ = 0;
while (true) {
lfs_size_t layer_ = lfsr_rbyd_eoff(rbyd);
off = layer;
@@ -4404,23 +4405,30 @@ static int lfsr_rbyd_appendcompaction(lfs_t *lfs, lfsr_rbyd_t *rbyd,
goto done;
}
// connect with an altle
// connect with an altle/altgt
//
// note we can't use an altas here, we need to encode the
// exact tag so we know the largest tag when building the
// next layer
// note we need to use altles for all but the last tag
// so we know the largest tag when building the next
// layer, but for that last tag we need an altgt so
// future appends maintain the balance of the tree
err = lfsr_rbyd_appendtag(lfs, rbyd,
LFSR_TAG_ALT(
(i == 0 && off < layer_)
? LFSR_TAG_R
: LFSR_TAG_B,
LFSR_TAG_LE,
tag),
(off < layer_)
? LFSR_TAG_ALT(
(i == 0) ? LFSR_TAG_R : LFSR_TAG_B,
LFSR_TAG_LE,
tag)
: LFSR_TAG_ALT(
LFSR_TAG_B,
LFSR_TAG_GT,
tag_),
weight,
lfsr_rbyd_eoff(rbyd) - trunk);
if (err) {
return err;
}
// keep track of the previous tag for altgts
tag_ = tag;
}
// terminate with a null tag