This was a simple typo. Unfortunately went unnoticed because the
lingering dataset assigned in the above for loop made the results look
mostly correct. Yay.
Before this, the only option for ordering the legend was by specifying
explicit -L/--add-label labels. This works for the most part, but
doesn't cover the case where you don't know the parameterization of the
input data.
And we already have -s/-S flags in other csv scripts, so it makes sense
to adopt them in plot.py/plotmpl.py to allow sorting by one or more
explicit fields.
Note that -s/-S can be combined with explicit -L/--add-labels to order
datasets with the same sort field:
$ ./scripts/plot.py bench.csv \
-bBLOCK_SIZE \
-xn \
-ybench_readed \
-ybench_proged \
-ybench_erased \
--legend \
-sBLOCK_SIZE \
-L'*,bench_readed=bs=%(BLOCK_SIZE)s' \
-L'*,bench_proged=' \
-L'*,bench_erased='
---
Unfortunately this conflicted with -s/--sleep, which is a common flag in
the ascii-art scripts. This was bound to conflict with -s/--sort
eventually, so a came up with some alternatives:
- -s/--sleep -> -~/--sleep
- -S/--coalesce -> -+/--coalesce
But I'll admit I'm not the happiest about these...
Whoops! A missing splat repetition here meant we only ever accepted
floats with a single digit of precision and no e/E exponents.
Humorously this went unnoticed because our scripts were only
_outputting_ single digit floats, but now that that's fixed, float
parsing also needs a fix.
Fixed by allowing >1 digit of precision in our CsvFloat regex.
This adds __csv__ methods to all Csv* classes to indicate how to write
csv/json output, and adopts Python's default float repr. As a plus, this
also lets us use "inf" for infinity in csv/json files, avoiding
potential unicode issues.
Before this we were reusing __str__ for both table rendering and
csv/json writing, which rounded to a single decimal digit! This made
float output pretty much useless outside of trivial cases.
---
Note Python apparently does some of its own rounding (1/10 -> 0.1?), so
the result may still not be round-trippable, but this is probably fine
for our somewhat hack-infested csv scripts.
So now the hidden variants of field specifiers can be used to manipulate
by fields and field fields without implying a complete field set:
$ ./scripts/csv.py lfs.code.csv \
-Bsubsystem=lfsr_file -Dfunction='lfsr_file_*' \
-fcode_size
Is the same as:
$ ./scripts/csv.py lfs.code.csv \
-bfile -bsubsystem=lfsr_file -Dfunction='lfsr_file_*' \
-fcode_size
Attempting to use -b/--by here would delete/merge the file field, as
cvs.py assumes -b/-f specify all of the relevant field type.
Note that fields can also be explicitly deleted with -D/--define's new
glob support:
$ ./scripts/csv.py lfs.code.csv -Dfile='*' -fcode_size
---
This solves an annoying problem specific to csv.py, where manipulating
by fields and field fields would often force you to specify all relevant
-b/-f fields. With how benchmarks are parameterized, this list ends up
_looong_.
It's a bit of a hack/abuse of the hidden flags, but the alternative
would be field globbing, which 1. would be a real pain-in-the-ass to
implement, and 2. affect almost all of the scripts. Reusing the hidden
flags for this keeps the complexity limited to csv.py.
Globs in CLI attrs (-L'*=bs=%(bs)s' for example), have been remarkably
useful. It makes sense to extend this to the other flags that match
against CSV fields, though this does add complexity to a large number of
smaller scripts.
- -D/--define can now use globs when filtering:
$ ./scripts/code.py lfs.o -Dfunction='lfsr_file_*'
-D/--define already accepted a comma-separated list of options, so
extending this to globs makes sense.
Note this differs from test.py/bench.py's -D/--define. Globbing in
test.py/bench.py wouldn't really work since -D/--define is generative,
not matching. But there's already other differences such as integer
parsing, range, etc. It's not worth making these perfectly consistent
as they are really two different tools that just happen to look the
same.
- -c/--compare now matches with globs when finding the compare entry:
$ ./scripts/code.py lfs.o -c'lfs*_file_sync'
This is quite a bit less useful that -D/--define, but makes sense for
consistency.
Note -c/--compare just chooses the first match. It doesn't really make
sense to compare against multiple entries.
This raised the question of globs in the field specifiers themselves
(-f'bench_*' for example), but I'm rejecting this for now as I need to
draw the complexity/scope _somewhere_, and I'm worried it's already way
over on the too-complex side.
So, for now, field names must always be specified explicitly. Globbing
field names would add too much complexity. Especially considering how
many flags accept field names in these scripts.
I don't know how I completely missed that this doesn't actually work!
Using del _does_ work in Python's repl, but it makes sense the repl may
differ from actual function execution in this case.
The problem is Python still thinks the relevant builtin is a local
variables after deletion, raising an UnboundLocalError instead of
performing a global lookup. In theory this would work if the variable
could be made global, but since global/nonlocal statements are lifted,
Python complains with "SyntaxError: name 'list' is parameter and
global".
And that's A-Ok! Intentionally shadowing language builtins already puts
this code deep into ugly hacks territory.
This was broken:
$ ./scripts/plotmpl.py -L'*=bs=%(bs)s'
There may be a better way to organize this logic, but spamming if
statements works well enough.
This still forces the block_rows_ <= height invariant, but also prevents
ceiling errors from introducing blank rows.
I guess the simplest solution is the best one, eh?
This carves out two more bits in cksum tags to store the "phase" of the
rbyd block (maybe the name is too fancy, this is just the lowest 2 bits
of the block address):
LFSR_TAG_CKSUM 0x300p v-11 ---- ---- -pqq
^ ^
| '-- phase bits
'---- perturb bit
The intention here is to catch mrootanchors that are "out-of-phase",
i.e. they've been shifted by a small number of blocks.
This can happen if we find the wrong mrootanchor (after, say, a magic
scan), and risks filesystem corruption:
formatted
.-----------------'-----------------.
mounted
.-----------------'-----------------.
.--------+--------+--------+--------+ ...
|(erased)| mroot |
| | anchor | ...
| | |
'--------+--------+--------+--------+ ...
Including the lower 2 bits of the block address in cksum tags avoids
this, for up to a 3 block shift (the maximum number of redund
mrootanchors).
---
Note that cksum tags really are the only place we could put these bits.
Anywhere else and they would interfere with the canonical cksum, which
would break error correction. By definition these need to be different
per block.
We include these phase bits in every cksum tag (because it's easier),
but these don't really say much about mdirs that are not the
mrootanchor. Non-anchor mdirs can have arbitrary block addresses,
therefore arbitrary phase bits.
You _might_ be able to do something interesting if you sort the rbyd
addresses and use the index as the phase bits, but that would add quite
a bit of code for questionable benefit...
You could argue this adds noise to our cksums, but:
1. 2 bits seems like a really small amount of noise
2. our cksums are just crc32cs
3. the phase bits humorously never change when you rewrite a block
---
As with any feature this adds code, but only a small amount. I think
it's worth the extra protection:
code stack ctx
before: 35792 2368 636
after: 35824 (+0.1%) 2368 (+0.0%) 636 (+0.0%)
Also added test_mount_incompat_out_of_phase to test this.
The dbg scripts _don't_ error (block mismatch seems likely when
debugging), but dbgrbyd.py at least adds phase mismatch notes in
-l/--log mode.
This drops the leading count/mode byte, and instead uses mid=0 to
terminate grms. This shaves off 1 bytes from grmdeltas.
Previously, we needed the count/mode byte for a couple reasons:
- We needed to know the number of grm entries somehow, and there wasn't
always an obvious sentinel value. mid=-1, for example, is
unrepresentable with our unsigned leb128 encoding.
But now that development has settled, we can use mid=0.0 to figure out
the end-of-queue. mid=0.0 should always map to the root bookmark,
which doesn't make sense to delete, so it makes for a reasonable null
terminator here.
- It provided a route for future grm extensions, which could use the >2
count/mode encodings.
But I think we can use additional grm tag encodings for this.
There's only one gdelta tag so far, but the current plan for future
gdelta tags is to carve out the bottom 2 bits for redund like we do
with the struct tags:
LFSR_TAG_GDELTA 0x01tt v--- ---1 -ttt ttrr
LFSR_TAG_GRMDELTA 0x0100 v--- ---1 ---- ----
LFSR_TAG_GBMAPDELTA 0x0104 v--- ---1 ---- -1rr
LFSR_TAG_GDDTREEDELTA 0x0108 v--- ---1 ---- 1-rr
LFSR_TAG_GPTREEDELTA 0x010c v--- ---1 ---- 11rr
...
Decoding is a bit more complicated for gstate, since we will need to
xor those bits if mutable, but this avoids needing a full byte just
for redund in every auxiliary tree.
Long story short, we can leverage the lower 2 bits of the grm tag for
future extensions using the same mechanism.
This may seem like a lot of effort for only a handful of bytes, but keep
in mind each gdelta lives in more-or-less every mdir in the filesystem.
Also saves a bit of code/ctx:
code stack ctx
before: 35772 2368 640
after: 35768 (-0.0%) 2368 (+0.0%) 636 (-0.6%)
This prevents some pretty unintuitive behavior with dbgbmap.py -H2 (the
default) in the terminal.
Consider before:
bd 4096x256, 7.8% mdir, 0.4% btree, 0.0% data
mm--------b-----mm--mm--mm--mmmmmmm--mm--mmmm-----------------------
Vs after:
bd 4096x256, 7.8% mdir, 0.4% btree, 0.0% data
m-----------------------------------b-mmmmmmmm----------------------
Compared to the original bmap (-H5):
bd 4096x256, 7.8% mdir, 0.4% btree, 0.0% data
mm------------------------------------------------------------------
--------------------------------------------------------------------
----------b-----mm--mm--mm--mmmmmmm--mm--mmmm-----------------------
--------------------------------------------------------------------
What's happening is dbgbmap.py is prioritizing aspect ratio over pixel
boundaries, so it's happy drawing a 4-row bmap to a 1-row Canvas. But of
course we can't see subpixels, so the result is quite confusing.
Prioritizing rows while tiling avoids this.
I was toying with making this look more like the mtree API in lfs.c (so
no lookupleaf/namelookupleaf, only lookup/namelookup), but dropped the
idea:
- It would be tedious
- The Mtree class's lookupleaf/namelookupleaf are also helpful for
returning inner btree nodes when printing debug info
- Not embedding mids in the Mdir class would complicate things
It's ok for these classes to not match littlefs's internal API
_exactly_. The goal is easy access for debug info, not to port the
filesystem to Python.
At least dropped Mtree.lookupnext, because that function really makes no
sense.
Now that we don't have to worry about name tag conflicts as much, we
can add name tags for things that aren't files.
This adds LFSR_TAG_BNAME for branch names, and LFSR_TAG_MNAME for mtree
names. Note that the upper 4 bits of the subtype match LFSR_TAG_BRANCH
and LFSR_TAG_MDIR respectively:
LFSR_TAG_BNAME 0x0200 v--- --1- ---- ----
LFSR_TAG_MNAME 0x0220 v--- --1- --1- ----
LFSR_TAG_BRANCH 0x030r v--- --11 ---- --rr
LFSR_TAG_MDIR 0x0324 v--- --11 --1- -1rr
The encoding is somewhat arbitrary, but I figured reserving ~31 types
for files is probably going to be plenty for littlefs. POSIX seems to
do just fine with only ~7 all these years, and I think custom attributes
will be more enticing for "niche" file types (symlinks, compressed
files, etc), given the easy backwards compatibility.
---
In addition to the debugging benefits, the new name tags let us stop
btree lookups on the first non-bname/branch tag. Previously we always
had to fetch the first struct tag as well to check if it was a branch.
In theory this saves one rbyd lookup, but in practice it's a bit muddy.
The problem is that there's two ways to use named btrees:
1. As buckets: mtree -> mdir -> mid
2. As a table: ddtree -> ddid
The only named btree we _currently_ have is the mtree. And the mtree
operates in bucket mode, with each mdir acting more-or-less as an
extension to the btree. So we end up needing to do the second tag lookup
anyways, and all we've done is complicated up the code.
But we will _eventually_ need the table mode for the ddtree, where we
care if the ddname is an exact match.
And returning the first tag is arguably the more "correct" internal API,
vs arbitrarily the first struct tag.
But then again this change is pretty pricey...
code stack ctx
before: 35732 2440 640
after: 35888 (+0.4%) 2480 (+1.6%) 640 (+0.0%)
---
It's worth noting the new BNAME/MNAME tags don't _require_ the btree
lookup changes (which is why we can get away with not touching the dbg
scripts). The previous algorithm of always checking for branch tags
still works.
Maybe there's an argument for conditionally using the previous API when
compiling without the ddtree, but that sounds horrendously messy...
Block conflict detection was originally implemented with non-dags in
mind. But now that dags are allowed, we shouldn't treat them as errors!
Instead, we only report blocks as conflicts if multiple references have
mismatching types.
This should still be very useful for debugging the upcoming bmap work.
Mainly to make room for some future planned stuff:
- Moved the mroot's redund bits from LFSR_TAG_GEOMETRY to
LFSR_TAG_MAGIC:
LFSR_TAG_MAGIC 0x003r v--- ---- --11 --rr
This has the benefit of living in a fixed location (off=0x5), which
may make mounting/debugging easier. It also makes LFSR_TAG_GEOMETRY
less of a special case (LFSR_TAG_MAGIC is already a _very_ special
case).
Unfortunately, this does get in the way of our previous magic=0x3
encoding. To compensate (and to avoid conflicts with LFSR_TAG_NULL),
I've added the 0x3_ prefix. This has the funny side-effect of
rendering redunds 0-3 as ascii 0-3 (0x30-0x33), which is a complete
accident but may actually be useful when debugging.
Currently all config tags fit in the 0x3_ prefix, which is nice for
debugging but not a hard requirement.
- Flipped LFSR_TAG_FILELIMIT/NAMELIMIT:
LFSR_TAG_FILELIMIT 0x0039 v--- ---- --11 1--1
LFSR_TAG_NAMELIMIT 0x003a v--- ---- --11 1-1-
The file limit is a _bit_ more fundamental. It's effectively the
required integer size for the filesystem.
These may also be followed by LFSR_TAG_ATTRLIMIT based on how future
attr revisits go.
- Rearranged struct tags so that LFSR_TAG_BRANCH = 0x300:
LFSR_TAG_BRANCH 0x030r v--- --11 ---- --rr
LFSR_TAG_DATA 0x0304 v--- --11 ---- -1--
LFSR_TAG_BLOCK 0x0308 v--- --11 ---- 1err
LFSR_TAG_DDKEY* 0x0310 v--- --11 ---1 ----
LFSR_TAG_DID 0x0314 v--- --11 ---1 -1--
LFSR_TAG_BSHRUB 0x0318 v--- --11 ---1 1---
LFSR_TAG_BTREE 0x031c v--- --11 ---1 11rr
LFSR_TAG_MROOT 0x032r v--- --11 --1- --rr
LFSR_TAG_MDIR 0x0324 v--- --11 --1- -1rr
LFSR_TAG_MTREE 0x032c v--- --11 --1- 11rr
*Planned
LFSR_TAG_BRANCH is a very special tag when it comes to bshrub/btree
traversal, so I think it deserves the subtype=0 slot.
This also just makes everything fit together better, and makes room
for the future planned ddkey tag.
Code changes minimal:
code stack ctx
before: 35728 2440 640
after: 35732 (+0.0%) 2440 (+0.0%) 640 (+0.0%)
This adds LFSR_TAG_ORPHAN, which simplifies quite a bit of the internal
stickynote handling.
Now that we don't have to worry about conflicts with future unknown
types, we can add whatever types we want internally. One useful one
is LFSR_TAG_ORPHAN, which lets us determine stickynote's orphan status
early (in lfsr_mdir_lookupnext and lfsr_mdir_namelookup):
- non-orphan stickynotes -> LFSR_TAG_STICKYNOTE
- orphan stickynotes -> LFSR_TAG_ORPHAN
This simplifies all the places where we need to check if a stickynote
really exists, which is most of the high-level functions.
One downside is that this makes stickynote _manipulation_ a bit more
delicate. lfsr_mdir_lookup(LFSR_TAG_ORPHAN) no longer works as expected,
for example.
Fortunately we can sidestep this issue by dropping down to
lfsr_rbyd_lookup when we need to interact with stickynotes directly,
skipping the is-orphan checks.
---
Saves a nice bit of code:
code stack ctx
before: 35984 2440 640
after: 35832 (-0.4%) 2440 (+0.0%) 640 (+0.0%)
It got a little muddy since this now include the unknown-type changes,
but here's the code diff from before we exposed LFSR_TYPE_STICKYNOTE to
users:
code stack ctx
before: 35740 2440 640
after: 35832 (+0.3%) 2440 (+0.0%) 640 (+0.0%)
This drops the requirement that all file types are introduced with a
related wcompat flag. Instead, the wcompat flag is only required if
modification _would_ leak resources, and we treat unknown file types as
though they are regular files.
This allows modification of unknown file types without the risk of
breaking anything.
To compare with before the unknown-type rework:
Before:
> Unknown file types are allowed and may leak resources if modified,
> so attempted modification (rename/remove) will error with
> LFS_ERR_NOTSUP.
Now:
> Unknown file types are allowed but must not leak resources if
> modified. If an unknown file type would leak resources, it should set
> a related wcompat flag to only allow mounting RDONLY.
Note this includes directories, which can leak bookmarks if removed, so
filesystems using directories should set the LFSR_WCOMPAT_DIR flag.
But we no longer need the LFSR_WCOMPAT_REG/LFSR_WCOMPAT_STICKYNOTE
flags.
---
The real tricky part was getting lfsr_rename to work with unknown types,
as this broke the invariant that we only ever commit tags we know about.
Fixing this required:
- Fetching the non-unknown-mapped tag in lfsr_rename
- Mapping all name tags to LFSR_TAG_NAME in lfsr_rbyd_appendrattr_
- Adopting LFSR_RATTR_NAME for bookmark name tags
This was broken by the above lfsr_rbyd_appendrattr_ change, but it's
probably good to handle these the same as other name tags anyways.
This adds a bit of code, but not enough that I think this isn't worth
it (or worth a build-time option):
code stack ctx
before: 35924 2440 640
after: 35992 (+0.0%) 2440 (+0.0%) 640 (+0.0%)
This changes how we approach unknown file types.
Before:
> Unknown file types are allowed and may leak resources if modified,
> so attempted modification (rename/remove) will error with
> LFS_ERR_NOTSUP.
Now:
> Unknown file types are only allowed in RDONLY mode. This avoids the
> whole leaking resources headache.
Additionally, unknown types are now mapped to LFS_TYPE_UNKNOWN, instead
of just being forwarded to the user. This allows us to add internal
types/tags to the LFSR_TAG_NAME type space without worrying about
conflicts with future types:
- reg -> LFS_TYPE_REG
- dir -> LFS_TYPE_DIR
- stickynote -> LFS_TYPE_STICKYNOTE
- everything else -> LFS_TYPE_UNKNOWN
Thinking about potential future types, it seems most (symlinks,
compressed files, etc) can be better implemented via custom attributes.
Using custom attributes doesn't mean the filesystem _can't_ inject
special behavior, and custom attributes allow for perfect backwards
compatibility.
So with future types less likely, forwarding type info to users is less
important (and potentially error prone). Instead, allowing on-disk +
internal types to be represented densely is much more useful.
And it avoids setting an upper bound on future types prematurely.
---
This also includes a minor rcompat/wcompat rework. Since we're probably
going to end up with 32-bit rcompat flags anyways, might as well make
them more human-readable (nibble-aligned):
LFS_RCOMPAT_NONSTANDARD 0x00000001 Non-standard filesystem format
LFS_RCOMPAT_WRONLY 0x00000002 Reading is disallowed
LFS_RCOMPAT_BMOSS 0x00000010 Files may use inlined data
LFS_RCOMPAT_BSPROUT 0x00000020 Files may use block pointers
LFS_RCOMPAT_BSHRUB 0x00000040 Files may use inlined btrees
LFS_RCOMPAT_BTREE 0x00000080 Files may use btrees
LFS_RCOMPAT_MMOSS 0x00000100 May use an inlined mdir
LFS_RCOMPAT_MSPROUT 0x00000200 May use an mdir pointer
LFS_RCOMPAT_MSHRUB 0x00000400 May use an inlined mtree
LFS_RCOMPAT_MTREE 0x00000800 May use an mdir btree
LFS_RCOMPAT_GRM 0x00001000 Global-remove in use
LFS_WCOMPAT_NONSTANDARD 0x00000001 Non-standard filesystem format
LFS_WCOMPAT_RDONLY 0x00000002 Writing is disallowed
LFS_WCOMPAT_REG 0x00000010 Regular file types in use
LFS_WCOMPAT_DIR 0x00000020 Directory file types in use
LFS_WCOMPAT_STICKYNOTE 0x00000040 Stickynote file types in use
LFS_WCOMPAT_GCKSUM 0x00001000 Global-checksum in use
---
Code changes:
code stack ctx
before: 35928 2440 640
after: 35924 (-0.0%) 2440 (+0.0%) 640 (+0.0%)
Now that LFS_TYPE_STICKYNOTE is a real type users can interact with, it
makes sense to group it with REG/DIR. This also has the side-effect of
making these contiguous.
---
LFSR_TAG_BOOKMARKs, however, are still hidden from the user. This
unfortunately means there will be a bit of a jump if we ever add
LFS_TYPE_SYMLINK in the future, but I'm starting to wonder if that's the
best way to approach symlinks in littlefs...
If instead LFS_TYPE_SYMLINKS were implied via custom attribute, you
could avoid the headache that comes with adding a new tag encoding, and
allow perfect compatibility with non-symlink drivers. Win win.
This seems like a better approach for _all_ of the theoretical future
types (compressed files, device files, etc), and avoids the risk of
oversaturating the type space.
---
This had a surprising impact on code for just a minor encoding tweak. I
guess the contiguousness pushed the compiler to use tables/ranges for
more things? Or maybe 3 vs 5 is just an easier constant to encode?
code stack ctx
before: 35952 2440 640
after: 35928 (-0.1%) 2440 (+0.0%) 640 (+0.0%)
This tweaks a number of extended revision count things:
- Added LFS_REVDBG, which adds debug info to revision counts.
This initializes the bottom 12 bits of every revision count with a
hint based on rbyd type, which may be useful when debugging:
- 68 69 21 v0 (hi!.) => mroot anchor
- 6d 72 7e v0 (mr~.) => mroot
- 6d 64 7e v0 (md~.) => mdir
- 62 74 7e v0 (bt~.) => file btree node
- 62 6d 7e v0 (bm~.) => mtree node
This may be overwritten by the recycle counter if it overlaps, worst
case the recycle counter takes up the entire revision count, but these
have been chosen to at least keep some info if partially overwritten.
To make this work required the LFS_i_INMTREE hack (yay global state),
but a hack for debug info isn't the end of the world.
Note we don't have control over data blocks, so there's always a
chance they end up containing what looks like one of the above
revision counts.
- Renamed LFS_NOISY -> LFS_REVNOISE
- LFS_REVDBG and LFS_REVNOISE are incompatible, so using both asserts.
This also frees up the theoretical 0x00000030 state for an additional
rev mode in the future.
- Adopted LFS_REVNOISE (and LFS_REVDBG) in btree nodes as well.
If you need rev noise, you probably want it in all rbyds/metadata
blocks, not just mdirs.
---
This had no effect on the default code size, but did affect
LFS_REVNOISE:
code stack ctx
before: 35688 2440 640
after: 35688 (+0.0%) 2440 (+0.0%) 640 (+0.0%)
revnoise before: 35744 2440 640
revnoise after: 35880 (+0.4%) 2440 (+0.0%) 640 (+0.0%)
default: 35688 2440 640
revdbg: 35912 (+0.6%) 2448 (+0.3%) 640 (+0.0%)
revnoise: 35880 (+0.5%) 2440 (+0.0%) 640 (+0.0%)
This was caused by including the shrub bit in the tag comparison in
Rbyd.lookup.
Fixed by adding an extra key mask (0xfff). Note this is already how
lfsr_rbyd_lookup works in lfs.c.
- Fixed Mtree.lookupleaf accepting mbid=0, which caused dbglfs.py to
double print all files with mbid=-1
- Fixed grm mids not being mapped to mbid=-1 and related orphan false
positives
I've made this mistake before!
One would think that it would be more interesting to show progs over
erases when they overlap, since progs always subset erases and show more
detail. However, erases occur much more rarely and are usually followed
by progs, so when rendering is low resolution (ascii) it's easy for
progs to completely cover up all erase operations.
Prioritizing erases prevents this.
At least this nuance is better documented this time around.
This was a surprising side-effect the script rework: Realizing the
internal btree/rbyd lookup APIs were awkwardly inconsistent and could be
improved with a couple tweaks:
- Adopted lookupleaf name for functions that return leaf rbyds/mdirs.
There's an argument this should be called lookupnextleaf, since it
returns the next bid, unlike lookup, but I'm going to ignore that
argument because:
1. A non-next lookupleaf doesn't really make sense for trees where
you don't have to fetch the leaf (the mtree)
2. It would be a bit too verbose
- Adopted commitleaf name for functions that accept leaf rbyds.
This makes the lfsr_bshrub_commit -> lfsr_btree_commit__ mess a bit
more readable.
- Strictly limited lookup and lookupnext to return rattrs, even in
complex trees like the mtree.
Most use cases will probably stick to the lookupleaf variants, but at
least the behavior will be consistent.
- Strictly limited lookup to expect a known bid/rid.
This only really matters for lfsr_btree/bshrub_lookup, which as a
quirk of their implementation _can_ lookup both bid + rattr at the
same time. But I don't think we'll need this functionality, and
limited the behavior may allow for future optimizations.
Note there is no lfsr_file_lookup. File btrees currently only ever
have a single leaf rattr, so this API doesn't really make sense.
Internal API changes:
- lfsr_btree_lookupnext_ -> lfsr_btree_lookupleaf
- lfsr_btree_lookupnext -> lfsr_btree_lookupnext
- lfsr_btree_lookup -> lfsr_btree_lookup
- added lfsr_btree_namelookupleaf
- lfsr_btree_namelookup -> lfsr_btree_namelookup
- lfsr_btree_commit__ -> lfsr_btree_commit_
- lfsr_btree_commit_ -> lfsr_btree_commitleaf
- lfsr_btree_commit -> lfsr_btree_commit
- added lfsr_bshrub_lookupleaf
- lfsr_bshrub_lookupnext -> lfsr_bshrub_lookupnext
- lfsr_bshrub_lookup -> lfsr_bshrub_lookup
- lfsr_bshrub_commit_ -> lfsr_bshrub_commitleaf
- lfsr_bshrub_commit -> lfsr_bshrub_commit
- lfsr_mtree_lookup -> lfsr_mtree_lookupleaf
- added lfsr_mtree_lookupnext
- added lfsr_mtree_lookup
- added lfsr_mtree_namelookupleaf
- lfsr_mtree_namelookup -> lfsr_mtree_namelookup
- added lfsr_file_lookupleaf
- lfsr_file_lookupnext -> lfsr_file_lookupnext
- added lfsr_file_commitleaf
- lfsr_file_commit -> lfsr_file_commit
Also added lookupnext to Mdir/Mtree in the dbg scripts.
Unfortunately this did add both code and stack, but only because of the
optional mdir returns in the mtree lookups:
code stack ctx
before: 35520 2440 636
after: 35548 (+0.1%) 2472 (+1.3%) 636 (+0.0%)
So mbid=0 now implies the mdir is not inlined.
Downsides:
- A bit more work to calculate
- May lose information due to masking everything when mtree.weight==0
- Risk of confusion when in-lfs.c state doesn't match (mbid=-1 is
implied by mtree.weight==0)
Upsides:
- Includes more information about the topology of the mtree
- Avoids multiple dbgmbids for the same physical mdir
Also added lfsr_dbgmbid and lfsr_dbgmrid to help make logging
easier/more consistent.
And updated dbg scripts.
So now:
(block_size)
mbits = nlog2(----------) = nlog2(block_size) - 3
( 8 )
Instead of:
( (block_size))
mbits = nlog2(floor(----------)) = nlog2(block_size & ~0x7) - 3
( ( 8 ))
This makes the post-log - 3 formula simpler, which we probably want to
prefer as it avoids a division. And ceiling is arguably more intuitive
corner case behavior.
This may seem like a minor detail, but because mbits is purely
block_size derived and not configurable, any quirks here will become
a permanent compatibility requirement.
And hey, it saves a couple bytes (I'm not really sure why, the division
should've been optimized to a shift):
code stack ctx
before: 35528 2440 636
after: 35520 (-0.0%) 2440 (+0.0%) 636 (+0.0%)
This replaces the previous fallback-to-what's-available behavior with
explicit flags:
- --tile-code - Tile based on code size (the default)
- --tile-stack - Tile based on stack limits
- --tile-frames - Tile based on stack frames
- --tile-ctx - Tile based on function context
- --tile-1 - Tile functions evenly
This has the benefit of 1. being easier to toggle, 2. being explicit,
and 3. allowing code/stack/ctx in punescapes (titles, labels, etc).
There is an interesting question if --no-stack should be implicit, since
showing two stack treemaps may be confusing, but I think that's trying
to be too clever. Instead I just added the -S/--no-stack shortform to
make it easier to toggle.
Also updated ctx.py's description string. Probably need to check what
else is out of date in other scripts as well.
Now that I know my way around the weirdness that is Python's class
scope, this just required another function indirection to capture the
class-level dicts correctly.
I was considering using the __subclasses__ trick, but it seems like that
would actually be more complicated here.
This is limited to dbgle32.py, dbgleb128.py, and dbgtag.py for now.
This more closely matches how littlefs behaves, in that we read a
bounded number of bytes before leb128 decoding. This minimizes bugs
related to leb128 overflow and avoids reading inherently undecodable
data.
The previous unbounded behavior is still available with -w0.
Note this gives dbgle32.py much more flexibility in that it can now
decode other integer widths. Uh, ignore the name for now. At least it's
self documenting that the default is 32-bits...
---
Also fixed a bug in fromleb128 where size was reported incorrectly on
offset + truncated leb128.
Do you see the O(n^2) behavior in this loop?
j = 0
while j < len(data):
word, d = fromleb(data[j:])
j += d
The slice, data[j:], creates a O(n) copy every iteration of the loop.
A bit tricky. Or at least I found it tricky to notice. Maybe because
array indexing being cheap is baked into my brain...
Long story short, this repeated slicing resulted in O(n^2) behavior in
Rbyd.fetch and probably some other functions. Even though we don't care
_too_ much about performance in these scripts, having Rbyd.fetch run in
O(n^2) isn't great.
Tweaking all from* functions to take an optional index solves this, at
least on paper.
---
In practice I didn't actually find any measurable performance gain. I
guess array slicing in Python is optimized enough that the constant
factor takes over?
(Maybe it's being helped by us limiting Rbyd.fetch to block_size in most
scripts? I haven't tested NAND block sizes yet...)
Still, it's good to at least know this isn't a bottleneck.
These mimic dbgtag.py, but provide debugging for the lower-level integer
primitives in littlefs:
$ ./scripts/dbgleb128.py -x 2a 80 80 a8 01
2a 42
80 80 a8 01 2752512
$ ./scripts/dbgle32.py -x 2a 00 00 00 00 00 2a 00
2a 00 00 00 42
00 00 2a 00 2752512
dbgleb128.py is probably going to be more useful, but I figured we might
as well include both for completeness. Though dbgle32.py is begging to
be generalized.
This just gives dbgtag.py a few more bells and whistles that may be
useful:
- Can now parse multiple tags from hex:
$ ./scripts/dbgtag.py -x 71 01 01 01 12 02 02 02
71 01 01 01 altrgt 0x101 w1 -1
12 02 02 02 shrubdir w2 2
Note this _does_ skip attached data, which risks some confusion but
not skipping attached data will probably end up printing a bunch of
garbage for most use cases:
$ ./scripts/dbgtag.py -x 01 01 01 04 02 02 02 02 03 03 03 03
01 01 01 04 gdelta 0x01 w1 4
03 03 03 03 struct 0x03 w3 3
- Included hex in output. This is helpful for learning about the tag
encoding and also helps identify tags when parsing multiple tags.
I considered also included offsets, which might help with
understanding attached data, but decided it would be too noisy. At
some point you should probably jump to dbgrbyd.py anyways...
- Added -i/--input to read tags from a file. This is roughly the same as
-x/--hex, but allows piping from other scripts:
$ ./scripts/dbgcat.py disk -b4096 0 -n4,8 | ./scripts/dbgtag.py -i-
80 03 00 08 magic 8
Note this reads the entire file in before processing. We'd need to fit
everything into RAM anyways to figure out padding.
This matches the behavior of paths and helps figure out which string is
associated with which crc32c/parity when checksumming multiple strings:
$ ./scripts/crc32c.py -s hi hello
f59dd9c2 hi
9a71bb4c hello
It also might help clear up confusion if someone forgets to quote a
string with spaces inside it.
- Added TreeArt __bool__ and __len__.
This was causing a crash in _treeartfrommtreertree when rtree was
empty.
The code was not updated in the set -> TreeArt class transition, and
went unnoticed because it's unlikely to be hit unless the filesystem
is corrupt.
Fortunately(?) realtime rendering creates a bunch of transiently
corrupt filesystem images.
- Tweaked lookupleaf to not include mroots in their own paths.
This matches the behavior of leaf mdirs, and is intentionally
different from btree's lookupleaf which needs to lookup the leaf rattr
to terminate.
- Tweaked leaves to not remove the last path entry if it is an mdir.
This hid the previous lookupleaf inconsistency. We only remove the
last rbyd from the path because it is redundant, and for mdirs/mroots
it should never be redundant.
I ended up just replacing the corrupt check with an explicit check
that the rbyd is redundant. This should be more precise and avoid
issues like this in the future.
Also adopted explicit redundant checks in Btree.leaves and
Lfs.File.leaves.
Two new tricks:
1. Hide the cursor while redrawing the ring buffer.
2. Build up the entire redraw in RAM first, and render everything in a
single write call.
These _mostly_ get rid of the cursor flickering issues in rapidly
updating scripts.
This allows -w to provide a shortform flag for both --wear and
--block-cycles, depending on if you include a cycles argument:
- -w => --wear
- -w100 => --block-cycles=100
I was originally hesitant to add this since it's inconsistent from
--read/--prog/--erase, which can't have shortforms due to flag
conflicts, but --wear is probably a special enough case.
- CsvInt.x -> CsvInt.a
- CsvFloat.x -> CsvFloat.a
- Rev.x -> Rev.a
This matches CsvFrac.a (paired with CsvFrac.b), and avoids confusion
with x/y variables such as Tile.x and Tile.y.
The other contender was .v, since these are cs*v* related types, but
sticking with .a gets the point across that the name really doesn't have
any meaning.
There's also some irony that we're forcing namedtuples to have
meaningless names, but it is useful to have a quick accessor for the
internal value.
This prefix was extremely arbitrary anyways.
The prefix Csv* has slightly more meaning than R*, since these scripts
interact with .csv files quite a bit, and it avoids confusion with
rbyd-related things such as Rattr, Ralt, etc.
This affects the table renderers as well as csv.py's ratio expr.
This is a bit more correct, handwaving 0/0 (mapping 0/0 -> 100% is
useful for cov.py, please don't kill me mathematicians):
frac(1,0) => 1/0 (∞%)
frac(0,0) => 0/0 (100.0%)
frac(0,1) => 0/1 (0.0%)
So now the result scripts always require -d/--diff to diff:
- before: ./scripts/csv.py a.csv -pb.csv
- after: ./scripts/csv.py a.csv -db.csv -p
For a couple reasons:
- Easier to toggle
- Simpler internally to only have one diff path flag
- The previous behavior was a bit unintuitive
After all, who doesn't love a good bit of flickering.
I think I was trying to be too clever, so reverting.
Printing these with no padding is the simplest solution, provides the
best information density, and worst case you can always add -s1 to limit
the update frequency if flickering is hurting readability.
This automatically minimizes the status strings without flickering, all
it took was a bit of ~*global state*~.
---
If I'm remembering correctly, this was actually how tracebd.py used to
work before dbgbmap.py was added. The idea was dropped with dbgbmap.py
since dbgbmap.py relied on watch.py for real-time rendering and couldn't
persist state.
But now dbgbmap.py has its own -k/--keep-open flag, so that's not a
problem.
This isn't true, especially for dbgbmap.py, 100% is very possible in
filesystems with small files. But by limiting padding to 99.9%, we avoid
the annoying wasted space caused by the rare but occasional 100.0%.
No one is realistically ever going to use this.
Ascii art is just too low resolution, trying to pad anything just wastes
terminal space. So we might as well not support --padding and save on
the additional corner cases.
Worst case, in the future we can always find this commit and revert
things.
This matches dbgbmap.py and fits in with the other dbg scripts.
The choice of tracebd.py for the name was arbitrary anyways, we just
needed something that wouldn't conflict with other scripts.
Should've probably been two commits, but:
1. Cleaned up tracebd.py's header generation to be consistent with
dbgbmap.py and other scripts.
Percentage fields are now consistently floats in all scripts,
allowing user-specified precision when punescaping.
2. Moved header generation up to where we still have the disk open (in
dbgbmap[d3].py), to avoid issues with lazy Lfs attrs trying to access
the disk after it's been closed.
Found while testing with --title='cksum %(cksum)08x'. Lfs tries to
validate the gcksum last minute and things break.
This can be hit when dealing with very small maps, which is common since
we're rendering to the terminal. Not crashing here at least allows the
header/usage string to be shown.
Christopher Haster