So now the hot path participates in sorting, folding, etc:
$ ./scripts/stack.py ./lfs.ci ./lfs_util.ci \
-Dfunction=lfsr_mount -t -sframe
function frame limit
lfsr_mount 96 2736
|-> lfsr_mdir_commit 512 2368
|-> lfsr_btree_commit__.constprop 336 1648
|-> lfs_alloc 272 1296
|-> lfsr_btree_commit 208 1856
|-> lfsr_btree_lookupnext_ 208 720
|-> lfsr_mtree_gc 192 2560
|-> lfsr_mtree_traverse 176 1024
|-> lfsr_rbyd_lookupnext 160 448
|-> lfsr_bd_readtag.constprop 128 288
|-> lfsr_mtree_lookup 128 848
|-> lfsr_bd_read 80 160
|-> lfsr_bd_read__ 80 80
|-> lfsr_fs_gc 80 2640
|-> lfsr_rbyd_sublookup 64 512
'-> lfsr_rbyd_alloc 16 1312
TOTAL 96 2736
This risks some rather unintuitive behavior now that the hot path
rendering no longer matches the call stack, but in theory the extra
sorting features are more useful?
This is a bit of an experiment, if this is more confusing than useful,
we can always revert to the strict call-order ordering.
Note that you can _usually_ get the call-order ordering by sorting by
limit, but this trick breaks if any call frames are zero sized...
This fixes an issue where mixing recursive renderers (-t/--hot or
-z/--depth) with defines (-Dfunction=lfsr_mount) would not account for
children entry widths. An unexpected side-effect of no longer filtering
the children entries.
We could continue to try to estimate the width without table rendering,
but it would basically need two full recursive pass at this point...
Instead, I've just moved the recursive stuff before table rendering,
which should remove any issues with width calculation while also
deduplicating the recursive passes.
It's invasive for a small change, but probably worthwhile long term.
The downside is this does mean our recursive scripts now build the full
table (including all recursive calls!) before they start printing. When
mixed with unbounded recursive depth (-z0 or --depth=0) this can get
quite large and cause quite a slow start.
But I guess that was the tradeoff in adopting this sort of intermediate
table rendering... At least it does make the code simpler and less bug
prone...
This makes -D/--define more useful in stack.py/perf.py/perfbd.py by no
longer hiding undfined children entries.
For example:
$ ./scripts/stack.py lfs.ci lfs_util.ci -Dfunction=lfsr_mount -t
function frame limit
lfsr_mount 96 2816
|-> lfsr_fs_gc 80 2720
|-> lfsr_mtree_gc 176 2640
|-> lfsr_mdir_commit 576 2464
... snip ...
Now shows all functions in the hot path of lfsr_mount, where before it
would only show functions in the hot path of lfsr_mount that were also
_named_ lfsr_mount.
The previous behavior was technically not wrong... but not very useful
(and confusing).
---
This was actually quite a bit annoying to get working because of the
possibility of function call cycles.
I ended up turning stack.py's result type into a fully connected graph,
which only works because Python has a cycle detector. (Actually this
script is so short-lived we probably wouldn't care if this leaked
memory.)
A nice side effect of this is now all the recursive scripts (stack.py,
perf.py, and perfbd.py) share the same internal result representation
and recursive printing logic, which is probably a good thing.
To better match -z/--depth and -t/--hot.
The fact that these short forms all don't match the first letter of the
long forms is humorous but unintentional. There's only so many letters
in the alphabet!
As a convenience, -d/--diff in our measurement scripts hides entries
that are unchanged by default.
Unfortunately this was broken during a recent refactor that ended up
filtering the line info but not the actual names.
Instead of reverting the broken part of the refactor, I've just moved the
filtering up to where we calculate the names. Hopefully this fixes the
bug while also simplifying this messy chunk of a logic a bit.
This is mainly useful for stack.py, where -t/--hot lets you quickly see
everything that contributes to the stack limit for each function.
This was (and still is) possible with -s + -z, but it was pretty
annoying to use:
- The stack trace rendered _diagonally_ as a consequence of -z, which is
probably the worst use of screen real estate.
- This trick only really worked with -s, which was the opposite order of
what you usually want on the command line: -S.
Adding a special for-purpose -t/--hot flag makes looking at the hot path
much easier, at the cost of more hacky python code (and I _mean_ hacky,
making the hot path selection useful while following exising sort rules
was annoyingly complicated).
Also added -t/--hot to perf.py and perfbd.py for consistency, though it
makes a bit less sense there.
Also also reworked related code in all three scripts: stack.py, perf.py,
perfbd.py. The logic should be a bit more equivalent, and
perf.py/perfbd.py detect cycles now.
stack.py actually already had a simple cycle detector, since we needed
one to calculate stack limits without getting stuck.
Copying this simple cycle detector into the actual table rendering code
lets us print a nice little "cycle detected" message, instead of just
vomiting to stdout forever:
$ ./scripts/stack.py lfs.ci lfs_util.ci -z -s
function frame limit
lfsr_format 320 ∞
|-> lfsr_mountinited 304 ∞
| |-> lfsr_mountmroot 80 ∞
| | |-> lfsr_mountmroot 80 ∞ (cycle detected)
| | |-> lfsr_mdir_lookup 48 576
... snip ...
The cycle detector is a bit naive, just building a new set each step,
but it gets the job done.
As for perf.py and perfbd.py, it turns out they can't actually create
cycles, so no need for a cycle detector. This is good because I didn't
really want to test these scripts again :)
code.py, specifically, was getting messed up by inconsequential GCC
objdump errors on Clang -g3 generated binaries.
Now stderr from child processes is just redirected to /dev/null when
-v/--verbose is not provided.
If we actually depended on redirecting stderr->stdout these scripts
would have been broken when -v/--verbose was provided anyways. Not
really sure what the original code was trying to do...
The original idea was to allow merging a whole bunch of different csv
results into a single lfs.csv file, but this never really happened. It's
much easier to operate on smaller context-specific csv files, where the
field prefix:
- Doesn't really add much information
- Requires more typing
- Is confusing in how it doesn't match the table field names.
We can always use summary.py -fcode_size=size to add prefixes when
necessary anyways.
We already rely on this symbol in these scripts, so might use it to
display the mathematically correct ratio for new entries.
This has the added benefit of ordering new entries vs extremely big
changes correctly:
$ ./scripts/code.py -u test.after.csv -d test.before.csv
function (1 added, 0 removed) osize nsize dsize
test_a - 49 +49 (+∞%)
test_b 19 719 +700 (+3684.2%)
test_c 91 191 +100 (+109.9%)
TOTAL 110 959 +849 (+771.8%)
This is a bit more complicated, but make testmarks really showed how
confusing this could get.
Now, instead of:
suite passed time
test_alloc 304/304 1.6 (100.0%)
test_badblocks 6880/6880 1323.3 (100.0%)
... snip ...
test_rbyd 385878/385878 592.7 (100.0%)
test_relocations 7899/7899 318.8 (100.0%)
TOTAL 548206/548206 6229.7 (100.0%)
Percents/notes are interspersed next to their relevant fields:
suite passed time
test_alloc 304/304 (100.0%) 1.6
test_badblocks 6880/6880 (100.0%) 1323.3
... snip ...
test_rbyd 385878/385878 (100.0%) 592.7
test_relocations 7899/7899 (100.0%) 318.8
TOTAL 548206/548206 (100.0%) 6229.7
Note has no effect on scripts with only a single field (code.py, etc).
But it does make multi-field diffs a bit more readable:
$ ./scripts/stack.py -u after.stack.csv -d before.stack.csv -p
function frame limit
lfsr_bd_sync 8 (+100.0%) 216 (+100.0%)
lfsr_bd_flush 40 (+25.0%) 208 (+4.0%)
... snip ...
lfsr_file_flush 32 (+0.0%) 2424 (-0.3%)
lfsr_file_flush_ 216 (-3.6%) 2392 (-0.3%)
TOTAL 9008 (+0.4%) 2600 (-0.3%)
This matches how diff percentages are rendered, and simplifies the
internal table rendering by making Frac less of a special case. It also
allows for other type notes in the future.
One concern is how all the notes are shoved to the side, which may make
it a bit harder to find related percentages. If this becomes annoying we
should probably look into interspersing all notes (including diff
percentages) between the relevant columns.
Before:
function lines branches
lfsr_rbyd_appendattr 230/231 99.6% 172/192 89.6%
lfsr_rbyd_p_recolor 33/34 97.1% 11/12 91.7%
lfs_alloc 40/42 95.2% 21/24 87.5%
lfsr_rbyd_appendcompaction 54/57 94.7% 39/42 92.9%
...
After:
function lines branches
lfsr_rbyd_appendattr 230/231 172/192 (99.6%, 89.6%)
lfsr_rbyd_p_recolor 33/34 11/12 (97.1%, 91.7%)
lfs_alloc 40/42 21/24 (95.2%, 87.5%)
lfsr_rbyd_appendcompaction 54/57 39/42 (94.7%, 92.9%)
...
Previously, with -d/--diff, we would only show non-zero percentages. But
this was ambiguous/confusing when dealing with multiple results
(stack.py, summary.py, etc).
To help with this, I've switched to showing all percentages unless all
percentages are zero (no change). This matches the -d/--diff row-hiding
logic, so by default all rows should show all percentages.
Note -p/--percent did not change, as it already showed all percentages
all of the time.
Previously, the intention of upper case -Z was the match -W/--width and
-H/--height, which are uppercase to avoid conflicts with -h/--help.
But -z/--depth isn't _really_ related to -W/-H.
This avoids a conflict with -Z/--lebesgue, but may conflict with
-z/--cat. Fortunately we don't currently have any conflicts with the
latter. Since -z/--depth and -Z/--lebesgue are both disk-layout related,
the risk of conflicts are probably much higher there.
Note there's a bit of subtlety here, field _types_ are still infered,
but the intention of the fields, i.e. if the field contains data vs
row name/other properties, must be unambiguous in the scripts.
There is still a _tiny_ bit of inference. For most scripts only one
of --by or --fields is strictly needed, since this makes the purpose of
the other fields unambiguous.
The reason for this change is so the scripts are a bit more reliable,
but also because this simplifies the data parsing/inference a bit.
Oh, and this also changes field inference to use the csv.DictReader's
fieldnames field instead of only inspecting the returned dicts. This
should also save a bit of O(n) overhead when parsing CSV files.
The whitespace sensitivity of field args was starting to be a problem,
mostly for advanced plotmpl.py usage (which tbf might be appropriately
described as "super hacky" in how it uses CLI parameters):
./scripts/plotmpl.py \
-Dcase=" \
bench_rbyd_attr_append, \
bench_rbyd_attr_remove, \
bench_rbyd_attr_fetch, \
..."
This may present problems when parsing CSV files with whitespace, in
theory, maybe. But given the scope of these scripts for littlefs...
just don't do that. Thanks.
With the quantity of data being output by bench.py now, filtering ASAP
while parsing CSV files is a valuable optimization. And thanks to how
CSV files are structured, we can even avoid ever loading the full
contents into RAM.
This does end up with use filtering for defines redundantly in a few
places, but this is well worth the saved overhead from early filtering.
Also tried to clean up the plot.py/plotmpl.py's data folding path,
though that may have been wasted effort.
This makes it easier to read the output, at a cost of these scripts not
terminating if the underlying call sctucture contains loops.
Previously these scripts would not terminate, but at least output the
call tree as they visit each function. This was hard to read, and wasn't
really that useful? If you hit a case with infinite recursion, you can
limit the output size explicitly with -Z.
Note this also drops --tree in stack.py. Since we get more readable
output, this flag is less useful. This simplifies the script a bit.
- Renamed struct_.py -> structs.py again.
- Removed lfs.csv, instead prefering script specific csv files.
- Added *-diff make rules for quick comparison against a previous
result, results are now implicitly written on each run.
For example, `make code` creates lfs.code.csv and prints the summary, which
can be followed by `make code-diff` to compare changes against the saved
lfs.code.csv without overwriting.
- Added nargs=? support for -s and -S, now uses a per-result _sort
attribute to decide sort if fields are unspecified.
- Fixed added/removed count in scripts when an entry has no field in
the expected results
- Fixed a python-sort-type issue when by-field is missing in a result
- Changed --(tool)-tool to --(tool)-path in scripts, this seems to be
a more common name for this sort of flag.
- Changed BUILDDIR to not have implicit slash, makes Makefile internals
a bit more readable.
- Fixed some outdated names hidden in less-often used ifdefs.
Based loosely on Linux's perf tool, perfbd.py uses trace output with
backtraces to aggregate and show the block device usage of all functions
in a program, propagating block devices operation cost up the backtrace
for each operation.
This combined with --trace-period and --trace-freq for
sampling/filtering trace events allow the bench-runner to very
efficiently record the general cost of block device operations with very
little overhead.
Adopted this as the default side-effect of make bench, replacing
cycle-based performance measurements which are less important for
littlefs.
