More proof the tests are working.
This bug was in the code that does an extra lookup for the was-split entry
during merge, so we make sure we have the right id to attach the split
name to. Humorously, this code was already set up correctly, the
"split_id" just wasn't actually used. Unfortunately since
lfsr_rbyd_lookup uses out-pointers to return multiple things the
compiler couldn't detect the unused variable.
B-trees with names are now working, though this required a number of
changes to the B-tree layout:
1. B-tree no-longer require name entries (LFSR_TAG_MK) on each branch.
This is a nice optimization to the design, since these name entries
just waste space in purely weight-based B-trees, which are probably
going to be most B-trees in the filesystem.
If a name entry is missing, the struct entry, which is required,
should have the effective weight of the entry.
The first entry in every rbyd block is expected to be have no name
entry, since this is the default path for B-tree lookups.
2. The first entry in every rbyd block _may_ have a name entry, which
is ignored. I'm calling these "vestigial names" to make them sound
cooler than they actually are.
These vestigial names show up in a couple complicated B-tree
operations:
- During B-tree split, since pending attributes are calculated before
the split, we need to play out pending attributes into the rbyd
before deciding what name becomes the name of entry in the parent.
This creates a vestigial name which we _could_ immediately remove,
but the remove adds additional size to the must-fit split operation
- During B-tree pop/merge, if we remove the leading no-name entry,
the second, named entry becomes the leading entry. This creates a
vestigial name that _looks_ easy enough to remove when making the
pending attributes for pop/merge, but turns out the be surprisingly
tricky if the parent undergoes a split/merge at the same time.
It may be possible to remove all these vestigial names proactively,
but this adds additional rbyd lookups to figure out the exact tag to
remove, complicates things in a fragile way, and doesn't actually
reduce storage costs until the rbyd is compacted.
The main downside is that these B-trees may be a bit more confusing
to debug.
Name lookup brings back the O(m') scan-during-fetch approach of the
previous metadata layout. Since our rbyd trees map id+attr pairs and not
actual names, this beats the alternative O(m log(m)) scan of the tree.
Though tree searching does only include the current attributes, where as
scanning during fetch needs to also look at outdated attributes. Which
may make the winner less obvious depending on how we find the rbyd. But
being able to do the search in the same pass as fetch is an extra plus.
---
What turned out to be surprisingly complicated was the propagation of
names during B-tree splits and merges. The on-disk reference,
lfsr_data_t, does most of the heavy lifting here, but there's just a lot
of corner cases to consider.
At the moment this isn't working due to outdated names on the leading
entries of the rbyds, but to fix this bigger changes to the B-tree
layout may be needed.
- lfsr_rbyd_predictedlookup, the new B-tree approach means we hopefully
won't need this anymore. Worst case this remove can be reverted.
- LFSR_TAG_FROM - this will likely come back, but needs to be
rewrittern.
These benchmarks are now more useful for seeing how these B-trees perform.
In plot.py/plotmpl.py:
- Added --legend as another alias for -l, --legend-right.
- Allowed omitting of datasets from the legend by using empty strings
in --labels.
- Do not sum multiple data points on the same x coordinate. This was a
bad idea that risks invalid results going unnoticed.
As a plus multiple data points on the same x coordinate can be abused for
a cheap representation of measurement error.
TEST_PERMUTATION/BENCH_PERMUTATION make it possible to map an integer to
a specific permutation efficiently. This is helpful since our testing
framework really only parameterizes single integers.
The exact implementation took a bit of trial and error. It's based on
https://stackoverflow.com/a/7919887 and
https://stackoverflow.com/a/24257996, but modified to run in O(n) with
no extra memory. In the discussion it seemed like this may not actually
be possible for lexicographic ordering of permutations, but fortunately
we don't care about the specific ordering, only the reproducibility.
Here's how it works:
1. First populate an array with all numbers 0-n.
2. Iterate through each index, selecting only from the remaining
numbers based on our current permutation.
.- i%rem --.
v .----+----.
[p0 p1 |-> r0 r1 r2 r3]
Normally to maintain lexicographic ordering you should have to do a O(n)
shift at this step as you remove each number. But instead we can just swap
the removed number and number under the index. This effectively
shrinks the remaining part of the array, but permutes the numbers
a bit. Fortunately, since each successive permutation swaps
at the same location, the resulting permutations will be both
exhaustive and reproducible, if unintuitive.
Now permutation/fuzz tests can reproduce specific failures by defining
either -DPERMUTATION=x or -DSEED=x.
This reworks test.py/bench.py a bit to map arguments to ids as a first
step instead of defering as much as possible. This is a better design
and avoids the hackiness around -b/-B. As a plus, test_id globbing is
easy to add.
I wondered if walking in Python 2's footsteps was going to run into the
same issues and sure enough, memory backed iterators became unweildy.
The motivation for this change is that large ranges in tests, such as
iterators over seeds or permutations, became prohibitively expensive to
compile. This meant more iteration moving into tests with more steps to
reproduce failures. This sort of defeats the purpuse of the test
framework.
The solution here is to move test permutation generation out of test.py
and into the test runner itself. The allows defines to generate their
values programmatically.
This does conflict with the test frameworks support of sets of explicit
permutations, but this is fixed by also moving these "permutation sets"
down into the test runner.
I guess it turns out the closer your representation matches your
implementation the better everythign works.
Additionally the define caching layer got a bit of tweaking. We can't
precalculate the defines because of mutual recursion, but we can
precalculate which define/permutation each define id maps to. This is
necessary as otherwise figuring out each define's define-specific
permutation would be prohibitively expensive.
Another straightforward exercise of making sure the pending attributes
are setup correctly.
If you think this isn't worth its own function, consider how much
overhead the 3x commits for pop+push+push would add, especially for
large-prog devices.
Worst case this can be dropped in the future.
A single child is just another condition to watch out for during B-tree
merge, since a single-child obviously can't have a sibling.
This is a good safety to have, but I was surprised this can happen. But
it turns out to be quite easy since our rbyds defer the B-tree
operations until compaction. A merge down to a single child won't
propagate the merge until the parent compacts.
B-tree remove/merge is the most annoying part of B-trees.
The implementation here follows the same ideas implemented in push/split:
1. Defer splits/merges until compaction.
2. Assume our split/merge will succeed and play it out into the rbyd.
3. On the first sign of failure, revert any unnecessary changes by
appending deletes.
4. Do all of this in a single commit to avoid issues with single-prog
blocks.
Mapping this onto B-tree merge, the condition that triggers merge is
when our rbyd is <1/4 the block_size after compaction, and the condition
that aborts a merge is when our rbyd is >1/2 the block_size, since that
would trigger a split on a later compact.
Weaving this into lfsr_btree_commit is a bit subtle, but relatively
straightforward all things considered.
One downside is it's not physically possible to try merging with both
siblings, so we have to choose just one to attempt a merge. We handle
the corner case of merging the last sibling in a block explicitly, and
in theory the other sibling will eventually trigger a merge during its
own compaction.
Extra annoying are the corner cases with merges in the root rbyd that
make the root rbyd degenerate. We really should avoid a compaction in
this case, as otherwise we would erase a block that we immediately
inline at a significant cost. However determining if our root rbyd is
degenerate is tricky. We can determine a degenerate root with children
by checking if our rbyd's weight matches the B-tree's weight when we
merge. But determining a degenerate root that is a leaf requires
manually looking up both children in lfsr_btree_pop to see if they will
result in a degenerate root. Ugh.
On the bright side, this does all seem to be working now. Which
completes the last of the core B-tree algorithms.
This was a rather simple exercise. lfsr_btree_commit does most of the
work already, so all this needed was setting up the pending attributes
correctly.
Also:
- Tweaked dbgrbyd.py's tree rendering to match dbgbtree.py's.
- Added a print to each B-tree test to help find the resulting B-tree
when debugging.
This was particularly nasty to track down, the bad alts left in this way
are zero-weight, zero-tag alts that point out of the bounds of the rbyd.
This creates an immovable-object/unstoppable-force situation since the
alt that will never be followed should always be followed. This ended up
creating a confusing issue later since grows can follow this alt and
cause the alt state to fall apart.
The solution is to check for shrink leaves that drop to weight zero and
prune them. This has a side-effect of nicely handling over-sized
shrinks, though these shouldn't happen anyways and are being asserted
on.
Because I really, really don't want a regression, I've added a specific
test for this, though the minimal reproducible case is a bit complex.
The state of the rbyd is rather sensitive and it's not fully clear to me
what ultimately triggers the breakdown of the rbyd tree.
Also added a slightly better check for grow/shrink tags on altle leaves.
I don't know if this is strictly required but I know it keeps me sane.
Changed so there is no 1-to-1 mk-tag/id assumption, any unique ids
create a simulated lifetime to render. This fixes the issue where
grows/shrinks left-aligned ids confused dbgrbyd.py.
As a plus, now dbgrbyd.py can actually handle multi-id grow/shrinks, and
is more robust against out-of-sync grow/shrinks. This sort of lifetime issues
are when you'd want to run dgbrbyd.py, so it's a bit important this is handled
gracefully.
An example:
$ ./scripts/dbgbtree.py -B4096 disk 0xaa -t -i
btree 0xaa.1000, rev 35, weight 278
block ids name tag data
(truncated)
00aa.1000: +-+ 0-16 branch id16 3 7e d4 10 ~..
007e.0854: | |-> 0 inlined id0 1 73 s
| |-> 1 inlined id1 1 74 t
| |-> 2 inlined id2 1 75 u
| |-> 3 inlined id3 1 76 v
| |-> 4 inlined id4 1 77 w
| |-> 5 inlined id5 1 78 x
| |-> 6 inlined id6 1 79 y
| |-> 7 inlined id7 1 7a z
| |-> 8 inlined id8 1 61 a
| |-> 9 inlined id9 1 62 b
...
This added the idea of block+limit addresses such as 0xaa.1000. Added
this as an option to dbgrbyd.py along with a couple other tweaks:
- Added block+limit support (0x<block>.<limit>).
- Fixed in-device representation indentation when trees are present.
- Changed fromtag to implicitly fixup ids/weights off-by-one-ness, this
is consistent with lfs.c.
- After a B-tree split, when we're append pending attributes, it's
possible for the id chosen for bisection to be itself modified by
pending grows/shrinks. This needs to be accounted for in the two
passes for the two children.
But this means our tests are working.
This really just required care around calculating the expected B-tree id
and rbyd id (which are different!).
B-tree append, aka B-tree push with id=weight, is actually the outlier.
We need a B-tree id that can identify the rbyd we're appending to, but
this id itself doesn't exist in the tree yet, which can be a bit tricky.
In B-tree split we turn one rbyd into two by comparing each tag to an id we
as a mid-point.
I first implemented this by writing both children in parallel, which is
efficient, but requires two pcaches for low-level page alignment issues.
However we really don't have to write these in parallel. We can just write
each child sequentially by making two passes of the original rbyd.
---
With this fix, the rewrite of B-tree splitting without predicted rbyd
sizes now works.
The idea is, instead of predicting the rbyd size to decide whether or not
to split, assume we always fit, perform a normal compaction, and if it
turns out we don't fit, make a split, writing rm tags as necessary to revert
any ids that don't belong in the first child.
The neat thing about this is we can use low-level, uncommitting rbyd appends
to do all of this in a single commit, avoiding issues with single-prog
blocks.
This can waste some progs, up to 1/4 of a block during a B-tree split.
However, it removes the main need for the rbyd prediction operations,
which are complicated, error prone, and concerning. B-tree
removes/merges still need an implementation, but this may mean that we
can let the on-disk rbyd data-structure be the only source of knowledge
about tags, which is great for ensuring consistent behavior.
This does mean we don't predict rbyd changes during compaction. Any pending
attributes just get appended to the rbyd after compaction, so size-changing
operations such as removes can lead to splits that could be avoided. But
I think these cases can lead to unnecessary splits anyways depending on
when compaction occurs, so I'm not sure it's really an issue. But I can
always be wrong about that.
I didn't realize until testing, this approach requires two pcaches. This
completely breaks assumptions in the caching layer and requiring an
additional cache is probably too much of a cost to be acceptable. So
back to the drawing board.
This involves many, many hacks, but is enough to test the concept
and start looking at how it interacts with different block sizes.
Note only append (lfsr_btree_push on the end) is implemented, and it
makes some assumption about how the ids can interact when splitting
rbyds.
- Added test_rbyd_fuzz_mixed/test_rbyd_fuzz_sparse
- Added test_rbyd_unwritten_mixed_fuzz/test_rbyd_unwritten_sparse_fuzz
- Also renamed "random" tests to "fuzz", this describes their purpose a
bit better
These were a bit tricky to add since they need to simulate rbyd weights,
but they should give significant coverage over complicated rbyd corner
cases I may have not thought about.
Also fixed a miscalculation in lfsr_rbyd_pendinglookup when finding a
id that grew. Finding this bug is a good sign these tests are working.
This ends up surprisingly tricky with sparse ids. I feel like I'm missing
a simpler solution, but this at least proves an implementation is possible.
The implementation here does a single pass through the attributes
backwards (which should probably be changed from a linked-list), keeping
track of the best matching tag/id while updating everything based on
grows/shrinks. Once we find the source of the best id we adjust things
back to the pending id space.
The implementation here only works with some significant caveats:
1. This solution might be able to find the id weights by keeping track
of a lower bound, but it would be difficult and add complexity, so we
don't do it. Really lfsr_rbyd_pendinglookup is only going to be used
in full traversals as a part of compaction/splitting, so weight can
be derived trivially from neighboring ids.
2. We don't know the difference between grows/shrinks used to change a
branch's weight and used to create/delete ids. This is a bit of a
problem here, but we can work around it by assuming that
non-destructive grows/shrinks are always on the lower edge of a
weighted id.
Fortunately this assumption is only needed for in-flight attrs in
lfsr_rbyd_pendinglookup, so this is not a requirement on-disk or in
future implemenations.
1. Search backwards through our tags to find the most recent,
best matching id.
2. Replay tags after the found id to adjust for any pending changes.
In theory this should work in controlled cases, but there are a lot of
corner cases around grows and shrinks. Tests are written, and failing,
but I think it may be simpler and more efficient to implement this in a
single pass, with tighter assumptions about what grow/shrinks are
allowed.
This implements a common B-tree using rbyd's as inner nodes.
Since our rbyds actually map to sorted arrays, this fits together quite
well.
The main caveat/concern is that we can't rely on strict knowledge on the
on-disk size of these things. This first shows up with B-tree insertion,
we can't split in preparation to insert as we descend down the tree.
Normally, this means our B-tree would require recursion in order to keep
track of each parent as we descend down our tree. However, we can
avoid this by not storing our parent, but by looking it up again on each
step of the splitting operation.
This brute-force-ish approach makes our algorithm tail-recursive, so
bounded RAM, but raises our runtime from O(logB(n)) to O(logB(n)^2)
That being said, O(logB(n)^2) is still sublinear, and, thanks to
B-tree's extremely high branching factor, may be insignificant.
The way sparse ids interact with our flat id+attr tree is a bit wonky.
Normally, with weighted trees, one entry is associated with one weight.
But since our rbyd trees use id+attr pairs as keys, in theory each set of
id+attr pairs should share a single weight.
+-+-+-+-> id0,attr0 -.
| | | '-> id0,attr1 +- weight 5
| | '-+-> id0,attr2 -'
| | |
| | '-> id5,attr0 -.
| '-+-+-> id5,attr1 +- weight 5
| | '-> id5,attr2 -'
| |
| '-+-> id10,attr0 -.
| '-> id10,attr1 +- weight 5
'-------> id10,attr2 -'
To make this representable, we could give a single id+attr pair the
weight, and make the other attrs have a weight of zero. In our current
scheme, attr0 (actually LFSR_TAG_MK) is the only attr required for every
id, and it has the benefit of being the first attr found during
traversal. So it is the obvious choice for storing the id's effective weight.
But there's still some trickiness. Keep in mind our ids are derived from
the weights in the rbyd tree. So if follow intuition and implement this naively:
+-+-+-+-> id0,attr0 weight 5
| | | '-> id5,attr1 weight 0
| | '-+-> id5,attr2 weight 0
| | |
| | '-> id5,attr0 weight 5
| '-+-+-> id10,attr1 weight 0
| | '-> id10,attr2 weight 0
| |
| '-+-> id10,attr0 weight 5
| '-> id15,attr1 weight 0
'-------> id15,attr2 weight 0
Suddenly the ids in the attr sets don't match!
It may be possible to work around this with special cases for attr0, but
this would complicate the code and make the presence of attr0 a strict
requirement.
Instead, if we associate each attr set with not the smallest id in the
weight but the largest id in the weight, so id' = id+(weight-1), then
our requirements work out while still keeping each attr set on the same
low-level id:
+-+-+-+-> id4,attr0 weight 5
| | | '-> id4,attr1 weight 0
| | '-+-> id4,attr2 weight 0
| | |
| | '-> id9,attr0 weight 5
| '-+-+-> id9,attr1 weight 0
| | '-> id9,attr2 weight 0
| |
| '-+-> id14,attr0 weight 5
| '-> id14,attr1 weight 0
'-------> id14,attr2 weight 0
To be blunt, this is unintuitive, and I'm worried it may be its own
source of complexity/bugs. But this representation does solve the problem
at hand, so I'm just going to see how it works out.
- Fixed off-by-one id for unknown tags.
- Allowed block_size and block to go unspecified, assumes the block
device is one big block in that case.
- Added --buffer and --ignore-errors to watch.py, making it a bit better
for watching slow and sometimes error scripts, such as dbgrbyd.py when
watching a block device under test.
This turned out to be a bit tricky, and the scheme in bench_rbyd is
broken.
The core issue is that we don't have a distinction between physical and
logical block sizes, so we can't use a block device configured for one
geometry with a littlefs instance operating on a different geometry. For
this and other reasons we should probably have two configuration
variables in the future, but at the moment that is out of scope.
The problem with the approach in bench_rbyd, which changes the
lfs_config at runtime, is that this breaks emubd which also depends on
lfs_config due to a leaky abstraction. This causes unnoticed memory
corruption.
---
To get something working, the tests now change the underlying BLOCK_SIZE
test define before the tests are run. This starts the test with a block
device configured with a large block_size. To keep this from breaking
things the geometry definitions in the test and bench runners no longer
use default dependent definitions, instead defining everything
explicitly.
With block_size being so large, this makes some of the emubd operations
less performant, notably the --disk option for exposing block device
state during testing.
It would also be nice to use the copy-on-write backend of emubd for some
of the permutation testing, but since it operates on a block-by-block
basis, it doesn't really work when the block device is just one big
block.
- Removed ERASE_VALUE=-1 testing to save some time.
Since we never actually rewrite anything in these tests, this doesn't
really test anything different from the block device's default value.
- Removed checks for !rbyd.erased before calling lfsr_rbyd_commit.
This used to assert, but adding a check to lfsr_rbyd_commit simplifies
dependent logic and results in consistent behavior when
lfsr_rbyd_commit can't make progress. And since this check is now
expected behavior, the tests should test for this anyways.
- Correctly cleaned up dynamic allocations.
This matters for valgrind testing, and since many tests are ran in one
process we should be avoiding memory leaks when we can.
- Removed tests due for removal (have no value, replaced, etc).
Moving the main path flipping code to the end of the loop helped
organize things a bit better. Still, thanks to needing to track multiple
diverged paths, the state tracking ended up quite complicated. This
implementation uses 3-bits to store the current diverged state:
diverged=0 => not diverged
diverged=4 => diverged, on lower path
diverged=5 => diverged, on upper path
diverged=2 => diverged, found one tag, on lower path
diverged=3 => diverged, found one tag, on upper path
I also explored the early design using two variables (lt weight/gt weight)
instead of three (lower bound/upper bound/key), but it still has
problems:
- Keeping track of the found key in lfsr_rbyd_append requires an additional
variable, so the actual savings are unclear.
- Knowing when to diverge is a bit of a problem, before we only
needed one set of bounds and two different target keys, but with lt/gt
weights we'd need two sets of lt/gt weights.
We technically already pay the RAM cost for this, since we end up
needing two copies of the bounds after diverging, but deciding when
to update which lt/gt weights is complicated
There is a risk this whole thing is a premature optimization, but oh well,
I've probably been staring at this function for too long.
This increases the leb128 size from 4 bytes to 5 bytes for very little
gain, but 1 byte of RAM is not worth sweating over and this means fewer
surprises for a "32-bit" littlefs implementation.
If that 1 byte is worth saving, this should be configurable in the
future.
Preliminary comparisons show a minor improvement to code size at the cost
of stack usage. Really this boils down to a toss up, I'm currently
leaning towards this implementation of lfsr_rbyd_append as it has the
fewest moving parts, reusing the core rbyd loop for all mutating
operations.
Note these numbers are _very_ rough, there are likely some low-hanging
optimizations/cleanup and the rbyd size measurements are simply found
from fuzzing 1000 random permutations:
commit size rbyd size
code stack append/removes create/deletes
spiralrb: 3178 664 1609 1621
spiralb: 3058 664 1609 1606 (current)
2stepb: 3284 632 1609 1606
Well not really fixed, more just added an assert to make sure
lfsr_rbyd_lookup is not called with tag 0. Because our alt tags only
encode less-than-or-equal and greater-than, which can be flipped
trivially, it's not possible to encode removal of tag 0 during deletes.
Fortunately, this tag should already not exist for other pragmatic
reasons, it was just used as the initial value for traversals, where it
could cause this bug.
Christopher Haster