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.
If we combine rbyd ids and B-tree weights, we need 32-bit ids since this
will eventually need to cover the full range of a file. This simply
doesn't fit into a single word anymore, unless littlefs uses 64-bit tags.
Generally not a great idea for a filesystem targeting even 8-bit
microcontrollers.
So here is a tag encoding that uses 3 leb128 words. This will likely
have more code cost and slightly more disk usage (we can no longer fit
tags into 2 bytes), though with most tags being alt pointers (O(m log m)
vs O(m)), this may not be that significant.
Note that we try to keep tags limited to 14-bits to avoid an extra leb128 byte,
which would likely affect all alt pointers. To pull this off we do away
with the subtype/suptype distinction, limiting in-tree tag types to
10-bits encoded on a per-suptype basis:
in-tree tags:
ttttttt ttt00rv
^--^^- 10-bit type
'|- removed bit
'- valid bit
iiii iiiiiii iiiiiii iiiiiii iiiiiii
^- n-bit id
lllllll lllllll lllllll lllllll
^- m-bit length
out-of-tree tags:
ttttttt ttt010v
^---^- 10-bit type
'- valid bit
0000000
lllllll lllllll lllllll lllllll
^- m-bit length
alt tags:
kkkkkkk kkk1dcv
^-^^^- 10-bit key
'||- direction bit
'|- color bit
'- valid bit
wwww wwwwwww wwwwwww wwwwwww wwwwwww
^- n-bit weight
jjjjjjj jjjjjjj jjjjjjj jjjjjjj
^- m-bit jump
The real pain is that with separate integers for id and tag, it no
longer makes sense to combine these into one big weight field. This
requires a significant rewrite.
The original idea was weighted B-trees composed out of weighted rbyds,
with the two weight systems being independent. Descent down the B-tree
uses the same technique in the current metadata data-structure of
searching for which branch to take during fetch, basically getting the
search for free (well, on top of the already required O(m) fetch
operation).
But this is fundamentally flawed. While file names provide an absolute
reference for finding matches, weights are relative references. So we
don't have enough information to do weight-based lookup during fetch.
This smells just like the relative-vs-absolute key issues that led to
rbyd vs rbd trees in the first place...
One option is to do rbyd traversals at each B-tree node to build the
necessary information to figure out the weights. But with rbyd
traversals taking O(m log m), this makes B-tree lookups O(log n * m log m),
and B-tree traversals a messy O(n log n * m log m), which is acceptable, but
disapointing for what will likely be the most common operation in the
filesystem.
But the rbyd trees _are_ already weighted. A better solution might be to
go back and rethink the seperation of B-tree weights and rbyd ids.
Unfortunately, with only 16-bits available for rbyd ids, this would
likely require a rewrite of how rbyd tags are encoded...
This deserves more scrutiny in the future, but early benchmarking shows
caching here to add significant cost, since we often load branches we
don't take.
There may be smarter things we can do, but ideas quickly get into
branch-prediction territory, which is very, very out of scope.
- Added both uattr (limited to 256) and id (limited to 65535) benchmarks
covering the main rbyd operations
- Fixed issue where --defines gets passed to the test/bench runners when
querying id-specific information. After changing the test/bench
runners to prioritize explicit defines, this causes problems for
recorded benchmark results and debug related things.
- In plot.py/plotmpl.py, made --by/-x/-y in subplots behave somewhat
reasonably, contributing to a global dataset and the figure's legend,
colors, etc, but only shown in the specified subplot. This is useful
mainly for showing different -y values on different subplots.
- In plot.py/plotmpl.py, added --labels to allow explicit configuration
of legend labels, much like --colors/--formats/--chars/etc. This
removes one of the main annoying needs for modifying benchmark results.
This gives us the full 16-bit range of ids (65536) instead of the much
smaller 12-bit range (4096) when limited to truncated 4-byte leb128s.
The real motivation for the truncated 4-byte leb128s is to keep the
wasted space in crc padding down, which this doesn't matter for.
In the future this may be configurable. Or maybe not. Only if truncated
leb128 tags prove to have value, at the moment it looks more like a
premature optimization if anything...
Either way we do need to test for overflowing this.
I'm still not sure this is the best decision, since it may add some
complexity to tag parsing, but making most crcs one byte may be valuable
since these exist in every single commit.
This gives tags three high-level encodings:
in-tree tags:
iiiiiii iiiiitt ttTTTTT TTT00rv
^----^--------^--^^- 16-bit id
'--------|--||- 4-bit suptype
'--||- 8-bit subtype
'|- removed bit
'- valid bit
lllllll lllllll lllllll lllllll
^- n-bit length
out-of-tree tags:
------- -----TT TTTTTTt ttt01pv
^----^--^^- 8-bit subtype
'--||- 4-bit suptype
'|- perturb bit
'- valid bit
lllllll lllllll lllllll lllllll
^- n-bit length
alt tags:
wwwwwww wwwwwww wwwwwww www1dcv
^-^^^- 28-bit weight
'||- direction bit
'|- color bit
'- valid bit
jjjjjjj jjjjjjj jjjjjjj jjjjjjj
^- n-bit jump
Having the location of the subtype flipped for crc tags vs tree tags is
unintuitive, but it makes more crc tags fit in a single byte, while
preserving expected tag ordering for tree tags.
The only case where crc tags don't fit in a single byte if is non-crc
checksums (sha256?) are added, at which point I expect the subtype to
indicate which checksum algorithm is in use.
- Caching is still presenting issues with the new requirements for
rbyd trees, in this case the default bd, with 64 byte progs, revealed
and issue where rcache could become outdated when reading from disk
while ignoring what's in the pcache.
It assumes the pcache will always override the rcache, but this is not
true after pcache is flushed.
This didn't happen before as the rcache and pcache don't
interact while writing in the previous implementation. Because of
these new requirements the caching system probably deserves a
rework...
- The quick tests for sublinear space utilization don't work when
prog_size is > a byte, fortunately we should always have NOR-like
geometry under test, so we can limit these asserts to NOR-like
geometry.
- Lots of problems fitting these tests into 512-byte block_size
geometries, which is a bit concerning. This may be a larger change
from the previous implementation than expected. This may deserve more
scrutiny at small block sizes to see how things fit, since the
sublinear space utilization doesn't really kick in at this scale...
On the other hand it may just be that these tests are too aggressive
for 512-byte block sizes, since they don't yet do compaction, which
should help with padding/crc overhead...
$ ./scripts/dbgrbyd.py disk 4096 0 -t
mdir 0x0, rev 1, size 121
off tag data (truncated)
0000005e: +-+-+--> uattr 0x01 4 aa aa aa aa ....
0000000f: | | '--> uattr 0x02 4 aa aa aa aa ....
0000001d: | '----> uattr 0x03 4 aa aa aa aa ....
0000002d: | .----> uattr 0x04 4 aa aa aa aa ....
0000003d: | | .--> uattr 0x05 4 aa aa aa aa ....
0000004f: '-+-+-+> uattr 0x06 4 aa aa aa aa ....
00000004: '> uattr 0x07 4 aa aa aa aa ....
Unfortunately this tree can end up a bit confusing when alt pointers
live in unrelated search paths...
Toying around with the idea that since rbyd trees have strict height
gaurantees after compaction (2*log2(n)+1), we can proactively calculate
the maximum on-disk space required for a worst case tree+leb128
encoding.
This would _greatly_ simplify things such as metadata compaction and
splitting, and allow unstorable file metadata (too many custom
attributes) to error early.
One issue is that this calculated worst case will likely be ~4-5x worst
than the actual encoding due to leb128 compression. Though this may be an
acceptable tradeoff for the simplification and more reliable behavior.
Previously the subtype was encoded above the suptype. This was an issue
if you wanted to, say, traverse all tags in a given suptype.
I'm not sure yet if this sort of functionality is needed, it may be
useful for cleaning up/replacing classes of tags, such as file struct
tags, but not sure yet. At the very least is avoids unintuitive tag
ordering in the tree, which could potential cause problems for
create/deletes.
New encoding:
tags:
iiiiiii iiiiitt ttTTTTT TTT0trv
^----^--------^-^^^- 16-bit id
'--------|-'||- 5-bit suptype (split)
'--||- 8-bit subtype
'|- perturb/remove bit
'- valid bit
lllllll lllllll lllllll lllllll
^- n-bit length
alts:
wwwwwww wwwwwww wwwwwww www1dcv
^^^-^- 28-bit weight
'|-|- color bit
'-|- direction bit
'- valid bit
jjjjjjj jjjjjjj jjjjjjj jjjjjjj
^- n-bit jump
Also a large amount of name changes and other cleanup.
The idea here is to just use a separate variable for found tags, instead
of stealing a bit for tracking which tags were found.
I was hoping this would simplify more operations on the tags to avoid
some masking operations, but it doesn't seem like it.
Tag removal is basically a range-delete of one that doesn't change
the tree weights. This deduplicates the two methods of deleting tags and
completely gets rid of tombstoning.
Note we still need a "removed tag" encoding so that we can invalidate
tags that may be found during fetch operations. Fortunately this
encoding is basically free due to overlap with alt encoding.
- Unless there is a bug, rbyd trees should be strictly <= (2*log2(n)+1)
in height. The extra +1 from traditional red-black trees is due to the
introduced to-be-pruned alt, but since we clean those up as soon as we
can, only one will ever exist in any search path.
This also holds true with range deletion, however the definition of n
changes to the number of tree operations. This is the same for tombstoning.
- Delete-all recovery is a bit tricky because we have no tree at that
point, which is weird for an append-only data-structure.
Christopher Haster