After tinkering around with the scripts for a bit, I've started to
realize difflib is kinda... really slow...
I don't think this is strictly difflib's fault. It's a pure python
library (proof of concept?), may be prioritizing quality over speed, and
I may be throwing too much data at it.
difflib does have quick_ratio() and real_quick_ratio() for faster
comparisons, but while looking into these for correctness, I realized
there's a simpler heuristic we can use since GCC's optimized names seem
strictly additive: Choose the name that matches with the smallest prefix
and suffix.
So comparing, say, lfsr_rbyd_lookup to __lfsr_rbyd_lookup.constprop.0:
lfsr_rbyd_lookup
__lfsr_rbyd_lookup.constprop.0
|'------.-------''----.-----'
'-------|-----. .---'
v v v
key: (matches, 2, 12)
Note we prioritize the prefix, since it seems GCC's optimized names are
strictly suffixes. We also now fail to match if the dwarf name is not
substring, instead of just finding the most similar looking symbol.
This results in both faster and more robust symbol->dwarf mapping:
before: time code.py -Y: 0.393s
after: time code.py -Y: 0.152s
(this is WITH the fast dict lookup on exact matches!)
This also drops difflib from the scripts. So one less dependency to
worry about.
There is an argument for prefering nm for code size measurements due to
portability. But I'm not sure this really holds up these days with
objdump being so prevalent.
We already depend on objdump for ctx/structs/perf and other dwarf info,
so using objdump -t to get symbol information means one less tool to
depend on/pass around when cross-compiling.
As a minor benefit this also gives us more control over which sections
to include, instead of relying on nm's predefined t/r/d/b section types.
---
Note code.py/data.py did _not_ require objdump before this. They did use
objdump to map symbols to source files, but would just guess if
objdump wasn't available.
Without this, naming a column i/children/notes in csv.py could cause
things to break. Unlikely for children/notes, but very likely for i,
especially when benchmarking.
Unfortunately namedtuple makes this tricky. I _want_ to just rename
these to _i/_children/_notes and call the problem solved, but namedtuple
reserves all underscore-prefixed fields for its own use.
As a workaround, the table renderer now looks for _i/_children/_notes at
the _class_ level, as an optional name of which namedtuple field to use.
This way Result types can stay lightweight namedtuples while including
extra table rendering info without risk of conflicts.
This also makes the HotResult type a bit more funky, but that's not a
big deal.
This extends the recursive part of the table renderer to sort children
by the optional "i" field, if available.
Note this only affects children entries. The top-level entries are
strictly ordered by the relevant "by" fields. I just haven't seen a use
case for this yet, and not sorting "i" at the top-level reduces that
number of things that can go wrong for scripts without children.
---
This also rewrites -t/--hot to take advantage of children ordering by
injecting a totally-no-hacky HotResult subclass.
Now -t/--hot should be strictly ordered by the call depth! Though note
entries that share "by" fields are still merged...
This also gives us a way to introduce the "cycle detected" note and
respect -z/--depth, so overall a big improvement for -t/--hot.
We don't really need padding for the notes on the last column of tables,
which is where row-level notes end up.
This may seem minor, but not padding here avoids quite a bit of
unnecessary line wrapping in small terminals.
- Adopted higher-level collect data structures:
- high-level DwarfEntry/DwarfInfo class
- high-level SymInfo class
- high-level LineInfo class
Note these had to be moved out of function scope due to pickling
issues in perf.py/perfbd.py. These were only function-local to
minimize scope leak so this fortunately was an easy change.
- Adopted better list-default patterns in Result types:
def __new__(..., children=None):
return Result(..., children if children is not None else [])
A classic python footgun.
- Adopted notes rendering, though this is only used by ctx.py at the
moment.
- Reverted to sorting children entries, for now.
Unfortunately there's no easy way to sort the result entries in
perf.py/perfbd.py before folding. Folding is going to make a mess
of more complicated children anyways, so another solution is
needed...
And some other shared miscellany.
$ ./scripts/csv.py lfs.code.csv -bfunction -fsize -S
... blablabla ...
TypeError: cannot unpack non-iterable NoneType object
The issue was argparse's const defaults bypassing the type callback, so
the sort field ends up with None when it expects a tuple (well
technically a tuple tuple).
This is only an issue for csv.py because csv.py's sort fields can
contain exprs.
- Dropped --internal flag, structs.py includes all structs now.
No reason to limit structs.py to public structs if ctx.py exists.
- Added struct/union/enum prefixes to results (enums were missing in
ctx.py).
- Only sort children layers if explicitly requested. This should
preserve field order, which is nice.
- Adopt more advanced FileInfo/DwarfInfo classes.
- Adopted table renderer changes (notes rendering).
- Sorting struct fields by name? Eh, that's not a big deal.
- Sorting function params by name? Okay, that's really annoying.
This compromises by sorting only the top-level results by name, and
leaving recursive results in the order returned by collect by default.
Recursive results should usually have a well-defined order.
This should be extendable to the other result scripts as well.
This is a bit more readable and better matches the names used in the C
code (lfs_config vs struct lfs_config).
The downside is we now have fields with spaces in them, which may cause
problems for naive parsers.
ctx.py reports functions' "contexts", i.e. the sum of the size of all
function parameters and indirect structs, recursively dereferencing
pointers when possible.
The idea is this should give us a rough lower bound on the amount of
state that needs to be allocated to call the function:
$ ./scripts/ctx.py lfs.o lfs_util.o -Dfunction=lfsr_file_write -z3 -s
function size
lfsr_file_write 596
|-> lfs 436
| '-> lfs_t 432
|-> file 152
| '-> lfsr_file_t 148
|-> buffer 4
'-> size 4
TOTAL 596
---
The long story short is that structs.py, while very useful for
introspection, has not been useful as a general metric.
Sure it can give you a rough idea of the impact of small changes to
struct sizes, but it's not uncommon for larger changes to add/remove
structs that have no real impact on the user facing RAM usage. There are
some structs we care about (lfs_t) and some we don't (lfsr_data_t).
Internal-only structs should already be measured by stack.py.
Which raises the question, how do we know which structs we care about?
The idea here is to look at function parameters and chase pointers. This
gives a complicated, but I think reasonable, heuristic. Fortunately
dwarf-info gives us all the necessary info.
Some notes:
- This does _not_ include buffer sizes. Buffer sizes are user
configurable, so it's sort of up to the user to account for these.
- We include structs once if we find a cycle (lfsr_file_t.o for
example). Can't really do any better and this at least provides a
lower bound for complex data-structures.
- We sum all params/fields, but find the max of all functions. Note this
prevents common types (lfs_t for example) from being counted more than
once.
- We only include global functions (based on the symbol flag). In theory
the context of all internal functions should end up in stack.py.
This can be overridden with --everything.
Note this doesn't replace structs.py. structs.py is still useful for
looking at all structs in the system. ctx.py should just be more useful
for comparing builds at a high level.
It looks like the failure case in our scripts' subprocess stderr
handling was not tested well during a fix to stderr blocking (a735bcd).
This code was attempting to print stderr only if an error occured, but
with stderr=None this just results in a NoneType TypeError.
In retrospect, completely hiding stderr is kind of shitty if a
subprocess fails, but it doesn't seem possible to read from both stdin
and stderr with Python's APIs without getting stuck when the stderr's
buffer is full.
It might be possible to work around this with either multithreading,
select calls, or a temp file, but I'm not sure slightly less verbose
scripts are worth the added complexity in every single subprocess call.
For now just reverting to unconditionally forwarding stderr from the
child process. This is the simplest/most robust option.
- stack.py:collect -> collect + collect_cov
- perf.py:collect_syms_and_lines -> collect_syms + collect_dwarf_lines
- perfbd.py:collect_syms_and_lines -> collect_syms + collect_dwarf_lines
This should hopefully lead to both better readability and better code
reuse.
Note collect_dwarf_lines is a bit different than collect_dwarf_files in
code.py/data.py/etc, but the extra complexity of collect_dwarf_lines is
probably not worth sharing here.
This breaks the collect function down into collect_dwarf_files,
collect_dwarf_info, and collect_sizes. This makes the dwarf-info parser
a bit easier to share with structs.py, etc.
Sharing easily copy-pastable chunks of code in scripts like this has
allowed for better code reuse without intricately tying script
dependencies together. Being able to run each of these scripts
standalone is a goal.
The fact that our scripts' table renderer was slightly different for
recursive scripts (stack.py, perf.py) and non-recursive scripts
(code.py, structs.py) was a ticking time bomb, one innocent edit away
from breaking half the scripts.
The makes the table renderer consistent across all scripts, allowing for
easy copy-pasting when editing at the cost of some unused code in
scripts.
One hiccup with this though is the difference in cycle detection
behavior between scripts:
- stack.py:
lfsr_bd_sync
'-> lfsr_bd_prog
'-> lfsr_bd_sync <-- cycle!
- structs.py:
lfsr_bshrub_t
'-> u
'-> bsprout
'-> u <-- not a cycle!
To solve this the table renderer now accepts a simple detect_cycles
flag, which can be set per-script.
Dwarf-info doesn't actually provide alignment info with the current
tools I'm using (but it does look like DW_AT_alignment was added in a
recent version), so for now this is just a heuristic based on the
largest base/pointer type.
This heuristic is still useful info and probably correct for the types
littlefs cares about (no SIMD here!).
This is also another field that folds using max, so that's fun.
This reworks structs.py's internal dwarf-info parser to be a bit more
flexible. The eventual plan is to adopt this parser in other scripts.
The main difference is we now parse the dwarf-info into a full tree,
with optional filtering, before extracting the fields we care about.
This is both more flexible and gives us more confidence the parser is
not misparsing something.
(Unrelated but apparently misparsing is a real word.)
This also extends structs.py to include field info for structs and
unions. This is quite useful for understanding the size of things:
$ ./scripts/structs.py thumb/lfs.o -Dstruct=lfsr_bptr_t -z
struct size
lfsr_bptr_t 20
|-> cksize 4
|-> cksum 4
'-> data 12
|-> size 4
'-> u 8
|-> buffer 4
'-> disk 8
|-> block 4
'-> off 4
TOTAL 20
The field info uses the same -z/--depth flag from stack.py/perf.py/
perbd.py, however the cycle detector needed a bit of tweaking. Detecting
cycles purely by name doesn't quite work with structs:
file->o.o.flags
^ |
'-' not a cycle!
Unfortunately, we do lose the field order in structs. But this info is
still useful.
Oh, we also prefer typedef names over struct/union names now. These are
a bit easier to read since they are more common in the codebase.
This will probably only have niche uses, but may be useful for small
test sets or for running specific tests with -O-.
Though it is a bit funny that -q -O- turns test.py/bench.py into more or
less just a complicated way to run a C program.
A couple problems:
1. We should probably also support negative ranges, but this is a bit
annoying since we can't tell if the range is negative or positive
until expr evaluation.
2. Evaluating the range exprs at compile-time is inconsistent from other
C exprs in our tests/benches (normal defines, if filters, etc), and
severely limiting since we can't use other defines before the define
system is initialized.
2. Attempting to move these range exprs into their own lazily evaluated
functions does not seem tractable...
We'd need to evaluate defines to know how many permutations there
are, but how can we evaluate defines before knowing which permutation
we're on?
I think this circular dependency would make the permutation count
undecidable?
Even if we could move these exprs to their own lazily evaluated
functions (which would solve the inconsistency issue), the complexity
risks outweighing the benefit. Keep in mind it's useful if external
tools can parse our tests. So reverting for now.
Though I am keeping some of the refactoring in test.py/bench.py. Having
a special DRange type is useful if we ever want to add more define
functions in the future.
This enables full C exprs in test/bench define ranges by simply passing
them on to the C compiler.
So this:
defines.N = 'range(1,20+1)'
Becomes this, in N's define function:
if (i < 0 + ((((20+1)-1-(1))/(1) + 1))) return ((i-(0))*(1) + (1));
Which is a bit of a mess, but generates the correct range at runtime.
This allows for much more flexible exprs in range defines without
needing a full expr parser in Python.
Note though that we need to evaluate the range length at compile time.
This is notably before the test/bench define system is initialized, so
all three range args (start, stop, step) are limited to really only
simple C literals and exprs.
This makes the -p/--percent flag a bit more consistent with -d/--diff
and -c/--compare, both of which change the printing strategy based on
additional context.
This showcases the sort of high-level result printing where -c/--compare
is useful:
$ make summary-diff
code data stack structs
BEFORE 57057 0 3056 1476
AFTER 68864 (+20.7%) 0 (+0.0%) 3744 (+22.5%) 1520 (+3.0%)
There was one hiccup though: how to hide the name of the first field.
It may seem minor, but the missing field name really does help
readability when you're staring at a wall of CLI output.
It's a bit of a hack, but this can now be controlled with -Y/--summary,
which has the sole purpose of disabling the first field name if mixed
with -c/--compare.
-c/--compare is already a weird case for the summary row anyways...
Example:
$ ./scripts/csv.py lfs.code.csv \
-bfunction -fsize \
-clfsr_rbyd_appendrattr
function size
lfsr_rbyd_appendrattr 3598
lfsr_mdir_commit 5176 (+43.9%)
lfsr_btree_commit__.constprop.0 3955 (+9.9%)
lfsr_file_flush_ 2729 (-24.2%)
lfsr_file_carve 2503 (-30.4%)
lfsr_mountinited 2357 (-34.5%)
... snip ...
I don't think this is immediately useful for our code/stack/etc
measurement scripts, but it's certainly useful in csv.py for comparing
results at a high level.
And by useful I mean it replaces a 40-line long awk script that has
outgrown its original purpose...
This may be a (very javascript-esque) mistake, but implicit conversion
to strings is useful when mixing fields and strings in -b/--by field
exprs:
$ ./scripts/csv.py input.csv -bcase='"test"+n' -fn
Note that this now (mostly) matches the behavior when the n field is
unspecified:
$ ./scripts/csv.py input.csv -bcase='"test"+n'
Er... well... mostly. When we specify n as a field, csv.py does
typecheck and parse the field, which ends up sort of canonicalizing the
field, unlike omitting n which leaves n as a string... But at least if
the field was already canonicalized the behavior matches...
It may also be better to force all -b/--by expr inputs to strings first,
but this would require us to know which expr came from where. It also
wouldn't solve the canonicalization problem.
So in:
$ ./scripts/csv.py input.csv -fa='b?c:d'
c and d must have matching types or else an error is raised.
This requires an explicit definition for the ternary operator since it's
a special case in that the type of b does not matter.
Compare to a 3-arg max call:
$ ./scripts/csv.py input.csv -fa='int(b)?float(c):float(d)' # ok
$ ./scripts/csv.py input.csv -fa='max(int(b),float(c),float(d))' # error
The main benefit of this is allowing the sort order to be controlled by
fields that don't necessarily need to be printed:
./scripts/csv.py input.csv -ba -sb -fc
By default this sorts lexicographically, but this can be changed by
providing an expression:
./scripts/csv.py input.csv -ba -sb='int(b)' -fc
Note that sort fields do _not_ change inferred by fields, this allows
sort flags to be added to existing queries without changing the results
too much:
./scripts/csv.py input.csv -fc
./scripts/csv.py input.csv -sb -fc
This was the one piece needed to be able to replace amor.py with csv.py.
The missing feature in csv.py is the ability to keep track of a
running-sum, but this is a bit of a hack in amor.py considering we
otherwise view csv entries as unordered.
We could add a running-sum to csv.py, or instead, just include a running
sum as a part of our bench output. We have all the information there
anyways, and if it simplifies the mess that is our csv scripts, that's a
win.
---
This also replaces the bench "meas", "iter", and "size" fields with the
slightly simpler "m" (measurement? metric?) and "n" fields. It's up to
the specific benchmark exactly how to interpret "n", but one field is
sufficient for existing scripts.
The issue here is quite nuanced, but becomes a problem when you want to
both:
1. Filter results by a given field: -Dmeas=write
2. Output a new value for that field: -bmeas='"write+amor"'
If you didn't guess from the example, this comes up often in scripts
dealing with bench results, where we often find ourselves wanting to
append/merge modified results based on the raw measurements.
Fortunately the fix is relatively easy: We already filter by defines
in our collect function, so we don't really need to filter by defines
again when folding.
Folding occurs after expr evaluation, but collect occurs before, so this
limits filtering to the input fields _before_ expr evaluation.
This does mean we no longer filter on the output of exprs, but I don't
know if such behavior was ever intentionally desired. Worst case it can
be emulated by stacking multiple csv.py calls, which may be annoying,
but is at least well-intentioned and well-defined.
---
Note that the other result scripts, code.py, stack.py, etc, are a bit
different in that they rely on fold-time filtering for filtering
generated results. This may deserve a refactor at some point, but since
these scripts don't also evaluate exprs, it's not an immediate problem.
This may make some mathematician mad, but these are informative scripts.
Returning +-inf is much more useful than erroring when dealing with
several hundred rows of results.
And hey, if it's good enough for IEEE 754, it's good enough for us :)
Also fixed a division operator mismatch in RFrac that was causing
problems.
Not sure if this is an old habit from Python 2, or just because it looks
nicer next to __mul__, __mod__, etc, but in Python 3 this should be
__truediv__ (or __floordiv__), not __div__.
This is now inconsistent with csv.py, and I don't really want to add a
full expr parser to every script that might want to rename fields.
Field renaming (or any expr really!) can be accomplished with
intermediate calls to csv.py anyways. No reason to make these scripts
more complicated than they need to be.
The only reason RFloats reused RInt's operator definitions was to save a
few keystrokes. But this dependency is unnecessary and will get in the
way if we ever add a script that only uses RFloats.
So now the available field exprs can be queried with --help-exprs:
$ ./scripts/csv.py --help-exprs
uops:
+a Non-negation
-a Negation
!a 1 if a is zero, otherwise 0
bops:
a * b Multiplication
a / b Division
... snip ...
I was a bit torn on if this should be named --help-exprs or --list-exprs
to match test.py/bench.py, but decided on --help-exprs since it's
querying something "inside" the script, whereas test.py/bench.py's
--list-cases is querying something "outside" the script.
Internally this uses Python's docstrings, which is a nice language
feature to lean on.
Mainly for consistency with int operators, though it's unclear if either
mod is useful in the context of csv.py and related scripts.
This may be worth reverting at some point.
Now, by default, an error is raised if any branch of an expr has an
inconsistent type.
This isn't always what we want. The ternary operator, for example,
doesn't really care if the condition's type doesn't match the branch
arms. But it's a good default, and special cases can always override the
type function with their own explicit typechecking.
There's a bit of a push and pull when it comes to typechecking CSV
fields in our scripts. On one hand, we want the flexibility to accepts
scripts with various mismatched fields, on the other hand, we _really_
want to know if a typo caused a field to be quietly replaced with all
zeros...
I _think_ it's safe to say: if no fields across _all_ input files match
a requested field, we should error.
But I may end up wrong about this. Worst case we can always revert in
the future, maybe with an explicit flag to ignore missing fields.
- Updated the example in the header comment.
The previous example was way old, from back when fields were separated
by commas! Introduced in 20ec0be87 in 2022 according to git blame.
- Renamed a couple internal RExpr classes:
- Not -> NotNot
- And -> AndAnd
- Or -> OrOr
- Ife -> IfElse
This is mainly to leave room for bitwise operators in case we every
want to add them.
- Added isinf, isnan, isint, etc:
- isint(a)
- isfloat(a)
- isfrac(a)
- isinf(a)
- isnan(a)
In theory useful for conditional exprs based on the field's type.
- Accept +-nan as a float literal.
Niche, but seems necessary for completeness. Unfortunately this does
mean a field named nan (or inf) may cause problems...
I still think the 24 (23+1) char minimum is a good default for 2 column
output such as help text, especially if you don't have automatic width
detection. But our result scripts need to be a bit more flexible.
Consider:
$ make summary
code data stack structs
TOTAL 68864 0 3744 1520
Vs:
$ make summary
code data stack structs
TOTAL 68864 0 3744 1520
Up until now we were just kind of working around this with cut -c 25- in
our Makefile, but now that our result scripts automatically scale the
table widths, they should really just default to whatever is the most
useful.
- RInt/RFloat now accepts implicitly castable types (mainly
RInt(RFloat(x)) and RFloat(RInt(x))).
- RInt/RFloat/RFrac are now "truthy", implements __bool__.
- More operator support for RInt/RFloat/RFrac:
- __pos__ => +a
- __neg__ => -a
- __abs__ => abs(a)
- __div__ => a/b
- __mod__ => a%b
These work in Python, but are mainly used to implement expr eval in
csv.py.
- Allow single-arg frac:
- frac(a) => a/a
- frac(a, b) => a/b
This was already supported internally.
- Implicitly cast to frac in frac ops:
- ratio(3) => ratio(3/3) => 1.0 (100%)
- total(3) => total(3/3) => 3
This makes a bit more sense than erroring.
This now returns 1.0 if the total part of the fraction is 0.
There may be a better way to handle this, but the intention is for 0/0
to map to 100% for thing like code coverage (cov.py), test coverage
(test.py), etc.
So csv.py should now be mostly feature complete, aside from bugs.
I ended up dropping most of the bitwise operations for now. I can't
really see them being useful since csv.py and related scripts are
usually operating on purely numerical data. Worst case we can always add
them back in at some point.
I also considered dropping the logical/ternary operators, but even
though I don't see an immediate use case, the flexibility
logical/ternary operators add to a language is too much to pass on.
Another interesting thing to note is the extension of all fold functions
to operate on exprs if more than one argument is provided:
- max(1) => 1, fold=max
- max(1, 2) => 2, fold=sum
- max(1, 2, 3) => 3, fold=sum
To be honest, this is mainly just to allow a binary max/min function
without awkward naming conflicts.
Other than those changes this was pretty simple fill-out-the-definition
work.
This was more tricky than expected since Python's class scope is so
funky (I just eneded up with using lazy cached __get__ functions that
scan the RExpr class for tagged members), but these decorators help avoid
repeated boilerplate for common expr patterns.
We can even deduplicate binary expr parsing without sacrificing
precedence.
This is a work-in-progress, but the general idea is to replace the
existing rename mechanic in csv.py with a full expr parser:
$ ./scripts/csv.py input.csv -ba=x -fb=y+z
I've been putting this off for a while, as it feels like too big a jump
in complexity for what was intended to be a simple script. But
complexity is a bit funny in programming. Even if a full parser is more
difficult to implement, if it's the right grammar for the job, the
resulting script should end up both easier to understand and easier to
extend.
The original intention was that any sufficiently complicated math could
be implemented in ad-hoc Python scripts that operate directly on the CSV
files, but CSV parsing in Python is annoying enough that this never
really worked well.
But I'm probably overselling the complexity. This is classic CS stuff:
1. build a syntax tree
2. map symbols to input fields
3. typecheck, fold, eval, etc
One neat thing is that in addition to providing type and eval
information, our exprs can also provide information on how to "fold" the
field after eval. This kicks in when merging muliple rows when grouping
by -b/--by, and for finding the TOTAL results.
This can be used to merge stack results correctly with max:
$ ./scripts/csv.py stack.csv \
-fframe='sum(frame)' -flimit='max(limit)'
Or can be used to find other interesting measurements:
$ ./scripts/csv.py stack.csv \
-favg='avg(frame)' -fstddev='stddev(frame)'
These changes also make the eval order of input/output fields much
stricter which is probably a good thing.
This should replace all of the somewhat hacky fake-expr flags in csv.py:
- --int => -fa='int(b)'
- --float => -fa='float(b)'
- --frac => -fa='frac(b)'
- --sum => -fa='sum(b)'
- --prod => -fa='prod(b)'
- --min => -fa='min(b)'
- --max => -fa='max(b)'
- --avg => -fa='avg(b)'
- --stddev => -fa='stddev(b)'
- --gmean => -fa='gmean(b)'
- --gstddev => -fa='gstddev(b)'
If you squint you might be able to see a pattern.
This seems like a more fitting name now that this script has evolved
into more of a general purpose high-level CSV tool.
Unfortunately this does conflict with the standard csv module in Python,
breaking every script that imports csv (which is most of them).
Fortunately, Python is flexible enough to let us remove the current
directory before imports with a bit of an ugly hack:
# prevent local imports
__import__('sys').path.pop(0)
These scripts are intended to be standalone anyways, so this is probably
a good pattern to adopt.
Christopher Haster