faf8c4b64114b5f81673867726ba0dc81ea6360f
1484 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Christopher Haster
|
faf8c4b641 |
Tweaked alt-tag encoding to match color/dir naming order
This is mainly to avoid mistakes caused by names/encodings disagreeing:
LFSR_TAG_ALT 0x4kkk v1cd kkkk -kkk kkkk
^ ^^ '------+-----'
'-||--------|------- valid bit
'|--------|------- color
'--------|------- dir
'------- key
Notably, the LFSR_TAG_ALT() macro has already caused issues by being
both 1. ambiguous, and 2. not really type-checkable. It's easy to get
the order wrong and things not really break, just behave poorly, it's
really not great!
To be honest the exact order is a bit arbitrary, the color->dir naming
appeared by accident because I guess it felt more natural. Maybe because
of English's weird implicit adjective ordering? Maybe because of how
often conditions show up as the last part of the name in other
instruction sets?
At least one plus is that this moves the dir-bit next to the key. This
makes it so all of the condition information is encoding is the lowest
13-bits of the tag, which may lead to minor optimization tricks for
implementing flips and such.
Code changes:
code stack
before: 34080 2864
after: 34068 (-0.0%) 2864 (+0.0%)
|
||
|
Christopher Haster
|
884982987e |
Tried to adopt consistent flip/flop/follow indention in rbyd functions
The intention here is to try to help readability by keeping arg locations somewhat consistent. Readability is already difficult enough given that these functions are so context dependent... |
||
|
Christopher Haster
|
8bfb1be926 |
Added some more tree transformation comments to lfsr_rbyd_appendattr
Hopefully having something to help visualize these tree operations will help make lfsr_rbyd_appendattr easier to understand. This one function is probably the most complicated function in littlefs, but for good reason. |
||
|
Christopher Haster
|
77c45827e5 |
rbyd-rr: Explicitly deduplicated diverging conditions
I'm not really sure why the compiler isn't taking care of this for us.
Usually I prefer duplicated logic over more variables since it means
less state to keep track of when reading/debugging, and the compiler
will optimize it away anyways. But I guess these conditions are just too
complicated in this case?
Maybe the compiler is trying to take advantage of &&/|| short-circuiting
even with -Os?
Even marking the lfsr_tag_diverging* functions with
__attribute__((noinline, pure, const)) doesn't help...
Oh well, this is a case where we can just make the deduplication
explicit for a bit of code savings:
code stack
before: 34176 2864
after: 34080 (-0.3%) 2864 (+0.0%)
code frame stack
appendattr before: 2162 208 560
appendattr after: 2104 (-2.7%) 216 (+3.8%) 568 (+1.4%)
|
||
|
Christopher Haster
|
94eb672315 |
rbyd-rr: Rearranged diverged pruning/trimming after flipping
This was a bit more tricky than the other eager-flip related
transformations, mainly because we have to be careful to not prune the
diverging alt that connects the two diverged trunks. The diverging alt,
i.e. the first alt that diverges, passes all the criteria for pruning,
but is a bit special in that we need to keep it around until we stitch
the trunks together.
I ended up more-or-less just reverting the handling of both-diverging
nodes to being collapsed as a special case of our first encounter with
the diverging alt. Because we eagerly prune, both-diverging nodes can
only happen if they include the diverging alt. We can leveraging this to
simplify our diverging logic a bit, which is already crazy complicated.
Not only does this finish moving all of the alt-related logic into
"flipped space", it also moves all of the diverging logic together,
which is more readable and hopefully leads to better code deduplication
by the compiler.
Long story short, more code savings!
code stack
before: 34244 2864
after: 34176 (-0.2%) 2864 (+0.0%)
code frame stack
appendattr before: 2232 216 568
appendattr after: 2162 (-3.1%) 208 (-3.7%) 560 (-1.4%)
---
All of these code savings are making our 2-trunk range removal algorithm
more appealing:
code stack
rr-div-naive: 33968 2864
rr-div-altn: 34304 (+1.0%) 2864 (+0.0%)
rr-2trunk-altn: 34176 (+0.6%) 2864 (+0.0%)
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: 2198 (+13.3%) 216 (+17.4%) 568 (+6.0%)
appendattr rr-2trunk-altn: 2162 (+11.4%) 208 (+13.0%) 560 (+4.5%)
That being said, it is getting increasingly hard to compare these
functions. You could argue the eager-flip transformations would also
result in code savings for the earlier iterations of our algorithm,
but it is worth noting the 2-trunk approach _did_ require more flips to
get working, so...
|
||
|
Christopher Haster
|
00a2332417 |
rbyd-rr: Tweaked both-diverged trimming to not pop
This adds some code:
code stack
before: 34224 2864
after: 34244 (+0.1%) 2864 (+0.0%)
code frame stack
appendattr before: 2190 216 568
appendattr after: 2232 (+1.9%) 216 (+0.0%) 568 (+0.0%)
But makes it so both diverged-trimming cases end up with a zero weight
unreachable alt, which may lead to more simplification...
|
||
|
Christopher Haster
|
82ddb33510 |
rbyd-rr: Rearranged pruning to only need lfsr_tag_unreachable*
An excellent example of the sort of simplification that eagerly flipping
gives us.
By flipping _before_ pruning, all unavoidable alts are transformed into
unreachable. This lets us check for one condition instead of two:
code stack
before: 34320 2864
after: 34224 (-0.3%) 2864 (+0.0%)
code frame stack
appendattr before: 2280 216 568
appendattr after: 2190 (-3.9%) 216 (+0.0%) 568 (+0.0%)
|
||
|
Christopher Haster
|
8f8dd9f981 |
rbyd-rr: Eagerly flip, adopt branch before/after to disambiguate ysplits
It's been annoying for a while how many flip operations we need in
lfsr_rbyd_appendattr to implement diverging range removals correctly.
Unfortuantely, we need all of these flips since we need to know the
original alt ordering in order to know how to split yellow nodes.
Keep in mind yellow splits depend on what alts exist in our history:
<y >b
.-------'| .-'|
| <r take red/yellow | >b
| .----'| => .-----|-'|
| | <b | <b |
| | .-'| | .-'| |
1 2 3 4 1 2 3 4 1
<b
.-'|
<y |
take black .-------'| |
=> | <r |
| .----' |
| | <b
| | .----'|
1 2 3 4 4
Or so I thought! Turns out there is a sort of hack we can use to
figure out the yellow split even after flipping.
Take a look at this example yellow node, and the various possible
jump/branch destinations:
.-- branch = 0xb20
00000b10: altrle 0x401 w0 0xa10 -|-> p[0].jump = 0xa10
00000b20: altrle 0x402 w0 0xa20 <'-> jump = 0xa20
00000b30: altble 0x403 w0 0xa30 <--- branch_ = 0xb30
Anything jump out? That's right! only branch_ is > branch.
This holds even after flips:
branch = 0xb20 branch = 0xb20 flip2 branch = 0xb20
p[0].jump = 0xa10 flip p[0].jump = 0xa10 --.---> p[0].jump = 0xb30
jump = 0xa20 --.--> jump = 0xb30 --'-.-> jump = 0xa20
branch_ = 0xb30 --'--> branch_ = 0xa20 ----'-> branch_ = 0xa10
This is provable by noting that our alts can't even encode forward
jumps. So... proof by lack of encoding?
We can use this to determine which yellow split is needed even after
flipping:
- branch_ < branch && jump < branch => take yellow alt
- branch_ < branch && jump > branch => take red alt
- branch_ > branch => take black alt
This lets us move/deduplicate the flipping logic before the diverging
logic and operate in a sort of "flipped space", where branch_ is always
the next branch we will take.
Unfortunately we do need to flip red alts that don't get split back
before descending down red nodes, which sort of matches our weird access
pattern, but this extra flip is well worth the code savings elsewhere.
---
This greatly simplifies the state space of lfsr_rbyd_appendattr, and it
already shows in code size measurements:
code stack
before: 34528 2864
after: 34320 (-0.6%) 2864 (+0.0%)
code frame stack
appendattr before: 2378 216 568
appendattr after: 2280 (-4.1%) 216 (+0.0%) 568 (+0.0%)
But this is really only after simplifying the diverging logic and yellow
splits. I think there may be even more savings if we can figure out how
to move all of the alt logic into the "flipped space"...
|
||
|
Christopher Haster
|
f06ef46e8b |
rbyd-rr: Simplified diverging state machine, rely on relative a/b ordering
So instead of explicitly keeping track of which bound we are on, either via
separate DIVERGEDLOWER/DIVERGEDUPPER states or a d_upper bool, we can
infer the bound based on the relative ordering a_rid/tag and b_rid/tag:
- a_rid < b_rid || a_tag < b_tag => lower bound
- a_rid > b_rid || a_tag > b_tag => upper bound
- a_rid == b_rid && a_tag == b_tag => not diverging
This is more appealing now that we don't rely on the specific bound for
diverged triming. The only remaining state is if we have diverged yet, a
simple boolean.
Measuring code size was a bit confusing. During a partial edit, it
looked like this was going to save a bit of code, but the result was
actually worse. It seems that explicitly masking/oring a single bit in
the original uint8_t d_state is somehow cheaper than storing if we have
diverged as a bool?
code stack
before: 34516 2864
bitmask: 34504 (-0.0%) 2864 (+0.0%)
boolean: 34528 (+0.0%) 2864 (+0.0%)
code frame stack
appendattr before: 2366 216 568
appendattr bitmask: 2354 (-0.5%) 216 (+0.0%) 568 (+0.0%)
appendattr boolean: 2378 (+0.5%) 216 (+0.0%) 568 (+0.0%)
No idea why this would happen. If feels like some sort of
compiler/optimizer bug... But this is pretty close to the compiler noise
floor and compilers aren't perfect. I'm probably reading too much into
an extra 24 bytes...
This is still a worthwhile change as it's usually good to prefer
implicit state over explicit. Less things can fall out of sync this way.
|
||
|
Christopher Haster
|
ffc36b0f36 |
rbyd-rr: Added lfsr_tag_diverging and lfsr_tag_diverging2
If nothing else these at least makes the code a bit more readable.
Curiously this improved lfsr_rbyd_appendattr, but made the total code
size worse. I guess these really should be inlined, but don't pass some
compiler heuristic. Oh well, optimization is a hard problem:
code stack
before: 34492 2864
after: 34516 (+0.1%) 2864 (+0.0%)
code frame stack
appendattr before: 2414 216 568
appendattr after: 2366 (-2.0%) 216 (+0.0%) 568 (+0.0%)
|
||
|
Christopher Haster
|
660d323564 |
rbyd-rr: Renamed lfsr_rbyd_p_* -> lfsr_p_*
I didn't notice the inconsistency at first, but with the addition of the diverging state machine, we have to subcomponents in lfsr_rbyd_appendattr with different naming conventions: - lfsr_rbyd_p_* - the p-alt fifo - lfsr_d_* - the diverging state machine One of these needs to change, and lfsr_rbyd_d_isdiverged is such a keyful... |
||
|
Christopher Haster
|
d3e09b082f |
rbyd-rr: Minor tweaks, adopted diverging check for diverged triming
Previously we used the direction of post-diverged alts to decide if they
need to be trimmed or not:
lfsr_d_isdiverged(d_state)
&& lfsr_d_isupper(d_state)
^ lfsr_tag_isgt(alt)
^ lfsr_tag_follow2(
alt, weight,
p[0].alt, p[0].weight,
lower_rid, upper_rid,
a_rid, a_tag)
But this working is a bit accidental. The real condition that needs to
be met for trimming is if our bounds continue to diverge on the alt:
lfsr_d_isdiverged(d_state)
&& lfsr_tag_follow2(
alt, weight,
p[0].alt, p[0].weight,
lower_rid, upper_rid,
a_rid, a_tag)
^ lfsr_tag_follow2(
alt, weight,
p[0].alt, p[0].weight,
lower_rid, upper_rid,
b_rid, b_tag)
This may seem more complicated, and does add code, but I'm hopeful it
can eventually lead to better code deduplication with the preceding
not-diverged -> diverged checks:
code stack
before: 34468 2864
after: 34492 (+0.1%) 2864 (+0.0%)
code frame stack
appendattr before: 2390 216 568
appendattr after: 2414 (+1.0%) 216 (+0.0%) 568 (+0.0%)
I've also been trying to simplify/deduplicate the diverging logic more,
but it's proven difficult. There's an annoying catch-22 where 1. we need
to trim diverging alts before applying color transformations, but 2. we
need to resolve yellow splits before triming diverging alts.
|
||
|
Christopher Haster
|
01b28b3224 |
rbyd-rr: Rearranged some things so appendattr gotos make a bit more sense
- Renamed again: -> trunk:
- Added stem:, moved the awkward pre-stem logic into the not-alt check
- Kept leaf: unchanged
This organizes lfsr_rbyd_appendattr into logical trunk -> stem -> leaf
stages, which I think makes quite a bit of sense.
GCC is happy if we change the loop termination into goto stem, but I
think it's quite unfortunate that GCC's -Wunused-label warning
discourages labels for purely code organization. They're quite useful
for organizing complicated functions at a level higher than comments,
and GDB's break func:label syntax shows potential for external tooling.
Maybe we should disable -Wunused-label?
---
Not sure why this impacted code size, the transformation should have
been a noop. Then again, it's not too surprising, gotos are supposedly
pretty annoying to optimize around:
code stack
before: 34480 2864
after: 34468 (-0.5%) 2864 (+0.0%)
code frame stack
appendattr before: 2404 216 568
appendattr after: 2390 (-0.6%) 216 (+0.0%) 568 (+0.0%)
|
||
|
Christopher Haster
|
54c8beee70 |
rbyd-rr: Adopted a struct-based p-alt fifo representation
So instead of:
lfsr_tag_t p_alts[3];
lfsr_rid_t p_weights[3];
lfs_size_t p_jumps[3];
We now have:
lfsr_alt_t p[3];
Note this is the only place where we use the new lfsr_alt_t type,
hopefully using such a general name doesn't create confusion down the
road...
I was mostly just curious which representation the compiler
(GCC 11.4 -mthumb) would handle better. In theory a struct
representation will result in more efficient memmoves, since we usually
operate on entire alts at a time when manipulting our fifo.
The original motivation for the separate arrays was to avoid alignment
issues with the 16-bit lfsr_tag_t, but this was apparently premature:
code stack
before: 34644 2864
after: 34480 (-0.5%) 2864 (+0.0%)
code frame stack
appendattr before: 2452 216 568
appendattr after: 2404 (-2.0%) 216 (+0.0%) 568 (+0.0%)
Actually, it's a bit strange that lfsr_rbyd_appendattr showed _no_ stack
changes... I wonder why that is?
|
||
|
Christopher Haster
|
eb2c7a9a05 |
rbyd-rr: Switched diverging state from bools to a small state machine
The state machine is pretty simple:
NOTDIVERGEDLOWER
|
diverging?-no--.
yes |
v |
DIVERGEDLOWER |
| |
v |
NOTDIVERGEDUPPER |
| |
v |
DIVERGEDUPPER |
'--------. |
v v
done
The nice thing about the 2-trunk algorithm is we don't need any extra
states for cleanup and we don't need to predict if we will diverge or
not. The always start by writing out the common trunk, and switch to the
diverging state machine retroactively if necessary.
With only 4 states, the difference between bools and a small state
machine is negligible. I was mostly just curious which approach the
compiler (GCC 11.4 -mthumb) could optimize better.
Which is apparently the state machine:
code stack
before: 34656 2864
after: 34644 (-0.0%) 2864 (+0.0%)
code frame stack
appendattr before: 2464 224 576
appendattr after: 2452 (-0.5%) 216 (-3.6%) 568 (-1.4%)
Though word of warning, this is basically the compiler's noise floor.
|
||
|
Christopher Haster
|
64046d495e |
rbyd-rr: Cleaned up new 2-trunk range-removal algorithm
Removed a bunch of outdated code, printfs, old diverging state machine,
updated comments, etc.
Also tried to simplify the diverging alt logic as much as possible, but
the logic is quite stubborn. We can at least make some interesting
assumptions about alt ordering on the upper-diverged path, since we know
the lower-diverged path will flip and collapse 2-3 nodes.
---
Now that the dust has settled (again), we can compare our new 2-trunk
algorithm to our previous attempts:
code stack
rr-div-naive: 33968 2864
rr-div-altn: 34304 (+1.0%) 2864 (+0.0%)
rr-2trunk-altn: 34656 (+2.0%) 2864 (+0.0%)
Focusing on lfsr_rbyd_appendattr, which lets us compare further back in
history:
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: 2198 (+13.3%) 216 (+17.4%) 568 (+6.0%)
appendattr rr-2trunk-altn: 2464 (+27.0%) 224 (+21.7%) 576 (+7.5%)
And comparing the resulting tree color-balance:
2-tree 2-3-4-tree
rr-stitching: +~2x +~2x
rr-div-naive: +0 +~2x
rr-div-altn: +0 +~1 on red
rr-2trunk-altn: +0 +~1 on yellow
It's again an annoyingly expensive algorithm change, but necessary to
maintain the correct balance of our rbyds as much as possible. Keep in
mind range operations are used _everywhere_ in the high-level operations
in our filesystem. It's just too useful a tool.
The "+~1 on yellow" vs "+~1 on red" may not seem like that much of an
improvement, but keep in mind yellow alts are much less common, and
temporary. Decaying into black alts on the next append. At rest, most
alts are either black or red.
It's also worth mentioning that, in theory, the rr-2trunk-altn approach
_could_ be extended to be perfectly balancing, but this would likely
require duplicating the entire yellow-split logic, which is probably not
worth it in this implemention...
|
||
|
Christopher Haster
|
9c8a44a461 |
rbyd-rr: Enabled color preservation on diverging-lower alt
It's a great sign that this just worked.
Now, the only case where coloring is not preserved is the
diverging-upper alt, and only when encountering a yellow node. A rather
complicated corner case:
.-> .-> .-> h=4 -.
.-----b-> .-b-> .-b-> |
| .-> | .-> | .-> |
| .---b-> .-y-b-> .-y-b-> |
| | .-> | .-> | .-> |
| | .-b-> | .-b-> | .-b-> |
| | | .-> y-r-b-b-b-> .-b-b-b-> |
.-y-r-b-b-> rm me => | | => | +- unbal :(
| .-> rm me | | .-> | |
| .-b-> | | .-b-> r-b---b-b-> |
| | .-> | | | .-> | | .-> |
| .---b-b-> | '---b-b-> | '-b-> |
| | .-> | .-> | .-> |
| | .-b-> | .-b-> | .-b-> |
| | | .-> | | .-> | | .-> |
r-b---b-b-> '-----b-b-> '-----b-b-> h=3 -'
^ ^
diverging diverging/stitching
In theory it _is_ possible to preserve coloring on yellow nodes, but
right now this only seems possible by duplicating most of the
yellow-split logic, which doesn't seem worth it...
|
||
|
Christopher Haster
|
f957dad821 |
rbyd-rr: Implemented very ugly, but working! diverging 2-3 nodes
It's a mess, but all tests are passing.
We're still recoloring the diverging alt, so hopefully I won't need to
eat my words, but at least on paper this should be able to preserve
colors for all 2-3 permutations of the diverging alt.
The key observation here is that diverging 2-3 nodes have three possible
permutations:
1. Diverging on the black alt:
.-> .-> .->
.---b-> .---b-> .---b->
| .-> => r-b-b-> => | .-b->
| .-b-> rm me | | |
| | .-> rm me | .-> | |
r-b-b-> '-b-> r-b-b->
^ ^
diverging diverging
2. Diverging on the red alt:
.-> r-b-b-> .--->
.---b-> rm me | | |
| .-> rm me => | | .-> => |
| .-b-> | '-b-> r-b-b->
| | .-> | .-> | .->
r-b-b-> '---b-> '---b->
^ ^
diverging diverging
3. Diverging on both alts:
.-> b---b-> .---b->
.---b-> rm me | |
| .-> rm me => | => |
| .-b-> rm me | |
| | .-> rm me | .-> |
r-b-b-> '---b-> b---b->
^^^ ^
diverging diverging
With 3., both diverging, being the tricky one, where we need to both
switch to the diverged state while also collapsing the 3-node into a
2-node.
1. and 2. can both be deduplicated with a well-timed flip, but so far
it seems like 3. needs its own special case. At least these can all be
contained as extra conditions in the diverging alt logic, reducing the
possible states.
lfs_rbyd_appendattr is a complete mess now, and a lot of the diverging
logic is duplicated everywhere, but at least things seem to be working.
|
||
|
Christopher Haster
|
c370fbec1a |
rbyd-rr: Limping along, fixed test_files_many, all tests are passing now
The issue, found in test_files_many:h1g4j10l18, occurs when a SUBWIDE
tag follows a compaction.
When this happens, it's possible for our stitched diverging alt to be
followed/flipped when it shouldn't be. This is because the new
lower_rid/upper_rid window can make the stitched alt ambiguous.
I don't think this is strictly an issue with compaction, as much as
compaction is giving us a tree structure that's not reachable through
only appendattrs.
Here are the three culprit trunks:
altrle 0x300 w8 0x2c8
altble 0x203 w6 0x2d4 <- diverge
null
altrle 0x300 w8 0x2c8
altbgt 0x203 w0 0x2e8 <- diverge
altble 0x300 w4 0x2b4
altbn w0 0x0
altble 0x300 w1 0x228
altbn w0 0x0
null
altrle 0x300 w8 0x2c8
altbgt 0x300 w0 0x2e8 <- stitch
altbn w0 0x0
altbn w0 0x0
altbn w0 0x0
altbgt 0x201 w0 0x164
reg w1
And here is a simplified view, after compaction, before we do a subwide
append/replace:
.-> reg w1 a
.-------b-> data
| .-> reg w1 b
| .---b-> data
r-b-r-b-b-> orphan w1 <- removed as a part of our subwide op
^
diverging
'-+-'
weight=3
altrle data w1
altble orphan w1
First, as a part of our subwide append, we're going to write out the
lower trunk. We diverge on the first altble since the entire orphan is
inside our subwide range.
It may seem a bit strange to diverge on a null tag, but this isn't
actually an issue, we're allowed a single null tag to terminate our
tree:
.-> reg w1 a
.-------b-> data
| .-> reg w1 b
r-b-----b-> data
^
diverging
'-+-'
weight=3
altrle data w1
altbgt data w0
Nothing wrong so far. The weight of our leaves (2) don't match our
tree's weight (3), but this is normal for the lower trunk. We fix this
when we stitch the diverging alt on the upper trunk.
Speaking of the upper trunk, let's start writing it out, but pause at
the stitching alt:
.-> reg w1 a
.-------b-> data
| .-> reg w1 b
| .-----b-> data
r-b-?
^
stitching
'-+-'
weight=3
altrle data w1
altble data w1
Note we've flipped the altbgt data into an altble data, since we're
going down the other diverged path now.
But before we continue, as a part of stitching, we need to adjust our
tree weight to account for the weight of the orphan we deleted as a part
of our range operation:
.-> reg w1 a
.-------b-> data
| .-> reg w1 b
| .-----b-> data
r-b-?
^
stitching
'-+-'
weight=2
altrle data w1
altble data w1
Uh oh. Weight is 2 and both our alts add up to 2? All of a sudden it
looks like we should follow the stitched alt.
Our follow/flip logic kicks in, and disaster!
.-> reg w1 a
.-------b-> data
r-b-----b-> reg w1 c <- added as a part of our subwide op
'-> data <- somehow data survives
'-+-' but where did b go?
weight=2
altrle data w1
altbgt data w0
We go down the wrong path, and because our state machine thinks we've
diverged, we prune all le alts, destroying our tree.
---
So what's is going wrong?
The problem is that when we update our window, the stitched diverging
alt can become ambiguous.
Which sort of makes sense. The reason we update our window is so we can
continue down the tree veiwing it as it was _before_ the range
operation. But the stitched alt belongs to the tree _after_ the range
operation.
The solution here is to just make sure we never follow the stitched alt.
This is a bit annoying, as it makes the stitched alt a rather special
case, but as far as I can tell it's necessary to avoid ambiguity.
|
||
|
Christopher Haster
|
c4681fff0e |
rbyd-rr: Preserving diverging alt coloring with careful pruning rules
This seems to mostly be working, now passing rbyd tests at least. This pruning/triming logic desperately needs to be simplified/cleaned up, but preserving diverging alt color balance without breaking things is still proving to be difficult... |
||
|
Christopher Haster
|
c73749039e |
rbyd-rr: Trying another approach, 2-trunk diverging
This is a good checkpoint and is mostly working, though we're back to
recoloring the diverging alt black again. So no balance improvements.
But this already feels much better complexity-wise.
The fact that things could get back to a working state so quickly is a
good sign, or maybe just a sign I've been steeped in this algorithm for
too long...
---
The idea here is instead of a relatively complex 4-step state machine:
diverged? diverged
skip common -+-> write lower -> write common -> write upper -> done
'-> write common -> done
We just write two trunks: one for the lower bound, one for the upper
bound.
We _do_ need to keep track of where we diverge so we can prune
correctly, so this is _technically_ still 4-steps, but it is at least
conceptually, and in code, much simpler:
diverged? diverged
write common -+-> write lower -> write common -> write upper -> done
'-> done
Note that if we discover no tags in our range, we can terminate after
writing the lower/common trunk, which is nice. Previously we needed a
second pass.
The obvious downside is that we write the common trunk twice now. Which
is a bit of a downside, those alts will never really be used, but as a
tradeoff it really isn't that much of a waste. It's already possible for
range operations to need to write the full trunk twice, even for small
ranges:
.-------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
The original motivation for trying yet-another-range-removal-algorithm
comes from attempting to solve issues with the color-balance of the
diverging alt.
The core conundrum being the case of two pending yellow splits. In our
previous algorithm, we only have one common trunk, so trying to
propagate two red edges violates tail recursion:
.-> .-> .-> 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
But if we have two trunks? Even only temporarily? This allows both
yellow splits/red edge propagation to settle tail recursively:
.-> .-> .-> h=3 -.
.-----b-> .-b-> .-b-> |
| .-> | .-> | .-> |
| .---b-> .-y-b-> .-----y-b-> |
| | .-> | .-> | .-> |
| | .-b-> | .-b-> | .-b-> |
| | | .-> r---b-b-b-> | .---b-b-> |
.-y-r-b-b-> rm me => | => | | +- balanced :)
| .-> | .-> y-r-b-b-b-> |
| .-----b-> | .-----b-> | | '-> |
| | .-> | | .-> | | .-> |
| | .---b-> | | .---b-> | '-b-> |
| | | .-> | | | .-> | .-> |
| | | .-b-> | | | .-b-> | .-b-> |
| | | | .-> | | | | .-> | | .-> |
b-y-r-b-b-> '-y-r-b-b-> '-b-b-> h=3 -'
^
diverging
It's interesting to note while we _are_ currently recoloring the
diverging alt black (intentionally simplifying to algorithm to get
things moving), we are already allowing yellow splits/red edge
propagation by just writing out both trunks normally.
And that's what makes this yet-another-range-removal-algorithm appealing
and worth yet another iteration, the diverging trunks are no longer such
special cases. Not only will this make a better diverging color-balance
possible, it will hopefully make the whole diverging algorithm simpler,
easier, and cheaper. And it is already showing good signs so far.
|
||
|
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. |