233fc2c2129b04e0741b084f9be13b8ffe2cfbfe
1463 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Christopher Haster
|
233fc2c212 |
rbyd-rr: Attempting correct balance of the diverging node itself
So far, our color-balance preserving range removal algorithm is working
great:
- Common trunk? color-balance preserving ✓
- Lower-diverged trunk? color-balance preserving ✓
- Upper-diverged trunk? color-balance preserving ✓
The only hole in our algorithm is the color-balance of the diverging
node itself.
Up until now we've simply recolored the diverging alt black, as this
avoids a large number of complicated corner cases. Unfortunately this
has the consequence of potentially offsetting the balance of our tree
by +-1:
.-> b-> .---b-> h=2 -.
.---b-> rm me | |
| .-> => => b-b-b-> +- unbalanced :(
| .-b-> | '-> |
| | .-> | .-> |
r-b-b-> '-b-> h=3 -'
^
diverging
This attempts to preserve the coloring of the diverging alt, and
preserve the color-balance, but we quickly run into the, uh, previously
mentioned complicated corner cases...
- First to note, we _can_ preserve red coloring on the gt path:
.-> b-> .---b-> h=2 -.
.---b-> rm me | |
| .-> => => r-b-b-> +- balanced :)
| .-b-> | '-> |
| | .-> | .-> |
r-b-b-> '-b-> h=2 -'
^
diverging
But only if it isn't a part of a pending yellow split. If it _is_ a
pending yellow split, the yellow split may try to reference the
yellow node in the history, but this won't work because our history
has been modified:
.-> .-> h=2 -.
.-----b-> .-----b-> |
| .-> b-> | .---b-> |
| .---b-> rm me => => | | +- unbalanced :(
| | .-> r-b-b-b-> |
| | .-b-> | '-> |
| | | .-> '-b-> |
y-r-b-b-> '-> h=3 -'
^ '-+-'
diverging wants to have split
- As for the le path, we can't even preserve the red coloring! For this
to work we would need to somehow color a flipped alt red (so the
"follow" edge is red, not the "not-follow"), but this isn't possible
with our encoding scheme (and definitely not worth reserving a whole
additional bit in every alt for):
r-b-b-> .-b-> .-b-> h=3 -.
| | '-> | '-> | '-> |
| '-b-> => | .-> => | .-> +- unbalanced :(
| '-> b-b-> .-b-b-> |
'---b-> rm me | |
'-> b---b-> h=2 -'
^ ^
diverging this wants to be red
The reason we can preserve reds on the gt path but not the le path is
because we write the le path first and stitch on the gt path. If
instead you wrote the gt path first, this would be flipped:
r-b-b-> .-b-> h=2 -.
| | '-> | '-> |
| '-b-> => => | .-> +- balanced :)
| '-> r-b-b-> |
'---b-> rm me | |
'-> b-> '---b-> h=2 -'
^
diverging
In theory, you could do _another_ pass over the tree to figure out
which order is needed to preserve coloring. But this would be an even
more complicated mess...
Not to mention this wouldn't even completely solve the color-balance
of the diverging alt because of yellow split issues...
And we haven't even touched issues related to yellow split color
propagation! Fortunately this JustWorksTM on the gt path, since it
mostly looks like a normal trunk after stitching. But we completely
ignore yellow split color propagation on the le path since this runs
into many of the same issues as red flipping.
But if you manage to make it though all of this mess while preserving
color-balance (code size be damned), we arive on what seems to be an
impossible case: How do you preserve color balance of a diverging alt
when both paths contain a pending yellow split?
.-> .-> .-> h=4 -.
.-----b-> .-b-> .-b-> |
| .-> | .-> | .-> |
| .---b-> .-y-b-> .-y-b-> |
| | .-> | .-> | .-> |
| | .-b-> | .-b-> | .-b-> |
| | | .-> b-b-b-> .-b-b-b-> |
.-y-r-b-b-> rm me => => | +- unbalanced :(
| .-> r-b-b-b-> |
| .-----b-> | | '-> |
| | .-> | | .-> |
| | .---b-> | '-b-> |
| | | .-> | .-> |
| | | .-b-> | .-b-> |
| | | | .-> | | .-> |
b-y-r-b-b-> '-b-b-> h=3 -'
^ ^
diverging lost color propagation
This seems to violate tail recursion!
Anyways, this turned into a bit of a rant and a bit of a mess.
If anyone reads this and is interested in exploring the balancing issues
further, the diverging alt logic currently contains some commented-out
coloring conditions:
(true) / (false) / (lfsr_tag_isred(p_alts[0]))
These are currently commented-out to what is currently known to be
optimal (see above), but can be tweaked to try to preserve different
colorings.
|
||
|
Christopher Haster
|
4f14f3cef4 |
rbyd-rr: Fixed issue where red alts were just not being pruned
Not sure how I missed this earlier, but we aren't pruning unreachable/
unavoidable red alts.
There are two cases where we can use red alts to prune. Both cases
effectively collapse a 3-node into a 2-node, while converting isolated
black alts into altns effectively collase a 2-node into a 1-node:
.---> a rm me
| .-> b red prune .-> b <-- we weren't handling
-r-b-> c => ---b-> c this case correctly
.---> a .---> a
| .-> b rm me red prune |
-r-b-> c => -b---> c
.-> a rm me v------ altn
.-b-> b black flatten .-b-> b
| .-> c => | .-> c
-b-b-> d -b-b-> d
Humorously, we were handling the arguably more difficult case of pruning
a black alt following a red alt correctly. But we weren't handling the
case when a red alt itself needs to be pruned.
Fortunately this code is identical to pruning root alts (also arguably a
more tricky case!), so we can just extend the relevant if statement to
cover the case of an unreachable/unavoidable red alt.
And small code change means small code change:
code stack
before: 34288 2864
after: 34304 (+0.0%) 2864 (+0.0%)
|
||
|
Christopher Haster
|
37c45e1afc |
Fixed coloring conflicts in rbyd tree renderers
A bit of a hack, but rather than handling conditional alt branches, our dbg rbyd tree renderers just represent single-pointer alts as an alt with both branches pointing to the place. Unfortunately, the two branches technically have different colors. This resulted in a bit of contention when chosing how to color the tree. Basically Python's dict ordering would determine which color won. Which was a bit confusing when dbgrbyd.py displayed different tree colorings for the same rbyd. dbgrbyd.py should be idempotent! This is solved by adding another hack to check explicitly for same-destination branches. |
||
|
Christopher Haster
|
c3dc7cca10 |
Fixed underflow issue with truncating test/bench -C/--context
There was no check on context > stdout, so requesting more context than was actually printed by the test could result in a negative value. Python "helpfully" interpreted this as a negative index, resulting in somewhat random context lengths. This, combined with my tendency to just default to a large number like --context=100, led to me thinking a test was printing much less than it actually was... Don't get me wrong, I love Python, and I think Python's negative indices are a clever way to add flexibility to slice notation, but the value-dependent semantics are a pretty unfortunate footgun... |
||
|
Christopher Haster
|
1ce47bfc47 |
rbyd-rr: Implemented coloring during rbyd compaction
This tweaks our rbyd compaction algorithm to color the alts correctly to
represent a balanced 2-3-4 tree.
Previously, we didn't really care about coloring the compacted tree,
because we didn't really care about color when pruning unreachable
alts.
But now that we refuse to prune isolated black alts, or risk unbalancing
the underlying 2-3-4 tree, it's important we color the compacted tree
correctly. Otherwise the unreachable alts that terminate our binary nodes
will just never be pruned, unbalancing each layer of the tree by ~1.
Compaction without coloring:
tags: effective rby tree:
data a <. .---> a
data b <--. .---b-b-> b
data c <----. | .---> c
data d <------. b-b-b-b-> d
altble a <. | | |
altble b -|-' | |
null | | | effective 2-3-4 tree:
altble c <--.-' | .---o -.
altble d -|-|---' | o |
null | | .-o .-o +- h=4
altble b -' | | o | o |
altble d ---' a b c d -'
null
Compaction with coloring:
tags: effective rby tree:
data a <. .---> a
data b <--. .---r-b-> b
data c <----. | .---> c
data d <------. r-b-r-b-> d
altrle a <. | | |
altble b -|-' | |
null | | | effective 2-3-4 tree:
altrle c <--.-' | .---o -.
altble d -|-|---' .-o .-o +- h=2
null | | a b c d -'
altrle b -' |
altble d ---'
null
Note that if the compacted tree is not full, i.e. not a power-of-two, we
need to make sure the resulting unary nodes are still colored black.
Isolated red alts are not allowed and would create even more hilarious
problems.
Fortunately there is just enough context in lfsr_rbyd_appendcompaction,
since we know exactly where each layer ends, to determine if each node
is binary or unary without needing to attempt to read unnecessary tags.
It's also worth noting the resulting unary nodes may seem like an
unnecessary side effect, but they are actually quite useful here for
preserving the underlying 2-3-4 balance! In the same way unary nodes
preserve the 2-3-4 balance during range operations, unary nodes in the
compacted tree can be consumed later to introduce new attrs without
unbalancing the tree.
Now I'm wondering, how would a rebalancing algorithm even work on a
red-black tree without unary nodes...? Did I dodge a bullet here?
Unaligned compaction with coloring:
tags: effective rby 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 | | | | effective 2-3-4 tree:
altrle c <--.-' | | .-o -.
altble d -|-|---' | .---o o +- h=3
null | | | .-o .-o o |
altble e <----.---' a b c d e -'
null | | |
altrle b <. | |
altble d -|-' |
null | |
altble e <--.-'
null | |
altrle d -' |
altble e ---'
null
Code changes minimal, just needed some twiddly logic in
lfsr_rbyd_appendcompaction to make this work:
code stack
before: 34256 2864
after: 34288 (+0.1%) 2864 (+0.0%)
|
||
|
Christopher Haster
|
c08b7ccdd8 |
rbyd-rr: Fixed yellow-alt pruning being completely broken
At some point during all this refactoring, `branch_ = branch` snuck its
way into the common red-black pruning code:
// collapse unreachable red alts
if (lfsr_tag_isred(p_alts[0])) {
alt = p_alts[0] & ~LFSR_TAG_R;
weight = p_weights[0];
jump = p_jumps[0];
branch_ = branch; // <-- ???
lfsr_rbyd_p_pop(p_alts, p_weights, p_jumps);
What this ends up doing is forcing the appendattr logic to branch to
where it just was.
Ignoring concerns about forward-progress, this somewhat humorously
undoes the pruning of the alt. It's technically not an error, since the
alt was prunable, but certainly counter-productive.
First noticed because our post-split yellow alts were not getting
cleaned up correctly, even though all the correct conditions were being
hit.
---
Unfortunately, attempting to simply remove that line breaks things.
It turns out revisiting the pruned alt was hiding the fact that using an
lfsr_tag_follow2(a_rid, a_tag) check to determine if we take the pruned
alt is insufficient.
At first glance this appears to be sufficient, after all if an alt is
always taken, shouldn't lfsr_tag_follow2(a_rid, a_tag) always return
true?
The problem is when we look up a_rid/a_tag outside the tree.
lfsr_tag_follow2(a_rid, a_tag) may return false, but _in the context of
our current lower/upper bound_, the alt may always be taken and
lfsr_tag_prune2() may return true. This mismatch in lfsr_tag_prune2 and
lfsr_tag_follow2 breaks the underlying logic and causes the wrong branch
to be taken.
The fix here is to use the same reachability logic for both the pruning
check and follow check. So a_rid/a_tag should not be involved in the
pruning logic at all, which makes a bit of sense since a_rid/a_tag do
not determine if an alt is reachable.
I've also gone ahead and replaced lfsr_tag_prune{,2} with
lfsr_tag_unreachable{,2} (never taken) and lfsr_tag_unavoidable{,2}
(always taken) which I think capture/document the underlying conditions
we need a bit better.
Code changes:
code stack
before: 34220 2864
after: 34256 (+0.1%) 2864 (+0.0%)
It's good that even though we changed a number of functions, the code
changes match our expectation that the underlying logic didn't really
change all that much.
|
||
|
Christopher Haster
|
dcc67d22a8 |
rbyd-rr: Tweaked lfsr_tag_follow to make altn/alta implicit again
In theory, checking altn/alta tags for followability should be implicit.
These are encoding as altle/altgt tag 0, which should never be requested
in normal operation:
altn => altle 0
alta => altgt 0
But while that's good in theory, null tags, tag 0, has a tendency to
creep into these functions and has already caused a number of headaches.
Conditionally checking for altn/alta is safer, but asserting on tag 0 is
just as safe and adds no code cost.
Both lfsr_rbyd_appendattr and lfsr_rbyd_lookupnext have
`tag = lfs_max16(tag, 0x1)` guards now to comply with this rule. But
it's still a nice safety net to assert on tag 0 in lfsr_tag_follow*.
In case you were curious if the max16 guards were more expensive than
the explicit altn/alta checks, code size says no:
code stack
before: 34256 2864
after: 34220 (-0.1%) 2864 (+0.0%)
|
||
|
Christopher Haster
|
0475af0415 |
Renamed lower/upper -> lower_rid/upper_rid for consistency/clarity
The lower/upper names were introduced fairly early. I think before appendattr bounds included lower_tag/upper_tag? Since then the explicit lower_rid/upper_rid names have become more common. Changing for consistency, and because, you know, it's probably a bit better to indicate what these variables actually are the lower/upper bounds of... |
||
|
Christopher Haster
|
079f4f67fb |
rbyd-rr: Eagerly prune unreachable root alts
In a traditional B-tree/2-3-4 tree/red-black tree, balance is maintained
by enforcing a set of rules such that no operation changes the balance
of the tree. In such a ruleset, you quickly learn that the only way to
actually change the height of the tree is through the root, since the
root is the only node shared by all branches of the tree.
This is why B/2-3-4/red-black insert/removes usually end in "and then
if you hit the root of the tree, increase/decrease the height by one".
Our range removal algorithm is a bit different in that we aren't
guaranteed to reach the root, the requested range could be empty after
all, but we also aren't _prohibited_ from decreasing the height of the
tree if it only involves removing the root.
Removing the root still maintains the 2-3-4 structure and balance of our
tree.
---
This commit adds opportunistic root pruning to our set of possible
pruning conditions.
This also tweaks diverging-lower pruning to take advantage of root
pruning. Since we prune the entire diverging-lower path, we can pretend
diverging-lower alts are prunable roots up until we find the diverging
alt. This leads to a bit nicer code since root pruning is so simple.
This actually ended up revealing an issue with how we indirectly
trigger diverged pruning by triming diverging alts: Trimming works, but
we also need to zero any weight, or else later calculations get all
screwy...
I guess the extra coverage from reusing logic is a plus.
Code changes:
code stack
before: 34236 2864
after: 34256 (+0.1%) 2864 (+0.0%)
|
||
|
Christopher Haster
|
115fad0c80 |
rbyd-rr: Tweaked diverging machine for a bit better code reuse
Mainly deduplicating the pruning of pre-diverged-lower alts and the
diverging alt itself.
This saves some code:
code stack
before: 34308 2864
after: 34236 (-0.2%) 2864 (+0.0%)
|
||
|
Christopher Haster
|
120f0a2e17 |
rbyd-rr: Cleanup of new structure-preserving diverging algorithm
Since this set of changes are fairly stable now, and show improved
balancing during range operations, it's probably a good checkpoint to
summarize the changes to the diverging range-removal algorithm.
From a high-level, the range-removal algorithm is mostly unchanged:
1. Guess if we are performing a range operation. This is determined by
the delta and sup/sub bits. If we aren't, do a normal append.
2. Diverging-lower: Start traversing the rbyd, but don't write out any
alts yet. If we find an alt where our range would diverge, transition
to the next step. If we don't, fall back to a normal append. This
requested range contains no alts in this case.
3. Diverged-lower: Write out alts < requested range. Keep track of the
resulting lower trunk and lower bound.
4. Diverging-upper: Reset and start traversing the rbyd again, this time
writing out all alts that we know are common. This will become our
actual trunk.
When we find the diverging alt this time, replace it with a stitching
alt that points to the lower trunk.
5. Diverged-upper: Write out alts > requested range.
6. Create a new leaf alt as normal, but using the lower trunk's lower
bound and upper trunk's upper bound.
What has changed is how we prune alts in the requested range after we've
found the diverging alt.
Previously, we would simply remove these alts from the tree, but this
would throw away color information and result in an unbalanced 2-3-4
tree. Not an immediately obvious issue since the actual binary tree
stays more-or-less balanced, but as more rbyd operations pile on the
self-balancing breaks, and the resulting tree could become up to ~2x
unabalanced:
.-------o-------.
.---o---. .---o---.
.-o-. .-o-. .-o-. .-o-.
.o. .o. .o. .o. .o. .o. .o. .o.
a b c d e f g h i j k l m n o p
'------+------'
remove
.--------o
.---o---. |
.-o-. .-o-. |
.o. .o. .o. .o. |
a b c d e f g h i
^
append j'k'l'
.-----o
.--------o .-+-r
.---o---. | | | |
.-o-. .-o-. | | | |
.o. .o. .o. .o. | | | |
a b c d e f g h i j'k'l'
^
append m'n'o'p'q'r'
.-------------o
.---o .---+-----r
.--------o .-o .-o .-o .-+-r
.---o---. | | | | | | | | | |
.-o-. .-o-. | | | | | | | | | |
.o. .o. .o. .o. | | | | | | | | | |
a b c d e f g h i j'k'l'm'n'o'p'q'r'
Now, instead, we preserve 2-3-4 nodes by only removing alts that are red
or have a red neighbor. Black alts are not removed, but instead
converted to "alt-never" (altn) alts that represent a sort of empty
1-node:
.---> a rm me
| .-> b red prune .-> b
-r-b-> c => ---b-> c
.-> a rm me v-------- altn
.-b-> b black flatten .-b-> b -.
| .-> c => | .-> c +- note the tree is balanced
-b-b-> d -b-b-> d -'
lfsr_rbyd_p_recolor is extended such that if we push up a red alt into
an altn, instead of recoloring red, we just reclaim the altn. This
effectively transitions from a 1-node -> 2-node in the same way
recoloring transitions from a 2-node -> 3-node or 3->node -> 4-node:
.-> a' .-> a'
.-b-> b insert a' .-r-b-> b reclaim altn .-b-> b
| .-> c => | .-> c => | .-> c
-b-b-> d -b-b-> d -b-b-> d
The result, counterintuitively, is that by introducing otherwise
unecessary altns, we can preserve the structure of the 2-3-4 tree and
better preserve the balance of the tree:
.-------o-------.
.---o---. .---o---.
.-o-. .-o-. .-o-. .-o-.
.o. .o. .o. .o. .o. .o. .o. .o.
a b c d e f g h i j k l m n o p
'------+------'
remove
.--------o
.---o---. o
.-o-. .-o-. o
.o. .o. .o. .o. o
a b c d e f g h i
^
append j'k'l'm'
.----------------o
.---o---. o
.-o-. .-o-. .------o
.o. .o. .o. .o. .o. .-+-r
a b c d e f g h i j'k'l'm'
^
append n'o'p'q'r's'
.----------------------------o
.---o---. .-------------o
.-o-. .-o-. .---o .---+-----r
.o. .o. .o. .o. .-o .-o .-o .-o .-+-r
a b c d e f g h i j'k'l'm'n'o'p'q'r's'
Though I guess altns technically make this a 1-2-3-4 tree...
Note that this algorithm does _not_ maintain a strictly balanced tree in
terms of the current number of attrs, h<=log n. But it _does_ maintain a
balanced tree in terms of the worst possible sequence of append
operations. And since our rbyd are bounded by our block size, this is
strictly h<=log b.
---
This algorithm, as implemented, is not perfect.
We are correctly maintaining the 2-3-4 structure both before and after
the tree diverges, but this is a bit hand-wavey about the diverging alt
itself. And the diverging alt proves to be annoyingly tricky.
We want to replace the diverging alt with a stitching alt to tie
together the lower and upper diverged paths, but doing so while
maintaining the color the diverging alt interacts with later red flips
and yellow splits in _very_ ugly ways.
The solution right now is to just unconditionally recolor the diverging
alt black. This avoids a whole set of diverged-recoloring issues, but
does risk unbalancing our tree by +1 if we diverge on a red alt.
Still, this is a significant improvement over the +~2x of the previous
algorithm. And the altns introduce significant flexiblity into the tree,
so it may be possible to avoid this +1 unbalancing at some point in the
future.
---
This commit is mainly a cleanup commit, removing commented-out code,
debugging printfs, asserts, etc.
Other minor changes:
- Move y_branch updates to beginning of alt loop, instead of in every
single branch tail.
- Deduplicated black recoloring in lfsr_rbyd_p_recolor again.
- Made leaf-split red recoloring unconditional, since all leaf-split
alts are now red. This is a good sign that our new algorithm is more
correct.
Now that the dust has settled, we can look into how these algorithm
tweaks impact code cost:
code stack
rr-div-naive: 33968 2864
rr-div-altn: 34308 (+1.0%) 2864 (+0.0%)
If we focus on lfsr_rbyd_appendattr, which contains almost all of the
actual diverging logic, we can also compare against the original naive
stitching algorithm (rr-stitching). Keep in mind rr-stitching could
increase the binary height by ~2x, naive diverging (rr-div-naive) the
2-3-4 height by ~2x, and our current algorithm (rr-div-altn) the 2-3-4
height by ~1:
code frame stack
appendattr rr-stitching: 1940 184 536
appendattr rr-div-naive: 2028 (+4.5%) 200 (+8.7%) 552 (+3.0%)
appendattr rr-div-altn: 2584 (+33.2%) 232 (+26.1%) 584 (+9.0%)
Unfortunately our new algorithm does end up costly. This seems to mainly
be due to the extra altn-specific logic, as well as the more complicated
pruning logic. Maybe the pruning logic deserves more work?
Still, the value is having an actually correct algorithm. And thanks to
altns, we have much stronger proofs over how range operations affect the
underlying 2-3-4 tree balance.
|
||
|
Christopher Haster
|
7375172148 |
rbyd-rr: Cleaned up diverged+pruning interactions
This mainly cleans up the lingering/prune goto noodle soup. Which
duplicates a bit of code but allows for some forward progress cleaning
things up.
Some things to note:
- We can preserve the coloring of diverging red nodes, at least as long
as the non-diverging red alt occurs before the diverging black alt.
.-----> .---b->
| .-b-> | '-> rm me
| | '-> rm me | .--->
r-b---> r-b--->
^ ^
diverging, can preserve diverging, can't preserve currently
Note this only affects the upper divering path. The lower diverging
path doesn't care about colors until the diverging alt is found.
This isn't perfect. Recoloring some diverging red alts _can_ result in
unbalancing the tree by +-1 every range operation. Though this is at
least a significant improvement over the previous +-2x every range
operation in the previous algorithm.
Still, it may be worth further work in the future to eliminate this
+-1 unbalancing caused by the diverging alt itself. Unfortunately the
interactions with red flips and yellow splits get quite tricky...
Note also that maintaining _perfect_ balance during range operations
is impossible as long as we need to propagate yellow recolorings
tail-recursively. Consider what should happen if both paths from the
diverging alt were yellow nodes...
- We can make diverged pruning implicit by simply trimming the alts from
the rid/tag bounds so they appear unreachable to the common pruning
logic.
Note this only works because diverging pruning is eager and only
prunes outward-facing alts.
|
||
|
Christopher Haster
|
5858fd7c8b |
rbyd-rr: Consolidated all pruning before flips and stuff
This adds lfsr_tag_prune to compliment lfsr_tag_prune2, and deduplicates all of the reachability-related pruning logic to before any tree mutation occurs for red/black flips, yellow splits, etc. The previous, post-flips logic was arguably simpler and more mistake-proof, but moving pruning logic pre-flips ensures we don't miss any prunability due to yellow splits. This is the reason diverged pruning _must_ occur pre-flips. And code deduplication is always nice. Note lfsr_tag_prune is _not_ the same as lfsr_tag_unreachable it replaces. The prune functions check for both unreachability and only-reachability to determine if an alt should be pruned. |
||
|
Christopher Haster
|
0b6e2b243a |
rbyd-rr: Fixed unreachable red alts not being pruned
It turns out we were never pruning unreachable red alts. I thought we
were because of lfsr_tag_prune2 and related logic, but this was custom
tailored for the specific unreachable patterns created by yellow splits,
and is insufficient for all unreachable alts.
Consider this unreachable altbgt 0x300 (the second one):
altbgt 0x300 ----------> altbgt 0x300
altrle 0x200 -----.----> altrgt 0x300 <-- unreachable,
altbgt 0x300 -----'----> altble 0x200 should have
null => tag 0x100 been pruned
append
tag 0x100
Our prune logic doesn't catch this because the altbgt is pointing a
different direction than the altrle we end up taking.
This wasn't an issue for earlier range-removal algorithms, since we have
a separate explicit check for diverged pruning to avoid weight ambiguity
issues. Black altas were also not an issue because this logic does catch
unreachable black alts, which are a bit easier. But now that we are
emitting intentionally unreachable red alts with the expectation that
they will be cleaned up by our pruning logic, this is a bit of a
problem...
The solution here is to check for unreachable alts after red flips. This
duplicates quite a bit of code but avoids the logical complexity of
figuring out reachability in all the permutations of red 2-3 nodes.
|
||
|
Christopher Haster
|
a5999c892b |
rbyd-rr: Made remove leaf-splits red
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. |
||
|
Christopher Haster
|
5269f79431 |
rbyd-rr: Preliminary altn collapsing is working
This is the important part of the new range-removal algorithm:
reclaiming altns on yellow splits. This is what allows new alts to reuse
the old tree structure, otherwise we're just adding useless alts for no
reason:
.-----> a .---> a .-> a
| .---> b .-y-r-> b .-b-> b
| | | .-> c | | .-> c | | .-> c
b-b-y-r-b-> d => b-b---b-> d => b-b-b-> d
ysplit rprune
reclaim altn (eventually)
A few more corner cases need to be hammered out, but balance already
shows a noticable improvement.
|
||
|
Christopher Haster
|
2dcde5579b |
Fixed issue with test.py/bench.py -f/--fail not killing runners
While the -f/--fail logic was correctly terminating the test.py/bench.py runner thread, it was not terminating the actual underlying test process. This was causing test.py/bench.py to hang until the test runner completed all pending tests, which could take quite some time. This wasn't noticed earlier because test.py/bench.py still reports the test as failed, and most uses of -f/--fail involve specifying a specific test case, which usually terminates quite quickly. What's more interesting is this termination logic was copied from the handling of ctrl-C/SIGINT/KeyboardInterrupt, but this issue is not present there because SIGINT would be sent to all processes in the process tree, terminating the child process anyways. Fixed by adding an explicit proc.kill() to test.py/bench.py before tearing down the runner thread. |
||
|
Christopher Haster
|
74f4ad8669 |
rbyd-rr: More cleanup around diverged pruning, common goto, no more d_prune
- Dropped d_prune cludge! Thanks to d_tag being properly derived from lower_tag, it can no longer be null (though it may point to null and become an altn). This means we will always have something to replace the diverging-alt with. So no more concerns about a following yellow node splitting and pushing up a red into who-knows-what. The diverged-replacement-alt will always be able to eat this. - Bluntly deduplicated the common pruning logic into its own goto destination. It's interesting to note this should probably be a separate function (yes yes, gotos bad, blablabla), but the amount of appendattr specific context that is needed makes this a bit difficult. To make this work without the previous fallthrough this needed a way to continue appendattr without fetching a new alt. The "lingering" goto destination accomplishes this. Though this is starting to look like goto soup again... |
||
|
Christopher Haster
|
6b3730723e |
rbyd-rr: Cleaned up appendattr diverged handling a bit
- Made diverging-lower pruning its own case to simplify the pruning logic, though this does lead to a bit more code duplication... - Duplicated the diverging-alt pruning logic to simplify/give more control to the diverging-alt case. No more diverged_this_alt hack. Note that d_tag can no longer be null now that we have it deriving from lower_rid correctly. Maybe this can be deduplicated better (diverged-pruning be implicitly controlled by lower/upper bounds?), but I sort of want to get things just working first. |
||
|
Christopher Haster
|
8a646d5b8e |
Added dbgtag.py for easy tag decoding on the command-line
Example: $ ./scripts/dbgtag.py 0x3001 cksum 0x01 dbgtag.py inherits most of crc32c.py's decoding options. The most useful probably being -x/--hex: $ ./scripts/dbgtag.py -x e1 00 01 8a 09 altbgt 0x100 w1 -1162 dbgtag.py also supports reading from a block device if either -b/--block-size or --off are provided. This is mainly for consistency with the other dbg*.py scripts: $ ./scripts/dbgtag.py disk -b4096 0x2.1e4 bookmark w1 1 This should help when debugging and finding a raw tag/alt in some register. Manually decoding is just an unnecessary road bump when this happens. |
||
|
Christopher Haster
|
fe772e08cd |
Added dbgcat.py for extracted raw data from block devices
dbgcat.py is basically the same as dbgblock.py except:
- dbgcat.py pipes the block's contents directly to stdout.
The intention is to enable external tools with Unix pipes while
letting dbgcat.py take care of block address decoding:
$ ./scripts/dbgcat.py disk -b4096 0x1.8 -n8 | grep littlefs
littlefs
- Unlike dbgblock.py, dbgcat.py accepts multiple block addresses,
concatenating the data that resides at those blocks.
This is dbgCAT.py after all.
|
||
|
Christopher Haster
|
9905bd397a |
Extended crc32c.py to support hex sequences and strings
So now the following forms are supported: $ ./scripts/crc32c.py -x 41 42 43 44 fb9f8872 $ ./scripts/crc32c.py -s abcd fb9f8872 $ echo '00: 41 42 43 44' | xxd -r | ./scripts/crc32c.py fb9f8872 Hopefully this will make crc32c.py more useful. It hasn't seen very much use, though that may just be because of the difficulty marshalling data into a format crc32c.py can operate on. That and dbgblock.py's -x/--cksum flag covering one of the main use cases. |
||
|
Christopher Haster
|
54a03cfe3b |
Enabled both pruning/non-pruning dbg reprs, -t/--tree and -R/--rbyd
Now that altns/altas are more important structurally, including them in
our dbg script's tree renderers is valuable for debugging. On the other
hand, they do add quite a bit of visual noise when looking at large
multi-rbyd trees topologically.
This commit gives us the best of both worlds by making both tree
renderings available under different options:
-t/--tree, a simplified rbyd tree renderer with altn/alta pruning:
.-> 0 reg w1 4
.-+-> uattr 0x01 2
| .-> uattr 0x02 2
.---+-+-> uattr 0x03 2
| .-> uattr 0x04 2
| .-+-> uattr 0x05 2
| .-+---> uattr 0x06 2
+-+-+-+-+-> 1 reg w1 4
| | '-> 2 reg w1 4
| '---> uattr 0x01 2
'---+-+-+-> uattr 0x02 2
| | '-> uattr 0x03 2
| '-+-> uattr 0x04 2
| '-> uattr 0x05 2
| .-> uattr 0x06 2
| .-+-> uattr 0x07 2
| | .-> uattr 0x08 2
'-+-+-> uattr 0x09 2
-R/--rbyd, a full rbyd tree renderer:
.---> 0 reg w1 4
.---+-+-> uattr 0x01 2
| .---> uattr 0x02 2
.-+-+-+-+-> uattr 0x03 2
| .---> uattr 0x04 2
| .-+-+-> uattr 0x05 2
| .-+---+-> uattr 0x06 2
+---+-+-+-+-+-> 1 reg w1 4
| | '-> 2 reg w1 4
| '-----> uattr 0x01 2
'-+-+-+-+-+-+-> uattr 0x02 2
| | '---> uattr 0x03 2
| '---+-+-> uattr 0x04 2
| '---> uattr 0x05 2
| .---> uattr 0x06 2
| .-+-+-> uattr 0x07 2
| | .-> uattr 0x08 2
'-----+---+-> uattr 0x09 2
And of course -B/--btree, a simplified B-tree renderer (more useful for
multi-rbyds):
+-> 0 reg w1 4
| uattr 0x01 2
| uattr 0x02 2
| uattr 0x03 2
| uattr 0x04 2
| uattr 0x05 2
| uattr 0x06 2
|-> 1 reg w1 4
'-> 2 reg w1 4
uattr 0x01 2
uattr 0x02 2
uattr 0x03 2
uattr 0x04 2
uattr 0x05 2
uattr 0x06 2
uattr 0x07 2
uattr 0x08 2
uattr 0x09 2
|
||
|
Christopher Haster
|
abe68c0844 |
rbyd-rr: Reworking rbyd range removal to try to preserve rby structure
This is the start of (yet another) rework of rybd range removals, this
time in an effort to preserve the rby structure that maps to a balanced
2-3-4 tree. Specifically, the property that all search paths have the
same number of black edges (2-3-4 nodes).
This is currently incomplete, as you can probably tell from the mess,
but this commit at least gets a working altn/alta encoding in place
necessary for representing empty 2-3-4 nodes. More on that below.
---
First the problem:
My assumption, when implementing the previous range removal algorithms,
was that we only needed to maintain the existing height of the tree.
The existing rbyd operations limit the height to strictly log n. And
while we can't _reduce_ the height to maintain perfect balance, we can
at least avoid _increasing_ the height, which means the resulting tree
should have a height <= log n. Since our rbyds are bounded by the
block_size b, this means worst case our rbyd can never exceed a height
<= log b, right?
Well, not quite.
This is true the instance after the remove operation. But there is an
implicit assumption that future rbyd operations will still be able to
maintain height <= log n after the remove operation. This turns out to
not be true.
The problem is that our rbyd appends only maintain height <= log n if
our rby structure is preserved. If the rby structure is broken, rbyd
append assumes an rby structure that doesn't exist, which can lead to an
increasingly unbalanced tree.
Consider this happily balanced tree:
.-------o-------. .--------o
.---o---. .---o---. .---o---. |
.-o-. .-o-. .-o-. .-o-. .-o-. .-o-. |
.o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. |
a b c d e f g h i j k l m n o p => a b c d e f g h i
'------+------'
remove
After a range removal it looks pretty bad, but note the height is still
<= log n (old n not the new n). We are still <= log b.
But note what happens if we start to insert attrs into the short half of
the tree:
.--------o
.---o---. |
.-o-. .-o-. |
.o. .o. .o. .o. |
a b c d e f g h i
.-----o
.--------o .-+-r
.---o---. | | | |
.-o-. .-o-. | | | |
.o. .o. .o. .o. | | | |
a b c d e f g h i j'k'l'
.-------------o
.---o .---+-----r
.--------o .-o .-o .-o .-+-r
.---o---. | | | | | | | | | |
.-o-. .-o-. | | | | | | | | | |
.o. .o. .o. .o. | | | | | | | | | |
a b c d e f g h i j'k'l'm'n'o'p'q'r'
Our right side is generating a perfectly balanced tree as expected, but
the left side is suddenly twice as far from the root! height(r')=3,
height(a)=6!
The problem is when we append l', we don't really know how tall the tree
is. We only know l' has one black edge, which assuming rby structure is
preserved, means all other attrs must have one black edge, so creating a
new root is justified.
In reality this just makes the tree grow increasingly unbalanced,
increasing the height of the tree by worst case log n every range
removal.
---
It's interesting to note this was discovered while debugging
test_fwrite_overwrite, specifically:
test_fwrite_overwrite:1181h1g2i1gg2l15o10p11r1gg8s10
It turns out the append fragments -> delete fragments -> append/carve
block + becksum loop contains the perfect sequence of attrs necessary to
turn this tree inbalance into a linked-list!
.-> 0 data w1 1
.-b-> 1 data w1 1
| .-> 2 data w1 1
.-b-b-> 3 data w1 1
| .-> 4 data w1 1
| .-b-> 5 data w1 1
| | .-> 6 data w1 1
.---b-b-b-> 7 data w1 1
| .-> 8 data w1 1
| .-b-> 9 data w1 1
| | .-> 10 data w1 1
| .-b-b-> 11 data w1 1
| .-b-----> 12 data w1 1
.-y-y-------> 13 data w1 1
| .-> 14 data w1 1
.-y---------y-> 15 data w1 1
| .-> 16 data w1 1
.-y-----------y-> 17 data w1 1
| .-> 18 data w1 1
.-y-------------y-> 19 data w1 1
| .-> 20 data w1 1
.-y---------------y-> 21 data w1 1
| .-> 22 data w1 1
.-y-----------------y-> 23 data w1 1
| .-> 24 data w1 1
.-y-------------------y-> 25 data w1 1
| .---> 26 data w1 1
| | .-> 27-2047 block w2021 10
b-------------------r-b-> becksum 5
Note, to reproduce this you need to step through with a breakpoint on
lfsr_bshrub_commit. This only shows up in the file's intermediary btree,
which at the time of writing ends up at block 0xb8:
$ ./scripts/test.py \
test_fwrite_overwrite:1181h1g2i1gg2l15o10p11r1gg8s10 \
-ddisk --gdb -f
$ ./scripts/watch.py -Kdisk -b \
./scripts/dbgrbyd.py -b4096 disk 0xb8 -t
(then b lfsr_bshrub_commit and continue a bunch)
---
So, we need to preserve the rby structure.
Note pruning red/yellow alts is not an issue. These aren't black, so we
aren't changing the number of black edges in the tree. We've just
effectively reduced a 3/4 node into a 2/3 node:
.-> a
.---b-> b .-> a <- 2 black
| .---> c .-b-> b
| | .-> d | .-> c
b-r-b-> e <- rm => b-b-> d <- 2 black
The tricky bit is pruning black alts. Naively this changes the number of
black edges/2-3-4 nodes in the tree, which is bad:
.-> a
.-b-> b .-> a <- 2 black
| .-> c .-b-> b
b-b-> d <- rm => b---> c <- 1 black
It's tempting to just make the alt red at this point, effectively
merging the sibling 2-3-4 node. This maintains balance in the subtree,
but still removes a black edge, causing problems for our parent:
.-> a
.-b-> b .-> a <- 3 black
| .-> c .-b-> b
.-b-b-> d | .-> c
| .-> e .-b-b-> d
| .-b-> f | .---> e
| | .-> g | | .-> f
b-b-b-> h <- rm => b-r-b-> g <- 2 black
In theory you could propagate this all the way up to the root, and this
_would_ probably give you a perfect self-balancing range removal
algorithm... but it's recursive... and littlefs can't be recursive...
.-> s
.-b-> t .-> s
| .-> u .-----b-> t
.-b-b-> v | .-> u
| .-> w | .---b-> v
| .-b-> x | | .---> w
| | | | .-> y | | | | | | | .-> x
b-b- ... b-b-b-> z <- rm => r-b-r-b- ... r-b-r-b-> y
So instead, an alternative solution. What if we allowed black alts that
point nowhere? A sort of noop 2-3-4 node that serves only to maintain
the rby structure?
.-> a
.-b-> b .-> a <- 2 black
| .-> c .-b-> b
b-b-> d <- rm => b-b-> c <- 2 black
I guess that would technically make this 1-2-3-4 tree.
This does add extra overhead for writing noop alts, which are otherwise
useless, but it seems to solve most of our problems: 1. does not
increase the height of the tree, 2. maintains the rby structure, 3.
tail-recursive.
And, thanks to the preserved rby structure, we can say that in the worst
case our rbyds will never exceed height <= log b again, even with range
removals.
If we apply this strategy to our original example, you can see how the
preserved rby structure sort of "absorbs" new red alts, preventing
further unbalancing:
.-------o-------. .--------o
.---o---. .---o---. .---o---. o
.-o-. .-o-. .-o-. .-o-. .-o-. .-o-. o
.o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. o
a b c d e f g h i j k l m n o p => a b c d e f g h i
'------+------'
remove
Reinserting:
.--------o
.---o---. o
.-o-. .-o-. o
.o. .o. .o. .o. o
a b c d e f g h i
.----------------o
.---o---. o
.-o-. .-o-. .------o
.o. .o. .o. .o. .o. .-+-r
a b c d e f g h i j'k'l'm'
.----------------------------o
.---o---. .-------------o
.-o-. .-o-. .---o .---+-----r
.o. .o. .o. .o. .-o .-o .-o .-o .-+-r
a b c d e f g h i j'k'l'm'n'o'p'q'r's'
Much better!
---
This commit makes some big steps towards this solution, mainly codifying
a now-special alt-never/alt-always (altn/alta) encoding to represent
these noop 1 nodes.
Technically, since null (0) tags are not allowed, these already exist as
altle 0/altgt 0 and don't need any extra carve-out encoding-wise:
LFSR_TAG_ALT 0x4kkk v1dc kkkk -kkk kkkk
LFSR_TAG_ALTN 0x4000 v10c 0000 -000 0000
LFSR_TAG_ALTA 0x6000 v11c 0000 -000 0000
We actually already used altas to terminate unreachable tags during
range removals, but this behavior was implicit. Now, altns have very
special treatment as a part of determining bounds during appendattr
(both unreachable gt/le alts are represented as altns). For this reason
I think the new names are warranted.
I've also added these encodings to the dbg*.py scripts for, well,
debuggability, and added a special case to dbgrby.py -j to avoid
unnecessary altn jump noise.
As a part of debugging, I've also extended dbgrbyd.py's tree renderer to
show trivial prunable alts. Unsure about keeping this. On one hand it's
useful to visualize the exact alt structure, on the other hand it likely
adds quite a bit of noise to the more complex dbg scripts.
The current state of things is a mess, but at least tests are passing!
Though we aren't actually reclaiming any altns yet... We're definitely
_not_ preserving the rby structure at the moment, and if you look at the
output from the tests, the resulting tree structure is hilarious bad.
But at least the path forward is clear.
|
||
|
Christopher Haster
|
9b4e1b4cb7 |
Replace assert(!err) with assert(err == 0) in tests
This plays better with prettyasserts.py, which prints the err value on failure. We _could_ extend prettyasserts.py to print the contents of !err patterns, but this risks making the error message more confusing when the target is an actual boolean expression. Keep in mind prettyasserts.py is purely syntactical and doesn't really know the expression's type. |
||
|
Christopher Haster
|
16ca642508 |
Simplified the diverged state machine in lfsr_rbyd_appendattr
Just made d_pruned its own variable. Encoding d_pruned into the state
machine is overkill.
We're not on the hot-path, so stack usage is not a premium, and even if
we were this is a single bool that could probably fit in some padding
somewhere. And the compiler is going to likely be better at optimizing
with a simpler encoding.
Code changes:
code stack
before: 33988 2864
after: 33968 (+0.0%) 2864 (+0.0%)
|
||
|
Christopher Haster
|
531c2bcc4c |
Quieted test.py/bench.py status when stdout is aimed at stdout
This is a condition for specifically the -O- pattern. Doing anything fancier would be too much, so anything clever such as -O/dev/stdout will still be clobbered. This was a common enough pattern and the status updates clobbering stdout was annoying enough that I figured this warranted a special case. |
||
|
Christopher Haster
|
76593711ab |
Added -f/--fail to test.py/bench.py
This just tells test.py/bench.py to pretend the test failed and trigger any conditional utilities. This can be combined with --gdb to easily inspect a test that isn't actually failing. Up until this point I've just been inserting assert(false) when needed, which is clunky. |
||
|
Christopher Haster
|
62de865103 |
Eliminated null tag reachability in dbg scripts
This was throwing off tree rendering in dbglfs.py, we attempt to lookup the null tag because we just want to first tag in the tree to stitch things together. Null tag reachability is tricky! You only notice if the tree happens to create a hole, which isn't that common. I think all lookup implementations should have this max(tag, 1) pattern from now on to avoid this. Note that most dbg scripts wouldn't run into this because we usually use the traversal tag+1 pattern. Still, the inconsistency in impl between the dbg scripts and lfs.c is bad. |
||
|
Christopher Haster
|
3eb4ccdde7 |
Fixed block/becksum related tree rendering in dbglfs.py
The were a couple issues mixing high-level and low-level bptrs
representations:
1. The high-level vs low-level block representation needed to have
ordering priority over the actual tag in order for inner-node tree
renderings to make sense.
2. We need to flatten all tags to BLOCK/DATA/other data tags when _not_
rendering inner-nodes so interleaved becksums/other util tags don't
mess with the tree rendering.
Now things look like this:
littlefs v0.0 4096x256 0x{0,1}.a0c, rev 15, weight 0.512
{0000,0001}: -1.1 hello reg 1113, btree 0x27.8d8
0000.0a11: + 0-1112 btree w1113 9
0027.08d8: | .-+ 0-1112 block w1113 11
'-+-| > becksum 5
'-> 0-1112 block w1113 0x95.0 1113
Maybe a bit weird looking at first, but correct.
|
||
|
Christopher Haster
|
3d61030ccc |
Mark the on-disk version as experimental
Just in case... |
||
|
Christopher Haster
|
9cf115685b |
Don't duplicate config across all mroots, only magic
So, duplicating the config across multiple mroots allows a better
chance of manual recovery if things go wrong, right?
.--------. .--------.
.|littlefs|->|littlefs|
||bs=4096 | ||bs=4096 |
||bc=256 | ||bc=256 |
||crc32c | ||root dir|
|| | ||crc32c |
|'--------' |'--------'
'--------' '--------'
Well, this was the original thinking. But now I'm starting to think the
duplicated config isn't actually all that useful:
1. The config may be out-of-date, since only the last mroot is mutable.
This is becoming more common as littlefs evolves (on-disk version
bumps, compat flag changes, fs_grow, etc).
2. The most important bit of information is where the mtree is. And this
information is _only_ available on the last mroot because of
mutability requirements.
At the very least, all mroots point to the mtree (not Rome). So
finding the mtree may not be that hard if you find any mroot. But
this also gives you all the config sooo...
3. gstate is going to be hard (impossible?) to reconstruct anyways.
Though for reading this may only be an issue for interrupted grms.
4. Let's be honest, manual recovery is not going to be a common
occurence for these devices.
Point 1. is a the main issue and actually highlights a real risk with
duplicated config: It's easy to pick up out-of-date config.
In other implementations this risks easy mistakes that are hard to
notice until complex filesystem states. Assuming the first mroot
contains up-to-date config for the obvious example.
Even in our current implementation, duplicated config already poses some
tricky hard-to-get-right problems. What happens if we run into an
unknown compat flag on a not-last mroot? Hint, our implementation was
broken!
---
So this commit changes mroot extension to _not_ duplicate config, but
instead just rewrite the magic string and mroot chain to the new mroot
anchor:
.--------. .--------.
.|littlefs|->|littlefs|
||crc32c | ||bs=4096 |
|| | ||bc=256 |
|| | ||root dir|
|| | ||crc32c |
|'--------' |'--------'
'--------' '--------'
This leads to a nice simplification in lfsr_mdir_commit, so that's a
plus.
It does make lfsr_mount a bit trickier, since we need to figure out which
mroot is the last mroot before checking for config. But we would need
something trickier in lfsr_mount anyways to handle the above out-of-date
issues anyways.
The current implementation just does a redundant mroot lookup to figure
out if the current mroot is the last mroot. In theory this could be
avoided, but I couldn't figure out how to without making the code
unreasonably complex (lfsr_mount is already intertwined with
lfsr_traversal_read).
The end result is a bit of code and stack savings, thanks to
lfsr_mdir_commit being on the stack-depth hot-path (deep-path?):
code stack
before: 34060 2880
after: 33996 (-0.2%) 2864 (-0.6%)
It's also worth noting that there are plans to add block-level
redundancy at some point. Maybe it's best to leave recovering from
missing blocks to block-level redundancy which is actually designed for
this, and let the mroot chain do what the mroot chain does best:
allowing the mroots to participate in wear-leveling.
|
||
|
Christopher Haster
|
c71725d627 |
Added type info to dsize comments
This is mostly just a lot of leb128s, though we do use be16 for tags and le32 for cksums and revision counts. There are several places we use single-byte leb128s, which really are u8s with the top bit reserved. Still, notating this as leb128 indicates that the top bit really is reserved, even if you don't need full leb128 encoding/decoding in practice. |
||
|
Christopher Haster
|
2564100eaa |
Tweaked name/sizelimit parsing for consistency, default to 0xff/0x7fffffff
This is mostly just for consistency with changes to parsing other parts
of the fs config attrs.
This changes name/size limits to default to namelimit=0xff and
sizelimit=0x7fffffff. These are reasonable defaults for 32-bit systems,
which was the original use case for littlefs. Though, with the diversity
of embedded device, I suspect these will be overridden more often than
not. For this reason the 0xff/0x7fffffff case is not treated specially
during lfs_format and these limits are always written. Though this may
change in the future.
The intention behind these defaults is to align with other limits that
may be introduced in the future. Any new artificial limits will
necessarily require defaulting to their existing values for backwards
compatibility, so hopefully this allows all limits to be handled
consistently.
If a future use-case-specific implementation of littlefs can benefit
from assuming these defaults, that's a nice plus.
Name/size-limit attr encodings:
.---+---+---+---. tag (0x000c): 1 be16 2 bytes
| x000c | 0 |siz| weight (0): 1 leb128 1 byte
+---+---+---+---+ size: 1 leb128 1 byte
| name_limit | name_limit: 1 leb128 <=4 bytes
'---+- -+- -+- -' total: <=8 bytes
.---+---+---+---. tag (0x000d): 1 be16 2 bytes
| x000d | 0 |siz| weight (0): 1 leb128 1 byte
+---+---+---+---+- -. size: 1 leb128 1 byte
| size_limit | size_limit: 1 leb128 <=5 bytes
'---+- -+- -+- -+- -' total: <=9 bytes
Code changes:
code stack
before: 34040 2880
after: 34060 (+0.1%) 2880 (+0.0%)
|
||
|
Christopher Haster
|
9366674416 |
Replaced separate BLOCKSIZE/BLOCKCOUNT attrs with single GEOMETRY attr
This saves a bit of rbyd overhead, since these almost always come
together.
Perhaps more interesting, it carves out space for storing mroot-anchor
redundancy information. This uses the lowest two bits of the GEOMETRY
tag to indicate how many redundant blocks belong to the mroot-anchor:
LFSR_TAG_GEOMETRY 0x0008 v--- ---- ---- 1-rr
This solves a bit of a hole in our redundancy encoding. The plan is for
this info to be stored in the lowest two bits of every pointer, but the
mroot-anchor doesn't really have a pointer.
Though this is just future plans. Right now the redundancy information
is unused. Current implementations should use the GEOMETRY tag 0x0009,
which you may notice implied redundancy level-1. This matches our
current 2-block per mdir default.
Geometry attr encoding:
.---+---+---+---. tag (0x0008+r): 1 be16 2 bytes
|x0008+r| 0 |siz| weight (0): 1 leb128 1 byte
+---+---+---+---+ size: 1 leb128 1 byte
| block_size | block_size: 1 leb128 <=4 bytes
+---+- -+- -+- -+- -.
| block_count | block_count: 1 leb128 <=5 bytes
'---+- -+- -+- -+- -' total: <=13 bytes
Code changes:
code stack
before: 34092 2880
after: 34040 (-0.2%) 2880 (+0.0%)
|
||
|
Christopher Haster
|
796be705ac |
Simplified how we check on-disk versions a bit
We don't really need to do full leb128 decoding since our version
numbers are unlikely to ever actually exceed v127.127.
Worst case, if they do, the version that exceeds v127.127 can switch to
using leb128 decoding without breaking backwards compatibility.
Version attr encoding:
.---+---+---+---+---+---. tag (0x0004): 1 be16 2 bytes
| x0004 | 0 | 2 |maj|min| weight (0): 1 leb128 1 byte
'---+---+---+---+---+---' size (2): 1 leb128 1 byte
major_version: 1 leb128 1 byte
minor_version: 1 leb128 1 byte
total: 6 bytes
Code changes:
code stack
before: 34124 2880
after: 34092 (-0.1%) 2880 (+0.0%)
|
||
|
Christopher Haster
|
130281ac05 |
Reworked compat flags a bit
Now with a bit more granularity for possibly-future-optional on-disk
data structures:
LFSR_RCOMPAT_NONSTANDARD 0x0001 ---- ---- ---- ---1 (reserved)
LFSR_RCOMPAT_MLEAF 0x0002 ---- ---- ---- --1-
LFSR_RCOMPAT_MSHRUB 0x0004 ---- ---- ---- -1-- (reserved)
LFSR_RCOMPAT_MTREE 0x0008 ---- ---- ---- 1---
LFSR_RCOMPAT_BSPROUT 0x0010 ---- ---- ---1 ----
LFSR_RCOMPAT_BLEAF 0x0020 ---- ---- --1- ----
LFSR_RCOMPAT_BSHRUB 0x0040 ---- ---- -1-- ----
LFSR_RCOMPAT_BTREE 0x0080 ---- ---- 1--- ----
LFSR_RCOMPAT_GRM 0x0100 ---- ---1 ---- ----
LFSR_WCOMPAT_NONSTANDARD 0x0001 ---- ---- ---- ---1 (reserved)
LFSR_OCOMPAT_NONSTANDARD 0x0001 ---- ---- ---- ---1 (reserved)
This adds a couple reserved flags:
- LFSR_*COMPAT_NONSTANDARD - This flag will never be set by a standard
version of littlefs. The idea is to allow implementations with
non-standard extensions a way to signal potential compatibility issues
without worrying about future compat flag conflicts.
This is limited to a single bit, but hey, it's not like it's possible
to predict all future extensions.
If a non-standard extension needs more granularity, reservations of
standard compat flags can always be requested, even if they don't end
up implemented in standard littlefs. (Though such reservations will
need a strong motivation, it's not like these flags are free).
- LFSR_RCOMPAT_MSHRUB - In theory littlefs supports a shrubbed mtree,
where the root is inlined into the mroot. But in practice this turned
out to be more complicated than it was worth. Still, a future
implementation may find an mshrub useful, so preserving a compat flag
for such a case makes sense.
That being said, I have no plans to add support for mshrubs even in
the dbg scripts.
I would like the expected feature-set for debug tools to be
well-defined, but also conservative. This gets a bit tricky with
theoretical features like the mshrubs, but until mshrubs are actually
implemented in littlefs, I would like to consider them non-standard.
The implication of this is that, while LFSR_RCOMPAT_MSHRUB is
currently "reserved", it may be repurposed for some other meaning in
the future.
These changes also rename *COMPATFLAGS -> *COMPAT, and reorder the tags
by decreasing importance. This ordering seems more valuable than the
original intention of making rcompat/wcompat a single bit flip.
Implementation-wise, it's interesting to note the internal-only
LFSR_*COMPAT_OVERFLOW flag. This gets set when out-of-range bits are set
on-disk, and allows us to detect unrepresentable compat flags without
too much extra complexity.
The extra encoding/decoding overhead does add a bit of cost though:
code stack
before: 33944 2880
after: 34124 (+0.5%) 2880 (+0.0%)
|
||
|
Christopher Haster
|
76721638db |
Renamed lfsr_rbyd_p_red -> lfsr_rbyd_p_recolor
Recolor is actually a verb for one. |
||
|
Christopher Haster
|
85e43d51ba |
Found a balance-preserving solution to tail-recursive range recoloring
It feels a bit clumsy, but by using an additional bit of state to keep
track of if the last alt was pruned, we can cancel recolorings that may
risk recursion.
If we look at how this plays out on the underlying 2-3-4 tree:
.-----. .-------. .-------. .-------.
|.a.h.| |.a.c.h.| |.a.c.h.| |.a.c.h.|
'|-|-|' '|-|-|-|' '|-|-|-|' '|-|-|-|'
| .-' '-. .--' '--. .-' '-.
v v v v v v v
.-------. .---. .---. .---. .-------. .---. .---.
|.b.c.g.| => |.b.| |.g.| => |.b.| |.d.e.f.| => |.b.| |.e.|
'|-|-|-|' '|-|' '|-|' '|-|' '|-|-|-|' '|-|' '|-|'
| x | x .-' '-.
v v v v
.-------. .-------. .---. .---.
|.d.e.f.| |.d.e.f.| |.d.| |.f.|
'|-|-|-|' '|-|-|-|' '|-|' '|-|'
Note the important property that no nodes ended up at a height _worse_
than where they started.
It's interesting to note this is equivalent to splitting the nodes
_before_ prunning:
.-----. .-------. .-------. .-------.
|.a.h.| |.a.c.h.| |.a.c.h.| |.a.c.h.|
'|-|-|' '|-|-|-|' '|-|-|-|' '|-|-|-|'
| .-' '-. .-' '--. .-' '-.
v v v v v v v
.-------. .---. .---. .---. .-----. .---. .---.
|.b.c.g.| => |.b.| |.g.| => |.b.| |.e.g.| => |.b.| |.e.|
'|-|-|-|' '|-|' '|-|' '|-|' '|-|-|' '|-|' '|-|'
| x | x .---' | x .-' '-.
v v v v v v
.-------. .-------. .---. .---. .---. .---.
|.d.e.f.| |.d.e.f.| |.d.| |.f.| |.d.| |.f.|
'|-|-|-|' '|-|-|-|' '|-|' '|-|' '|-|' '|-|'
Which is probably why most of our 2-3-4 tree invariants hold.
In the actual implementation, we encode the current pruned state as a
part of our diverging state machine, since we don't non-trivially prune
outside of diverging trunks.
This ends up with the following, slightly-extended, diverging state
machine:
diverge possible? diverge not possible?
| |
v |
DIVERGINGLOWER-------------------. |
| | |
v v v
DIVERGEDLOWER<->PRUNEDLOWER NOTDIVERGING
| .------------' |
v v |
DIVERGINGUPPER |
| |
v |
DIVERGEDUPPER<->PRUNEDUPPER |
'------------. | .----------'
v v v
leaf stuff
Writing out the state machine like this actually highlights the slightly
annoying transition from PRUNEDUPPER to leaf stuff, which was buggy in
the first impl.
We also encode some common information (lower/upper, pruned, etc) in the
state machine's bit encoding to try to avoid too many if statements.
Though this impl does seem a bit heavy handed.
The additional complexity results in of course more code cost, but as a
trade-off our range recoloring should be a bit more sturdy and provably
preserves the h=2log2(b) worst case height of our tree:
code stack
broken recoloring: 33880 2880
unbalanced recoloring: 33912 (+0.1%) 2880 (+0.0%)
balanced recoloring: 33944 (+0.2%) 2880 (+0.0%)
|
||
|
Christopher Haster
|
0a89d0c254 |
Fixed recoloring tail-recursion violations during range removals
I spoke too soon and made a mistake when reenabling color preservation
during range removals.
I assumed, that thanks to replacing the diverging alt with a new black
alt for stitching together diverging trunks, we would avoid the issue
where a deleted diverging alt violates our rbyd's tail-recursive
recoloring invariant.
Unfortunately, this is not the case. All the stitching alt did was make
this violation more difficult to reach, but still reachable. Arguable a
worse situation.
Now, for this violation to happen, in addition to all of the other
requirements, we need the lower-diverging trunk to become empty.
This is the only case where we have no stitching alt, because we don't
need to stitch an empty trunk. Which means if the upper-diverging trunk
has yellow nodes both before and after the diverging alt, our
tail-recursive recoloring invariant can break.
Here's an example:
.-------------r-------------.
.-o-. .---+---y----. .-o-.
.o. .o. .o. .o. .o. .-y-+-. .o. .o.
a a a a a a a a c c c e e e e e e e
'--+--'
remove
Again, this doesn't capture the alt-layout, which _is_ important, so
here's the dbgrbyd.py view:
.-> aa .-> aa
.-b-> a .-b-> a
| .-> a | .-> a
.-----------b-b-> a .-------b-b-> a
| .-> a | .-> a
| .---------b-> a | .-b-> a
| | .-> a | | .-> a
| | .-------b-> a | .-b-b-> a
r-b-y-r-b-----b-> cc -. => y-y-r-b-----> ee <- two yellows!
| | '-> c + rm | '-----b-> e different dirs!
| | .-> c -' | '-> e should not happen!
| '-y-r-b-> ee | .-> e
| | '---> e | .-b-> e
| '-----> e | | .-> e
| .-> e '-----b-b-> e
| .-b-> e
| | .-> e
'---------b-b-> e
And the steps in our appendattr algorithm that led to this state, which
is insightful:
read <r => [<r]
read >b => [<r >b]
read <r => [<r >b <r]
read <r => [<r >b <r <r]
^--^------ red + red implies yellow
ysplit => [<r >r <b]
reorder => [<r <r >b]
^--^------------- yellow-same-dir invariant held
read >b => [<r <r >b >b]
diverge => [<r <r >b]
read >r => [<r <r >b >r]
read >r => <r [<r >b >r >r]
^-----------^-- our 4-alt fifo for flips/coloring
ysplit => <r [<r >r >b]
reorder => <r [>r >r <b]
^--^---------- yellow-same-dir invariant held
^---^------------- yellow-same-dir invariant NOT held
though 2 yellows is also a problem
The previous commit fixing this bug for the one-pass algorithm may also
be useful.
This tree is now tested in test_rbyd_delete_range_rydye and
test_rbyd_delete_range_rydye_backwards, though only
test_rbyd_delete_range_rydye_backwards reveals the bug, since the bug
requires _specifically_ the lower-diverging trunk to become empty (both
rydy and rydye now have in-order and backwards tests in case of other
chirality issues).
---
Taking a step back, and looking at this bug from a higher-level, the
core of the issue is that we are somewhat arbitrarily deleting nodes
after splitting nodes. This can break our tail-recursive recoloring
invariant.
What the heck is our tail-recursive recoloring invariant?
This is a property of 2-3-4 and greater B-trees, and transitively
red-black and red-black-yellow trees, that allows for tail-recursive,
self-balancing node insertion.
Basically, if you eagerly split any 4-nodes you encounter as you descend
down the tree, you will always be guaranteed to have an open slot in
your parent, so pushing up split nodes (or recoloring) only ever
propagates up a single level:
.-----. .-------. .-------.
|.a.h.| |.a.c.h.| |.a.c.h.|
'|-|-|' '|-|-|-|' '|-|-|-|'
| .-' '-. .-' '--.
v v v v v
.-------. .---. .---. .---. .-----.
|.b.c.g.| => |.b.| |.g.| => |.b.| |.e.g.|
'|-|-|-|' '|-|' '|-|' '|-|' '|-|-|'
| | .-' '-.
v v v v
.-------. .-------. .---. .---.
|.d.e.f.| |.d.e.f.| |.d.| |.f.|
'|-|-|-|' '|-|-|-|' '|-|' '|-|'
If you lazily split, you aren't guaranteed an open slot in your parent,
so you need recursion to solve splits. This is why 2-3 trees, though
self-balancing, are not tail-recursive:
.-----. .-----.
|.a.h.| |.a.h.|
'|-|-|' '|-|-|'
| |
v v
.-------. .'''''''''.
|.b.c.g.| => >.b.c.e.g.< 5!?
'|-|-|-|' '|.|.|.|.|'
| .-' '-.
v v v
.-------. .---. .---.
|.d.e.f.| |.d.| |.f.|
'|-|-|-|' '|-|' '|-|'
But if you are eagerly splitting while also deleting nodes:
.-----. .-------. .-------. .'''''''''.
|.a.h.| |.a.c.h.| |.a.c.h.| 5!? >.a.c.e.h.<
'|-|-|' '|-|-|-|' '|-|-|-|' '|.|.|.|.|'
| .-' '-. .-' '---. .---' | '---.
v v v v v v v v
.-------. .---. .---. .---. .-------. .---. .---. .---.
|.b.c.g.| => |.b.| |.g.| => |.b.| |.d.e.f.| => |.b.| |.d.| |.g.|
'|-|-|-|' '|-|' '|-|' '|-|' '|-|-|-|' '|-|' '|-|' '|-|'
| x | x
v v
.-------. .-------.
|.d.e.f.| |.d.e.f.|
'|-|-|-|' '|-|-|-|'
Suddenly, recursion. This is a problem.
The workaround implemented here is to check during pruning if our parent
may risk recursion, and if so, recolor the last alt so nothing will
break.
This ends up equivalent to the following transformation:
.-----. .-------. .-----. .-----.
|.a.h.| |.a.c.h.| |.a.c.| |.a.c.|
'|-|-|' '|-|-|-|' '|-|-|' '|-|-|'
| .-' '-. .-' '-. .-' '--.
v v v v v v v
.-------. .---. .---. .---. .---. .---. .-----.
|.b.c.g.| => |.b.| |.g.| => |.b.| |.h.| => |.b.| |.e.h.|
'|-|-|-|' '|-|' '|-|' '|-|' '|-|' '|-|' '|-|-|'
| x | x | .-' '-.
v v v v v
.-------. .-------. .-------. .---. .---.
|.d.e.f.| |.d.e.f.| |.d.e.f.| |.d.| |.f.|
'|-|-|-|' '|-|-|-|' '|-|-|-|' '|-|' '|-|'
You may notice this isn't exactly optimal. The >h branch ends up one
level lower, making the balance of the tree off by one. But it at least
ends up with a functional tree.
I may try to find a better solution...
---
The test_rbyd_delete_range_rydy/rydye tests should cover the cases where
a diverging alt is deleted.
I also tried to write tests for the cases where an alt is pruned, the
closest I got is in test_rbyd_delete_range_dryy_backwards, but I
couldn't actually come up with a sequence that would break our rbyds.
In theory it's possible, but it would need this substructure:
.-------> c y-r-b-------> c
y-r-b-y-r-b-> c or | | '-y-r-b-> c
| | | | | | | |
Which, as far as I can tell, can't actually be created with our current
algorithm...
Note the inverse structure:
.---------> c
| .-y-r-b-> c
y-r- | |
Will be pruned before it has a chance to split. So there is no invariant
concerns there. We only have issues when it's the tail alts that get
pruned, because we decide to split before we know if we are pruning or
not. I don't think this can be avoided without additional read-ahead.
Also, even if we could create the above substructure, because we are on
a diverged trunk, and by definition all alts point the same direction,
we would never end up violating our same-dir yellow invariant/assert...
Code changes:
code stack
before: 33880 2880
after: 33912 (+0.1%) 2880 (+0.0%)
|
||
|
Christopher Haster
|
4d90be94f9 |
Preserve coloring during range removals
This is the real kicker of our new-and-improved range removal algorithm.
We can actually preserve the existing tree coloring, and the underlying
rbyd invariants.
Well, sort of. We preserve the red-follows-yellow and black-follows-red
rules, but we don't (can't?) preserve the same-height for all black
edges property.
But note! The new range removal algorithm never creates _new_ black
edges. It can only delete black edges, and otherwise preserves the
structure of the underlying 2-3-4 tree.
This means that while the resulting tree may not be perfectly balanced
with h=2*log2n', where n' is the _new_ number of tags, the resulting tree
_is_ limited to h=2*log2(n) where n is the _old_ number of tags.
When applied to our bounded rbyd, with eventual compaction and
rebalancing, we end up with the guarantee that the rbyd's height will
never exceed h=2*log2(b) where b is the block size, even with arbitrary
range removals.
This is a great result!
---
Note that this algorithm does not suffer from the yellow-diverge-yellow
corner case that was an issue for preserving coloring in the previous
one-pass stitching algorithm. This is because the one-pass algorithm
effectively deleted the diverging alt, breaking the tail-recursive
invariant of the underlying 2-3-4 tree. With the new two-pass algorithm,
we _replace_ the diverging alt with a black stitching alt to stitch
together the diverging trunks, so no tail-recursive invariant breaking.
(Also note even if we could preserve coloring in the one-pass algorithm,
it would still be breaking invariants by introducing new black edges
when it stitches together diverging trunks. Worst case, resulting in ~2x
the height, even when stitching with red alts (The red alt stitching
brings this cost down from ~4x to ~2x worst case due to blanket
recoloring. With yellow alt stitching this could probably be brought
down to ~1.3x, but this would still mean every range removal could be
increasing the height of the tree, which is not great.).)
---
Pruning has to be a bit more complicated now, since we need to be able
to recolor skipped red alts. But other than pruning the cost of
recoloring vs not recoloring is pretty small:
code stack
one-pass, blanket recolor: 33852 2880
two-pass, blanket recolor: 33860 (+0.0%) 2880 (+0.0%)
two-pass, color preserving: 33880 (+0.1%) 2880 (+0.0%)
The non-rigorous random-file-write benchmark I've been using as a litmus
test did not really show any improvements, but in hindsight it might
have been a bit silly to use a uniform distribution of writes to test
for rbyd balancing issues... Building a tree from a uniform distribution
already results in a balanced tree without doing anything!
|
||
|
Christopher Haster
|
39413e7d78 |
Cleaned up reworked range removal/diverging trunk algorithm
Note this is still blanket recoloring alts to black in diverged trunks.
So we don't get the main benefit of this rework yet: preserving the rbyd
color invariants.
But this is a nice checkpoint that shows that our two-pass diverging
trunk algorithm works without breaking right-leaning invariants.
Before, we made a single pass, and stitched together alternating
alts in any diverged trunks in order to remove ranges of tags (ignore
coloring for now, coloring _does_ help here):
.-----------o
.-------o-------. | o-----------.
.---o---. .---o---. | .-----o |
.-o-. .-o-. .-o-. .-o-. .-o-. | o-----. .-o-.
.o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .--o--. .o. .o. .o.
a b c d e f g h i j k l m n o p => a b c d e f g j k l m n o p
'-+-'
remove
Now, we do two passes: One to write any alts on the lower-diverged
trunk. And one to write the non-diverging part of the trunk, stitch in the
lower-diverged trunk with an alt, and then write any alts on the
upper-diverged trunk.
This results in a somewhat complex diverging state machine:
diverge possible? diverge not possible?
| |
v |
DIVERGINGLOWER-----------. |
| | |
v v v
DIVERGEDLOWER NOTDIVERGING
| |
v |
DIVERGINGUPPER |
| |
v |
DIVERGEDUPPER |
'--------. .--------'
v v
leaf stuff
But the end result is a much more balanced tree, at least on first
inspection:
.-------o-------. .--o--.
.---o---. .---o---. .--------o o--------.
.-o-. .-o-. .-o-. .-o-. .-o-. .--o o--. .-o-.
.o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. | | .o. .o. .o.
a b c d e f g h i j k l m n o p => a b c d e f g j k l m n o p
'-+-'
remove
Unfortunately, this does mean we need, well, two passes, even if the
resulting tree doesn't actually end up with a diverging trunk. We can
avoid two passes if we know a diverged trunk is impossible, but we only
know this in the single attr append case (no negative deltas, no
submask, no supmask, etc).
At the very least we don't end up _progging_ any more alts than is
necessary. Even though we write two trunks, the number of alts after
pruning end up the same as in our single-pass algorithm. The only
additional cost is a single null tag (4 bytes) to terminate the diverged
trunk.
This two pass algorithm may sound more complicated, and therefore more
costly, but keep in mind this replaces the previous diverged-swapping
state, which I would argue resulted in more mess and a harder to
understand lfsr_rbyd_appendattr.
Now that the dust has settled, we can actually start comparing the
relative code costs, though again note we are still blanket recoloring.
The result is a surprising basically net-zero code code:
code stack
one-pass: 33852 2880
two-pass: 33860 (+0.0%) 2880 (+0.0%)
|
||
|
Christopher Haster
|
9c2c5b2391 |
Prevented writing useless null tags for unstitched diverged trunks
This can happen if we end up pruning all alts in a diverged trunk. Note this is subtly different than finding no diverged trunk, as we still need to switch to the LFSR_D_DIVERGED* state in order to prune alts on the non-empty diverged trunk. We need a special case here, because there's no way to represent an empty trunk without a reachable null tag. But a reachable null tag would violate our rbyd's right-leaning property and break lookupnext. We already had a special case for this situation, which would skip the alt that would stitch the trunks together, but we were still writing out a null tag for the diverged trunk even if it was empty. We don't need this null tag and it turns out not writing the null tag saves a null tag. |
||
|
Christopher Haster
|
03954a1ef9 |
Fixed diverged leaf coloring issue
Took some debugging to figure out what was going on, but this was just a refactoring oversight, didn't update the diverged state used to decide how to color the leaves. I guess this would have actually been caught earlier if I cleaned up the code before debugging the test failures. But I wanted to reach proof-of-concept first... Anyways, good news, all tests are passing, so the proof-of-concept new range removal algorithm works. |
||
|
Christopher Haster
|
dd31f610b3 |
Fixed null tags getting stuck in the tree during range removals
Please excuse the mess. There is a delicate game going on here with where null tags can appear in rbyd trees. Null tags _can_ appear at the end of the tree, and as a terminator of unreachable trunks. Null tags can _not_ appear inside the tree in reachable trunks, as this would violate our right-leaning property and prevent lookupnext from working correctly. Long story short we need to very careful to ensure the lower diverged trunk's null tag is truely unreachable. Otherwise the null tag necessary to terminate the trunk (so that fetch works) breaks things. The solution seems to be keep track of the last _alt_ on the lower-diverged trunk, and use this alt to stich together the two diverged trunks when writing the non-diverged + upper-diverged trunks. This feels very similar to how you swap to remove from tree heaps, which is interesting. The rbyd tests are passing now, but higher-level tests are failing, which isn't the greatest sign... |
||
|
Christopher Haster
|
0b6cf7e9a7 |
Attempting a different algorithm for rbyd range removals
The previous attempt to make range removals more rigorous highlighted a pretty significant design flaw: Every removals risks making the tree ~2x taller. In theory this is offset by the fact that removals, well, remove nodes, shrinking the height of the tree, but this isn't reflected in the underlying red-black-yellow structure. Blanket recoloring breaks the red-black-yellow invariants. This isn't the end of the world, we still rebalance during rbyd compaction, but it would be nice if we had stronger guarantees about the structure of rbyds before compaction. Especially since we rely on tree balance to defend our O(n log n) traversal overhead. --- This attempts to reimplement range removals with two separate passes for diverged trunk. The downside is range removals now need, well, two separate passes, even if we don't actually end up with a diverged trunk. The upside is in theory we can preserve the coloring information and related invariants. I would describe this commit as "almost working" and "a mess". There still seems to be some issues with null tags getting stuck in the tree after diverged trunks. On the plus side, our tests are certainly working... |
||
|
Christopher Haster
|
d93dce8db2 |
Deduplicated rbyd append initialization into lfsr_rbyd_prepareappend
Most of the lfsr_rbyd_append* functions have the same necessary prologue
in order to check the rbyd is fetched, erased, is prefixed with a
revision count, etc. Moving this into a common function saves a bit of
code:
code stack
before: 33912 2880
after: 33852 (-0.2%) 2880 (+0.0%)
|
||
|
Christopher Haster
|
5ce78799af |
Added some extra low-level rbyd append helpers
- lfsr_rbyd_appendtag
- lfsr_rbyd_appenddata
- lfsr_rbyd_appendattr_
The main benefit is readability.
The second benefit is minor code deduplication:
code stack
before: 34024 2880
after: 33912 (-0.3%) 2880 (+0.0%)
|
||
|
Christopher Haster
|
3942d643e5 |
Renamed */other_* -> a_*/b_*
I think this does a better job of indicating that we're operating on two different paths simultaneously. At the very least the prefix other_* was kind of ambiguous... |
||
|
Christopher Haster
|
5c45f07f1b |
Added LFSR_TAG_DIVERGEDDONE instead of reusing LFSR_TAG_RM in appendattr
I think this is a bit more readable.
Curiously, the bit flip and bit change resulted in a surprising code
cost, even though it removes a couple statements. I guess because the
sign bit is that much cheaper to predicate on?
code stack
before: 33980 2880
after: 34024 (+0.1%) 2880 (+0.0%)
|