This should either have checked diff_result==None, or we should be
mapping diff_result=None => diff_result_=None. To be safe I've done
both.
This was a nasty typo and I only noticed because ctx.py stopped printing
"cycle detected" for our linked-lists (which are expected to be cyclic).
It felt weird that adding hidden fields required changing existing
flags unrelated to the field you actually want to affect, and the
upper/lower flag thing seems to work well for -s/-S sooo...
- Replaced -l/--label with -B/--hidden-by for by fields that can
be hidden from the table renderer.
- Added -F/--hidden-field as a similar thing for field fields.
- Better integrated -i/--enumerate into by fields, now these actually
maintain related order. And of course added a matching
-I/--hidden-enumerate flag.
The only downside is this is eating a lot of flag names.. But one of the
nice thing about limiting this complexity to csv.py is it avoids these
flag names cluttering up the other result scripts.
---
The -F/--hidden-fields flag I'm not so sure about, since field exprs
can't really reference each other (single pass). But it does provide
symmetry with -B/--hidden-by, and reserves the name in case hidden field
fields are more useful in the future.
Unfortunately it _is_ annoyingly inconsistent with other hidden fields
(-S/--sort, -D/--define, etc) in that it does end up in output csvs...
But this script is already feeling way over-engineered as is.
Kind of a complicated corner case, but this shows up if you try to sort
by fields as numbers and not as strings. In theory this is possible by
creating a hidden sort field with a typed expr:
$ ./scripts/csv.py test.csv -bi -bfunction -Si=i
But we weren't typechecking sort fields that already exist in the by
fields, since these are usually strings.
This fix is to make sure all exprs are in the typechecked fields, even
if they are already in by fields. There's no real cost to this.
---
Note this version does _not_ typecheck i, and sorts by string:
$ ./scripts/csv.py test.csv -bi -bfunction -Si
This raises the question, should we always sort by string by default?
I don't think so. It's easy to miss the difference, and a typecheck
error is a lot safer than incorrect sorting.
So this will sort by number, with i as a hidden field:
$ ./scripts/csv.py test.csv -bfunction -Si
If you want to sort by string with a hidden field, this is still
possible with -l/--label:
$ ./scripts/csv.py test.csv -bi -lfunction -Si
There's an ordering issue with hotifying and folding when we have
multiple foldable results with children. This was hard to notice since
most of the recursive scripts have unique results, but it _is_ an issue
for perf.py/perfbd.py, which rely on result folding to merge samples.
The fix is to fold _before_ hotifying.
We could fold multiple times to avoid changing the behavior of the
result scripts, but instead I've just moved the folding in the table
renderer up into the relevant main functions. This means 1. we only fold
once, and 2. folding affects outputted csv/json files.
I'm a bit on the fence about this behavior change, but it is a bit more
consistent with how -r/--hot, -z/--depth, etc, affect both table and
csv/json results consistently.
Maybe we should move towards the table render always reflecting the
csv/json results? Most csv/json usage is with -q/--quiet anyways...
---
This does create a new risk in that the table renderer can hide results
if they aren't folded first.
To hopefully avoid this I've added an assert in the table renderer if it
notices results being hidden.
I guess in addition to its other utilities, csv.py is now also turning
into a sort of man database for some of the more complicated APIs in the
scripts:
./csv.py --help
./csv.py --help-exprs
./csv.py --help-mods
It's a bit minimal, but better than nothing.
Also dropped the %c modifier because this never actually worked.
This gives csv.py access to a hidden feature in our table renderer used
by some of the other scripts: fields that affect by-field grouping, but
aren't actually printed.
For example, this prevents summing same named functions in different
files, but only shows the function name in the table render:
$ ./scripts/csv.py lfs.code.csv -bfile -bfunction -lfunction
function size
lfs_alloc 398
lfs_alloc_discard 31
lfs_alloc_findfree 77
...
This is especially useful when enumerating results. For example, this
prevents any summing without extra table noise:
$ ./scripts/csv.py lfs.code.csv -i -bfunction -fsize -lfunction
function size
lfs_alloc 398
lfs_alloc_discard 31
lfs_alloc_findfree 77
...
I also tweaked -b/--by field defaults a bit to account to
enumerate/label fields a bit better.
This removes most of the special behavior around how -r/--hot and
-i/--enumerate interact. This does mean -r/--hot risks folding results
if -i/--enumerate is not specified, but this is _technically_ a valid
operation.
For most of the recursive result scripts, I've replaced the "i" field
with separate "z" and "i" fields for depth and field number, which I
think is a bit more informative/useful.
I've also added a default-hidden "off" field to structs.py/ctx.py, since
we have that info available. I considered replacing "i" with this, but
decided against it since non-zero offsets for union members would risk
being confusing/mistake prone.
Guh
This may have been more work than I expected. The goal was to allowing
passing recursive results (callgraph info, structs, etc) between
scripts, which is simply not possible with csv files.
Unfortunately, this raised a number of questions: What happens if a
script receives recursive results? -d/--diff with recursive results?
How to prevent folding of ordered results (structs, hot, etc) in piped
scripts? etc.
And ended up with a significant rewrite of most of the result scripts'
internals.
Key changes:
- Most result scripts now support -O/--output-json in addition to
-o/--json, with -O/--output-json including any recursive results in
the "children" field.
- Most result scripts now support both csv and json as input to relevant
flags: -u/--use, -d/--diff, -p/--percent. This is accomplished by
looking for a '[' as the first character to decide if an input file is
json or csv.
Technically this breaks if your json has leading whitespace, but why
would you ever keep whitespace around in json? The human-editability
of json was already ruined the moment comments were disallowed.
- csv.py requires all fields to be explicitly defined, so added
-i/--enumerate, -Z/--children, and -N/--notes. At least we can provide
some reasonable defaults so you shouldn't usually need to type out the
whole field.
- Notably, the rendering scripts (plot.py, treemapd3.py, etc) and
test/bench scripts do _not_ support json. csv.py can always convert
to/from json when needed.
- The table renderer now supports diffing recursive results, which is
nice for seeing how the hot path changed in stack.py/perf.py/etc.
- Moved the -r/--hot logic up into main, so it also affects the
outputted results. Note it is impossible for -z/--depth to _not_
affect the outputted results.
- We now sort in one pass, which is in theory more efficient.
- Renamed -t/--hot -> -r/--hot and -R/--reverse-hot, matching -s/-S.
- Fixed an issue with -S/--reverse-sort where only the short form was
actually reversed (I misunderstood what argparse passes to Action
classes).
- csv.py now supports json input/output, which is funny.
In addition to providing more functionality for creating -b/--by fields,
this lets us remove strings from the expr parser. Strings had no
well-defined operations and could best be described as an "ugly wart".
Maybe we'll reintroduce string exprs in the future, but for now csv.py's
-f/--field fields will be limited to numeric values.
As an extra plus, no more excessive quoting when injecting new -b/--by
fields.
---
This also fixed sorting on non-field fields, which was apparently
broken. Or at least mostly useless since it was defaulting to string
sorting.
This adopts the Attr rework for the --add-xticklabel and
--add-yticklabel flags.
Sort of.
These require a bit of special behavior to make work, but should at
least be externally consistent with the other Attr flags.
Instead of assigning to by-field groups, --add-xticklabel/yticklabel
assign to the relevant x/y coord:
$ ./scripts/plotmpl.py \
--add-xticklabel='0=zero' \
--add-yticklabel='100=one-hundred'
The real power comes from our % modifiers. As a special case,
--add-xticklabel/yticklabel can reference the special x/y field, which
represents the current x/y coord:
$ ./scripts/plotmpl.py --y2 --yticks=5 --add-yticklabel='%(y)d KiB'
Combined with format specifiers, this allows for quite a bit:
$ ./scripts/plotmpl.py --y2 --yticks=5 --add-yticklabel='0x%(y)04x'
---
Note that plot.py only shows the min/max x/yticks, so plot.py only
accepts indexed --add-xticklabel/yticklabels, and will error if the
assigning variant is used.
Unifying these complicated attr-assigning flags across all the scripts
is the main benefit of the new internal Attr system.
The only tricky bit is we need to somehow keep track of all input fields
in case % modifiers reference fields, when we could previously discard
non-data fields.
Tricky but doable.
Updated flags:
- -L/--label -> -L/--add-label
- --colors -> -C/--add-color
- --formats -> -F/--add-format
- --chars -> -*/--add-char/--chars
- --line-chars -> -_/--add-line-char/--line-chars
I've also tweaked Attr to accept glob matches when figuring out group
assignments. This is useful for matching slightly different, but
similarly named results in our benchmark scripts.
There's probably a clever way to do this by injecting new by fields with
csv.py, but just adding globbing is simpler and makes attr assignment
even more flexible.
No more special indexed attrs at the top-level, now all attrs are
indexed, even if assigned to a specific group.
This just makes it so group-specific cycles are possible:
$ ./scripts/treemap.py -Clfs.c=red -Clfs.c=green
Now, instead of specifying a specific field or comma-separated set of
order-defined constants, -L/--add-label, -C/--add-color, and
-./--add-char/--chars accept a by-field group assignment similar to
-L/--label in plotmpl.py.
I also reworked our % modifiers to behave a bit more like printf
modifiers with optional field targets.
It gets a bit complicated, but this ends up extremely flexible:
- Assign to a specific group:
$ ./scripts/treemap.py -Clfs.c,lfsr_format=orange
- Note this is heirarchical, with more specific groups taking priority:
$ ./scripts/treemap.py -Clfs.c=blue -Clfs.c,lfsr_format=orange
- We can still get the order-assigned behavior by specifying multiple
options, but note there is no longer a comma ambiguity! This is useful
if you want to specify a palette and don't care which dataset gets
which attr:
$ ./scripts/treemap.py -Cred -Cgreen -Cblue
- Mix and match:
$ ./scripts/treemap.py -Cred -Cgreen -Cblue -Clfsr_format=orange
- And with the new % modifiers, we can still use labels stored in a
field:
$ ./scripts/treemap.py -L'%(label_field)s'
- -./--add-char/--chars in treemap.py is a bit of a special case. Since
it only accepts single characters, we can still accept multiple
options with a single flag without having to worry about ambiguities:
$ ./scripts/treemap.py -.asdf
Well, unless you want to include a literal '='. This is possible, but
a bit messy:
$ ./scripts/treemap.py -.as -.=== -.df
Yes that is 3 equal signs... One for argparse, one for the assignment,
one for the '=' literal.
This one is minor, but nice for terseness.
A painful lesson learned from plot[mpl].py: we should never implicitly
sum results in a late-stage rendering script. It just makes it way to
easy to accidentally render incorrect/misleading data, while being
difficult to notice.
We should always render redundant results as redundant results.
If the redundant results are an error, this hopefully makes the problem
more obvious to the user. And if the user really does want summed
results, they can always use csv.py as an intermediate step:
$ ./scripts/treemap.py \
<(./scripts/csv.py lfs.code.csv -bfile -fsize -q -o-)
-fsize
This adds --rectify for a parent-aspect-ratio-preserving --squarify
variant, reverting squarify to try to match the aspect ratio of a
square (1:1).
I can see arguments for both of these. On one hand --squarify makes the
squarest squares, which according to Mark Bruls et al's paper on the
topic is easier visually compare. On the other hand --rectify may be
more visually pleasing and fit into parent tiles better.
d3 allows for any ratio, but at the moment I'm not seeing a strong
reason for the extra parameter.
Like treemap.py, but outputting an svg file, which is quite a bit more
useful.
Things svg is _not_:
- A simple vector graphics format
Things svg _is_:
- A surprisingly powerful high-level graphics language.
I might have to use svgs as an output format more often. It's
surprisingly easy to generate graphics without worrying about low-level
rendering details.
---
Aside from the extra flags for svg details like font, padding,
background colors, etc, the main difference between treemap.py and
treemapd3.py is the addition of the --nested mode, which renders a
containing tile for each recursive group (each -b/--by field).
There's no way --nested would've worked in treemap.py. The main benefit
is the extra labels per subgroup, which are already hard enough to read
in treemap.py.
Other than that, treemapd3.py is mostly the same as treemap.py, but with
a resolution that's actually readable.
Based on the d3 javascript library (https://d3js.org), treemap.py
renders heirarchical data as ascii art:
$ ./scripts/treemap.py lfs.code.csv \
-bfunction -fsize --chars=asdf -W60 -H8
total 65454, avg 369 +-366.8σ, min 3, max 4990
aaaassssddddddaaaadddddssddfffaaadfffaassaassfasssdfdfsddfad
aaaassssddddddaaaadddddssddfffaaadfffaassdfaafasssdfdfsddfsf
aaaassssddddddaaaafffffssddfffsssdaaaddffdfaadfaaasdfafaasfa
aaaassssddddddaaaafffffaaaddddsssaassddffdfaaffssfssfsfadffa
aaaassssffffffssssfffffaaaddddsssaassssffddffffssfdffsadfsad
aaaassssffffffssssaaaaasssffffddfaassssaaassdaaddadffsadadad
aaaassssffffffssssaaaaasssffffddfddffddssassdfassadffsadaffa
aaaassssffffffssssaaaaasssffffddfddffddssassdfaddsdadasfsada
(Normally this is also colored, but you know.)
I've been playing around with d3 to try to better visualize code costs
in littlefs, and it's been quite neat. I figured it would be useful to
directly integrate a similar treemap renderer into our result scripts.
That being said, this ascii rendering is probably too difficult to parse
for any non-trivial data. I'm also working on an svg-based renderer, so
treemap.py is really just for in-terminal previews and an exercise to
understand the underlying algorithms, similar to plot.py/plotmpl.py.
You forget one script, running in the background, hogging a whole
core, and suddenly watch's default 2 second sleep time makes a lot more
sense...
One of the main motivators for watch.py _was_ for shorter sleep times,
short enough to render realtime animations (watch is limited to 0.1
seconds for some reason?), but this doesn't mean it needs to be the
default. This can still be accomplished by explicitly specifying
-s/--sleep, and we probably don't want the default to hog all the CPU.
The use case for fast sleeps has been mostly replaced by -k/--keep-open
anyways.
For tailpipe.py and tracebd.py it's a bit less clear, but we probably
don't need to be spamming open calls 10 times a second.
That was confusing.
The -L/--label flag is already tricky enough to get right. Allowing
-L/--label to filter datasets is counter-intuitive and just makes it
harder to debug things.
Maybe it's just me, but it seems a bit more obvious cksum 2f629db4? is
an error vs cksum 2f629db4!. The second one just makes it seem like
dbglfs.py is really excited.
littlefs is intentionally designed to not rely on noise, even with cksum
collisions (hello, perturb bit!). So it makes sense for this to be an
optional feature, even if it's a small one.
Disabling revision count noise by default also helps with testing. The
whole point of revision count noise is to make cksum collisions less
likely, which is a bit counterproductive when that's something we want
to test!
This doesn't really change the revision count encoding:
vvvvrrrr rrrrrrnn nnnnnnnn nnnnnnnn
'-.''----.----''---------.--------'
'------|---------------|---------- 4-bit relocation revision
'---------------|---------- recycle-bits recycle counter
'---------- pseudorandom noise (optional)
I considered moving the recycle-bits down when we're not adding noise,
but the extra logic just isn't worth making the revision count a bit
more human-readable.
---
This saves a small bit of code in the default build, at the cost of some
code for the runtime checks in the LFS_NOISY build. Though I'm hoping
future config work will let users opt-out of these runtime checks:
code stack ctx
before: 38548 2624 640
default after: 38508 (-0.1%) 2624 (+0.0%) 640 (+0.0%)
LFS_NOISY after: 38568 (+0.1%) 2624 (+0.0%) 640 (+0.0%)
Honestly the thing I'm more worried about is using one of our precious
mount flags for this... There's not that many bits left!
altas, and to a lesser extend altns, are just too problematic for our
rbyd-append algorithm.
Main issue is these break our "narrowing" invariant, where each alt only
ever decreases the bounds.
I wanted to use altas to simplify lfsr_rbyd_appendcompaction, but
decided it wasn't worth it. Handling them correctly would require adding
a number of special cases to lfsr_rbyd_appendrat, adding complexity to
an already incredibly complex function.
---
Fortunately, we don't really need altns/altas on-disk, but we _do_ need
a way to mark alts as unreachable internally in order to know when we
can collapse alts when recoloring (at this point bounds information is
lost).
I was originally going to use the alt's sign bit for this, but it turns
out we already have this information thanks to setting jump=0 to assert
that an alt is unreachable. So no explicit flag needed!
This ends up saving a surprising amount of code for what is only a
couple lines of changes:
code stack ctx
before: 38512 2624 640
after: 38440 (-0.2%) 2624 (+0.0%) 640 (+0.0%)
We really shouldn't have two names for the same thing, it just makes
things more confusing, even if the public name doesn't quite match the
internal usage. Especially now that we internally rely on these being
the same flag.
This renames LFS_i_UNTIDY -> LFS_I_MKCONSISTENT and drops the untidy/
mktidy naming internally.
No code changes.
It doesn't really make sense to write to disk/trace files with multiple
threads, the result usually ends up clobbered and useless.
If we only pass disk/trace files to the first thread, the result is at
at least useable, even if it only represents 1/j tests.
This is actually quite a nice way to sample filesystem images in
multithreaded tests.
As a side effect, this also changes test.py/bench.py to no longer pass
-d/--disk or -t/--trace to runner queries, which is probably a good
thing? These should be ignored in queries anyways.
This was quite a puzzle.
The problem: How do we detect corrupt mdirs?
Seems like a simple question, but we can't just rely on mdir cksums. Our
mdirs are independently updateable logs, and logs have this annoying
tendency to "rollback" to previously valid states when corrupted.
Rollback issues aren't littlefs-specific, but what _is_ littlefs-
specific is that when one mdir rolls back, it can disagree with other
mdirs, resulting in wildly incorrect filesystem state.
To solve this, or at least protect against disagreeable mdirs, we need
to somehow include the state of all other mdirs in each mdir commit.
---
The first thought: Why not use gstate?
We already have a system for storing distributed state. If we add the
xor of all of our mdir cksums, we can rebuild it during mount and verify
that nothing changed:
.--------. .--------. .--------. .--------.
.| mdir 0 | .| mdir 1 | .| mdir 2 | .| mdir 3 |
|| | || | || | || |
|| gdelta | || gdelta | || gdelta | || gdelta |
|'-----|--' |'-----|--' |'-----|--' |'-----|--'
'------|-' '------|-' '------|-' '------|-'
'--.------' '--.------' '--.------' '--.------'
cksum | cksum | cksum | cksum |
| | v | v | v |
'---------> xor -------> xor -------> xor -------> gcksum
| v v v =?
'---------> xor -------> xor -------> xor ---> gcksum
Unfortunately it's not that easy. Consider what this looks like
mathematically (g is our gcksum, c_i is an mdir cksum, d_i is a
gcksumdelta, and +/-/sum is xor):
g = sum(c_i) = sum(d_i)
If we solve for a new gcksumdelta, d_i:
d_i = g' - g
d_i = g + c_i - g
d_i = c_i
The gcksum cancels itself out! We're left with an equation that depends
only on the current mdir, which doesn't help us at all.
Next thought: What if we permute the gcksum with a function t before
distributing it over our gcksumdeltas?
.--------. .--------. .--------. .--------.
.| mdir 0 | .| mdir 1 | .| mdir 2 | .| mdir 3 |
|| | || | || | || |
|| gdelta | || gdelta | || gdelta | || gdelta |
|'-----|--' |'-----|--' |'-----|--' |'-----|--'
'------|-' '------|-' '------|-' '------|-'
'--.------' '--.------' '--.------' '--.------'
cksum | cksum | cksum | cksum |
| | v | v | v |
'---------> xor -------> xor -------> xor -------> gcksum
| | | | .--t--'
| | | | '-> t(gcksum)
| v v v =?
'---------> xor -------> xor -------> xor ---> t(gcksum)
In math terms:
t(g) = t(sum(c_i)) = sum(d_i)
In order for this to work, t needs to be non-linear. If t is linear, the
same thing happens:
d_i = t(g') - t(g)
d_i = t(g + c_i) - t(g)
d_i = t(g) + t(c_i) - t(g)
d_i = t(c_i)
This was quite funny/frustrating (funnistrating?) during development,
because it means a lot of seemingly obvious functions don't work!
- t(g) = g - Doesn't work
- t(g) = crc32c(g) - Doesn't work because crc32cs are linear
- t(g) = g^2 in GF(2^n) - g^2 is linear in GF(2^n)!?
Fortunately, powers coprime with 2 finally give us a non-linear function
in GF(2^n), so t(g) = g^3 works:
d_i = g'^3 - g^3
d_i = (g + c_i)^3 - g^3
d_i = (g^2 + gc_i + gc_i + c_i^2)(g + c_i) - g^3
d_i = (g^2 + c_i^2)(g + c_i) - g^3
d_i = g^3 + gc_i^2 + g^2c_i + c_i^3 - g^3
d_i = gc_i^2 + g^2c_i + c_i^3
---
Bleh, now we need to implement finite-field operations? Well, not
entirely!
Note that our algorithm never uses division. This means we don't need a
full finite-field (+, -, *, /), but can get away with a finite-ring (+,
-, *). And conveniently for us, our crc32c polynomial defines a ring
epimorphic to a 31-bit finite-field.
All we need to do is define crc32c multiplication as polynomial
multiplication mod our crc32c polynomial:
crc32cmul(a, b) = pmod(pmul(a, b), P)
And since crc32c is more-or-less just pmod(x, P), this lets us take
advantage of any crc32c hardware/tables that may be available.
---
Bunch of notes:
- Our 2^n-bit crc-ring maps to a 2^n-1-bit finite-field because our crc
polynomial is defined as P(x) = Q(x)(x + 1), where Q(x) is a 2^n-1-bit
irreducible polynomial.
This is a common crc construction as it provides optimal odd-bit/2-bit
error detection, so it shouldn't be too difficult to adapt to other
crc sizes.
- t(g) = g^3 is not the only function that works, but it turns out to be
a pretty good one:
- 3 and 2^(2^n-1)-1 are coprime, which means our function t(g) = g^3
provides a one-to-one mapping in the underlying fields of all crc
rings of size 2^(2^n).
We know 3 and 2^(2^n-1)-1 are coprime because 2^(2^n-1)-1 =
2^(2^n)-1 (a Fermat number) - 2^(2^n-1) (a power-of-2), and 3
divides Fermat numbers >=3 (A023394) and is not 2.
- Our delta, when viewed as a polynomial in g: d(g) = gc^2 + g^2c +
c^3, has degree 2, which implies there are at most 2 solutions or
1-bit of information loss in the underlying field.
This is optimal since the original definition already had 2
solutions before we even chose a function:
d(g) = t(g + c) - t(g)
d(g) = t(g + c) - t((g + c) - c)
d(g) = t((g + c) + c) - t(g + c)
d(g) = d(g + c)
Though note the mapping of our crc-ring to the underlying field
already represents 1-bit of information loss.
- If you're using a cryptographic hash or other non-crc, you should
probably just use an equal sized finite-field.
Though note changing from a 2^n-1-bit field to a 2^n-bit field does
change the math a bit, with t(g) = g^7 being a better non-linear
function:
- 7 is the smallest odd-number coprime with 2^n-1, a Fermat number,
which makes t(g) = g^7 a one-to-one mapping.
3 humorously divides all 2^n-1 Fermat numbers.
- Expanding delta with t(g) = g^7 gives us a 6 degree polynomial,
which implies at most 6 solutions or ~3-bits of information loss.
This isn't actually the best you can do, some exhaustive searching
over small fields (<=2^16) suggests t(g) = g^(2^(n-1)-1) _might_ be
optimal, but that's a heck of a lot more multiplications.
- Because our crc32cs preserve parity/are epimorphic to parity bits,
addition (xor) and multiplication (crc32cmul) also preserve parity,
which can be used to show our entire gcksum system preserves parity.
This is quite neat, and means we are guaranteed to detect any odd
number of bit-errors across the entire filesystem.
- Another idea was to use two different addition operations: xor and
overflowing addition (or mod a prime).
This probably would have worked, but lacks the rigor of the above
solution.
- You might think an RS-like construction would help here, where g =
sum(c_ia^i), but this suffers from the same problem:
d_i = g' - g
d_i = g + c_ia^i - g
d_i = c_ia^i
Nothing here depends on anything outside of the current mdir.
- Another question is should we be using an RS-like construction anyways
to include location information in our gcksum?
Maybe in another system, but I don't think it's necessary in littlefs.
While our mdir are independently updateable, they aren't _entirely_
independent. The location of each mdir is stored in either the mtree
or a parent mdir, so it always gets mixed into the gcksum somewhere.
The only exception being the mrootanchor which is always at the fixed
blocks 0x{0,1}.
- This does _not_ catch "global-rollback" issues, where the most recent
commit in the entire filesystem is corrupted, revealing an older, but
still valid, filesystem state.
But as far as I am aware this is just a fundamental limitation of
powerloss-resilient filesystems, short of doing destructive
operations.
At the very least, exposing the gcksum would allow the user to store
it externally and prevent this issue.
---
Implementation details:
- Our gcksumdelta depends on the rbyd's cksum, so there's a catch-22 if
we include it in the rbyd itself.
We can avoid this by including it in the commit tags (actually the
separate canonical cksum makes this easier than it would have been
earlier), but this does mean LFSR_TAG_GCKSUMDELTA is not an
LFSR_TAG_GDELTA subtype. Unfortunate but not a dealbreaker.
- Reading/writing the gcksumdelta gets a bit annoying with it not being
in the rbyd. For now I've extended the low-level lfsr_rbyd_fetch_/
lfsr_rbyd_appendcksum_ to accept an optional gcksumdelta pointer,
which is a bit awkward, but I don't know of a better solution.
- Unlike the grm, _every_ mdir commit involves the gcksum, which means
we either need to propagate the gcksumdelta up the mroot chain
correctly, or somehow keep track of partially flushed gcksumdeltas.
To make this work I modified the low-level lfsr_mdir_commit__
functions to accept start_rid=-2 to indicate when gcksumdeltas should
be flushed.
It's a bit of a hack, but I think it might make sense to extend this
to all gdeltas eventually.
The gcksum cost both code and RAM, but I think it's well worth it for
removing an entire category of filesystem corruption:
code stack ctx
before: 37796 2608 620
after: 38428 (+1.7%) 2640 (+1.2%) 644 (+3.9%)
The gcksum isn't actually implemented yet, I mostly just wanted to
measure this code cost separately:
code stack ctx
before: 37768 2608 620
after: 37796 (+0.1%) 2608 (+0.0%) 620 (+0.0%)
I may be procrastinating a little bit...
Now that we perturb commit cksums with the odd-parity zero, the q-bit no
longer serves a purpose other than extra debug info. But this is a
double-edged sword, because redundant info just means another thing that
can go wrong.
For example, should we assert? If the q-bit doesn't reflect the
previous-perturb state it's a bug, but the only thing that would break
would be the q-bit itself. And if we don't assert what's the point of
keeping the q-bit around?
Dropping the q-bit avoids answering this question and saves a bit of
code:
code stack ctx
before: 37772 2608 620
after: 37768 (-0.0%) 2608 (+0.0%) 620 (+0.0%)
Most of littlefs's metadata is encoded in leb128s now, with the
exception of tags (be16, sort of), revision counts (le32), cksums
(le32), and flags.
It makes sense for tags to be a special case, these are written and
rewritten _everywhere_, but less so for flags, which are only written to
the mroot and updated infrequently.
We might as well save a bit of code by reusing our le32 machinery.
---
This changes lfsr_format to just write out compat flags as le32s, saving
a tiny bit of code at the cost of a tiny bit of disk usage (the real
benefit being a tiny bit of code simplification):
code stack ctx
before: 37792 2608 620
after: 37772 (-0.1%) 2608 (+0.0%) 620 (+0.0%)
Compat already need to handle trailing zeros gracefully, so this doesn't
change anything at mount time.
Also had to switch from enums to #defines thanks to C's broken enums.
Wooh. We already use #defines for the other flags for this reason.
LFS_WCOMPAT_RDONLY seems generally useful for tools that just want to
mark a filesystem is read-only. This is a common flag that exists in
other filesystems (RO_COMPAT_READONLY in ext4 for example).
LFS_RCOMPAT_WRONLY, on the other hand, is a bit more of a joke, but
there could be some niche use cases for it (preventing double mounts?).
Fortunately, these flags require no extra code, and fall out naturally
from our wcompat/rcompat handling.
---
Originally, the idea was to also add LFS_F_RDONLY, to match LFS_M_RDONLY
and set the LFS_WCOMPAT_RDONLY flag during format.
But this doesn't really work with the current API, since lfsr_format
would just give you an empty filesystem you can't write to. Which is a
bit silly.
Maybe we should add something like lfsr_fs_mkrdonly in the future? This
is probably low-priority.
Mainly to add LFS_RCOMPAT_MSPROUT. It makes sense that a littlefs driver
may not want to support mroot-inlined mdirs, and this flag would be the
only way to indicate that. (Currently inlined mdir -> mtree is one way,
but this may not always be the case.)
This also makes space for a couple planned features:
LFS_RCOMPAT_NONSTANDARD 0x00000001 Non-standard filesystem format
LFS_RCOMPAT_WRONLY* 0x00000002 Reading is disallowed
LFS_RCOMPAT_GRM 0x00000004 May use a global-remove
LFS_RCOMPAT_MSPROUT 0x00000010 May use an inlined mdir
LFS_RCOMPAT_MLEAF 0x00000020 May use a single mdir pointer
LFS_RCOMPAT_MSHRUB 0x00000040 May use an inlined mtree
LFS_RCOMPAT_MTREE 0x00000080 May use an mdir btree
LFS_RCOMPAT_BSPROUT 0x00000100 Files may use inlined data
LFS_RCOMPAT_BLEAF 0x00000200 Files may use single block pointers
LFS_RCOMPAT_BSHRUB 0x00000400 Files may use inlined btrees
LFS_RCOMPAT_BTREE 0x00000800 Files may use btrees
*Planned
I've gone ahead and included rcompat flags we reserve but don't
currently use (LFS_RCOMPAT_MSHRUB). It seems like a good idea to make
these reservations explicit. Though we should still prohibit their use
until there is a good reason, in case we want to repurpose these flags
in the future.
Code changes minimal (larger literal? compiler noise?):
code stack ctx
before: 37788 2608 620
after: 37792 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
This is mainly to free up space for flags, we're pretty close to running
out of 32-bits with future planned features:
1. Reduced file type info from 8 -> 4 bits
We don't really need more than this, but it does mean type info is
no longer a simple byte load.
2. Moved most internal file-state flags into the next 4 bits
These are mostly file-type specific (except LFS_o_ZOMBIE), so we
don't need to worry too much about overlap.
3. Compacted ck-flags into 5 bits:
LFS_M_CKPROGS 0x00000800
LFS_M_CKFETCHES 0x00001000
LFS_M_CKPARITY 0x00002000
LFS_M_CKMETAREDUND* 0x00004000
LFS_M_CKDATACKSUMS 0x00008000
*Planned
Now that ck-flags are a bit more mature, it's pretty clear we'll
probably never have CKMETACKSUMS (ckcksums + small tag reads is
crazy expensive) or CKDATAREDUND (non-trivial parity fanout makes
this crazy expensives. So reserving bits for these just wastes bits.
This also moves things around so ck-flags no longer overlap with open
flags.
It's a tight fit, and I still think file-specific ck-flags are out-of-
scope, but this at least decreases flag ambiguity.
New jenga:
8 8 8 8
.----++----++----++----.
.-..-..-.-------.------.
o_flags: |t||f||t| | o |
|-||-||-|-------:--.---'
|-||-||-'--.----.------.
t_flags: |t||f|| t | | tstt |
'-''-'|----|----'------'
.----.|----|.--.:--:.--.
m_flags: | f || t ||c ||o ||m |
|----||-.--'|--|'--''--'
|----||-|---|--|.------.
f_flags: | f ||t| |c || f |
'----''-'---'--''------'
Fortunately no major code costs:
code stack ctx
before: 37792 2608 620
after: 37788 (-0.0%) 2608 (+0.0%) 620 (+0.0%)
dbgerr.py and dbgtag.py have proven to be incredibly useful for quick
debugging/introspection, so I figured why not have more of that.
My favorite part is being able to quickly see all flags set on an open
file handle:
(gdb) p file.o.o.flags
$2 = 24117517
(gdb) !./scripts/dbgflags.py o 24117517
LFS_O_WRONLY 0x00000001 Open a file as write only
LFS_O_CREAT 0x00000004 Create a file if it does not exist
LFS_O_EXCL 0x00000008 Fail if a file already exists
LFS_O_DESYNC 0x00000100 Do not sync or recieve file updates
LFS_o_REG 0x01000000 Type = regular-file
LFS_o_UNFLUSH 0x00100000 File's data does not match disk
LFS_o_UNSYNC 0x00200000 File's metadata does not match disk
LFS_o_UNCREAT 0x00400000 File does not exist yet
The only concern is if dbgflags.py falls out-of-sync often, I suspect
flag encoding will have quite a bit more churn than flags/tags. But we
can always drop this script in the future if this turns into a problem.
---
While poking around this also ended up with a bunch of other small
changes:
- Added LFS_*_MODE masks for consistency with other "type<->flag
embeddings"
- Added compat flag comments
- Adopted lowercase prefix for internal flags (LFS_o_ZOMBIE), though
not sure if I'll keep this yet...
- Tweaked dbgerr.py to also match ERR_ prefixes and to ignore case
Found a bug in our toml parser that's difficult to work around:
defines.GC_FLAGS = """ => {
LFS_GC_MKCONSISTENT "GC_FLAGS": "blablabla",
| LFS_GC_LOOKAHEAD } // where did defines go?
"""
This appears to be this bug:
https://github.com/uiri/toml/issues/286
But since it was opened 4 years ago, I think it's safe to say this toml
library is now defunct...
---
Apparently tomllib/tomli is the new hotness, which started as tomli
before being adopt in Python 3.11 as tomllib. Fortunately tomli is still
maintained so we don't have to worry about Python versions too much.
Adopting tomli was relatively straightforward, the only hiccup being
that it doesn't support text files? Curious, but fortunately Python
exposes the underlying binary file handle in f.buffer.
Unfortunately the import sys in the argparse block was hiding missing
sys imports.
The mistake was assuming the import sys in Python would limit the scope
to that if block, but Python's late binding strikes again...
Apparently __builtins__ is a CPython implementation detail, and behaves
differently when executed vs imported???
import builtins is the correct way to go about this.
I've been unhappy with LFSR_TAG_ORPHAN for a while now. While it's true
these represent orphaned files, they also represent zombied files. And
as long as a reference to the file exists in-RAM, I find it hard to say
these files are truely "orphaned".
We're also just using the term "orphan" for too many things.
Really this tag just represents an mid reservation. The term stickynote
works well enough for this, and fits in with the other internal tag,
LFSR_TAG_BOOKMARK.
Moved local import hack behind if __name__ == "__main__"
These scripts aren't really intended to be used as python libraries.
Still, it's useful to import them for debugging and to get access to
their juicy internals.
Instead of trying to be too clever, this just adds a bunch of small
flags to control parts of table rendering:
- --no-header - Don't show the header.
- --small-header - Don't show by field names.
- --no-total - Don't show the total.
- -Q/--small-table - Equivalent to --small-header + --no-total.
Note that -Q/--small-table replaces the previous -Y/--summary +
-c/--compare hack, while also allowing a similar table style for
non-compare results.
This ended up being a pretty in-depth rework of prettyasserts.py to
adopt the shared Parser class. But now prettyasserts.py should be both
more robust and faster.
The tricky parts:
- The Parser class eagerly munches whitespace by default. This is
usually a good thing, but for prettyasserts.py we need to keep track
of the whitespace somehow in order to write it to the output file.
The solution here is a little bit hacky. Instead of complicating the
Parser class, we implicitly add a regex group for whitespace when
compiling our lexer.
Unfortunately this does make last-minute patching of the lexer a bit
messy (for things like -p/--prefix, etc), thanks to Python's
re.Pattern class not being extendable. To work around this, the Lexer
class keeps track of the original patterns to allow recompilation.
- Since we no longer tokenize in a separate pass, we can't use the
None token to match any unmatched tokens.
Fortunately this can be worked around with sufficiently ugly regex.
See the 'STUFF' rule.
It's a good thing Python has negative lookaheads.
On the flip side, this means we no longer need to explicitly specify
all possible tokens when multiple tokens overlap.
- Unlike stack.py/csv.py, prettyasserts.py needs multi-token lookahead.
Fortunately this has a pretty straightforward solution with the
addition of an optional stack to the Parser class.
We can even have a bit of fun with Python's with statements (though I
do wish with statements could have else clauses, so we wouldn't need
double nesting to catch parser exceptions).
---
In addition to adopting the new Parser class, I also made sure to
eliminate intermediate string allocation through heavy use of Python's
io.StringIO class.
This, plus Parser's cheap shallow chomp/slice operations, gives
prettyasserts.py a much needed speed boost.
(Honestly, the original prettyasserts.py was pretty naive, with the
assumption that it wouldn't be the bottleneck during compilation. This
turned out to be wrong.)
These changes cut total compile time in ~half:
real user sys
before (time make test-runner -j): 0m56.202s 2m31.853s 0m2.827s
after (time make test-runner -j): 0m26.836s 1m51.213s 0m2.338s
Keep in mind this includes both prettyasserts.py and gcc -Os (and other
Makefile stuff).
This was flipped in b5e264b.
Infering the type from the right-hand side is tempting here, but the
right-hand side if often a constant, which gets a bit funky in C.
Consider:
assert(lfs->cfg->read != NULL);
gcc: warning: ISO C forbids initialization between function pointer
and ‘void *’ [-Wpedantic]
assert(err < 0ULL);
gcc: warning: comparison of unsigned expression in ‘< 0’ is always
false [-Wtype-limits]
Prefering the left-hand type should hopefully avoid these issues most of
the time.
This reverts per-result source file mapping, and tears out of a bunch of
messy dwarf parsing code. Results from the same .o file are now mapped
to the same source file.
This was just way too much complexity for slightly better result->file
mapping, which risked losing results accidentally mapped to the wrong
file.
---
I was originally going to revert all the way back to relying strictly on
the .o name and --build-dir (490e1c4) (this is the simplest solution),
but after poking around in dwarf-info a bit, I realized we do have
access to the original source file in DW_TAG_compile_unit's
DW_AT_comp_dir + DW_AT_name.
This is much simpler/more robust than parsing objdump --dwarf=rawline,
and avoid needing --build-dir in a bunch of scripts.
---
This also reverts stack.py to rely only on the .ci files. These seem as
reliable as DW_TAG_compile_unit while simplifying things significantly.
Symbol mapping used to be a problem, but this was fixed by using the
symbol in the title field instead of the label field (which strips some
optimization suffixes?)
It's a bit funny, the motivation for a new Parser class came from the
success of simple regex + space munching in csv.py, but adopting Parser
in csv.py makes sense for a couple reasons:
- Consistency and better code sharing with other scripts that need to
parse things (stack.py, prettyasserts.py?).
- Should be more efficient, since we avoid copying the entire string
every time we chomp/slice.
Though I don't think this really matters for the size of csv.py's
exprs...
- No need to write every regex twice! Since Parser remembers the last
match.
If we're not using these results, no reason to collect all of the
children.
Note that we still need to recurse for other measurements (limit, struct
size, etc).
This has a measurable, but small, impact on runtime:
stack.py -z0 -Y: 0.202s
stack.py -z1 -Y: 0.162s (~-19.8%)
ctx.py -z0 -Y: 0.112s
ctx.py -z1 -Y: 0.098s (~-12.5%)
Now that cycle detection is always done at result collection time, we
don't need this in the table renderer itself.
This had a tendency to cause problems for non-function scripts (ctx.py,
structs.py).
God, I wish Python had an OrderedSet.
This is a fix for duplicate "cycle detected" notes when using -t/--hot.
This mix of merging both _hot_notes and _notes in the HotResult class is
tricky when the underlying container is a list.
The order is unlikely to be guaranteed anyways, when different results
with different notes are folded.
And if we ever want more control over the order of notes in result
scripts we can always change this back later.
- Error on no/insufficient files.
Instead of just returning no results. This is more useful when
debugging complicated bash scripts.
- Use elf magic to allow any file order in perfbd.py/stack.py.
This was already implemented in stack.py, now also adopted in
perfbd.py.
Elf files always start with the magic string "\x7fELF", so we can use
this to figure out the types of input files without needing to rely on
argument order.
This is just one less thing to worry about when invoking these
scripts.
It's been a while since I've been hurt by Python's late-binding
variables. In this case the scope-creep of the "file" variable hid that
we didn't actually know which recursive result belonged to which file.
Instead we were just assigning whatever the most recent top-level result
was.
This is fixed by looking up the correct file in childrenof. Though this
unfortunately does add quite a bit of noise.
Christopher Haster