Mainly for ^ for centering:
"%{hi!%}^8s" -> " hi! "
Note we can right bias with the nested modifier!
"%{%{hi!%}^7s%}>8s" -> " hi! "
Also note this does _not_ support fill characters like python's format.
I'm not sure it's tractable with %-modifiers, python's format must get
up to some funky parsing to make this work ("%s<s"?).
I think this also fixed the behavior of left-aligning numbers? Seems we
weren't handling that before.
The previous implementation of punescape nesting only supported one
layer, because I didn't want to rewrite the pure re.sub approach.
But regex is not a pushdown automaton!
I.e. it's impossible to match both of these correctly:
%{a%}s%{b%}s
%{a%{a%}s%}s
This rewrites punescape and psplit to properly parse the punescape
string, recursing when we encounter "%{" and terminating on "%}s".
As a plus this shows that the punescape grammar is sound. This was a bit
up in the air with the hacky regex globbing.
Maybe a bit overkill, but I needed more flexibility around adding
arbitrary whitespace.
New modifiers:
%s A space
%{ Start a substring
%}s End and format a subtring
%aaa[mod] Repeat this mod aaa times
With this, it's easy to add arbitrary spaces:
"%8s" -> " "
Or equivalently:
"%8{ %}s" -> " "
Which allows arbitrary repitition of any substring:
"%4{hi!%}s" -> "hi!hi!hi!hi!"
This substring modifier includes its own format string, which allows for
some really nice padding normally impossible in printf:
"%{%(a)d/%(b)d%}8s" % {a:1,b:2} -> " 1/2"
"%{%(a)d/%(b)d%}8s" % {a:12,b:34} -> " 12/34"
This tweaks how we build legends in plot.py and plotmpl.py to merge
legend labels if they would end up identical (same label, same color,
same format, char, linechar, etc).
Identical labels are confusing anyways, so we might as well minimize the
size of the legend when this happens.
---
Though the real motivation for this is to simplify legend labels that
span multiple subplots. Before, you had to awkwardly glob out subplot
labels you didn't want repeated in the legend:
./scripts/plot.py test.csv \
-L3,readed=lfs3 \
-L3,progged= \
-L3,erased= \
-L2,readed=lfs2 \
-L2,progged= \
-L2,erased=
But now you can specify them willy-nilly, and in the final legend any
redundant labels will be automatically merged:
./scripts/plot.py test.csv \
-L3=lfs3 \
-L2=lfs2
So far, I think the use of ratios for subplot widths/heights has worked
well, with the exception of the default behavior for repeated neighbors
being a bit garbage.
Before, the default was a simple 0.5x of the current row/column:
./scripts/csv.py test.csv -xx \
--subplot="-ya" \
--subplot-right="-yb -W0.5" \
--subplot-right="-yc -W0.5" \
--subplot-right="-yd -W0.5" \
--subplot-right="-ye -W0.5"
And while this is certainly simple, it's behavior is not the most
intuitive. When -ye takes 0.5x, it takes 0.5x of the _whole_ grid,
squishing -ya + -yb + -yc + -yd into the other 0.5x as needed. As a
result, -ya ends up with 0.0625x of the final grid.
You could argue this is confusing behavior, but I worry trying to make
it "smarter" will just make it more confusing when multiple dirs/
nestings are mixed.
---
But we can at least change the _default_ behavior to be less confusing.
Now, instead of defaulting to 0.5x, we keep a sum of the number of
subplots seen in the current direction (row vs column), and default the
next subplot's width/height to 1/n.
As a result, repeated subplots end up like the following:
./scripts/csv.py test.csv -xx \
--subplot="-ya" \
--subplot-right="-yb -W0.5" \
--subplot-right="-yc -W0.3333333" \
--subplot-right="-yd -W0.25" \
--subplot-right="-ye -W0.2"
Which may look crazy, but cancels out the nested ratio so the final grid
is a set of evenly distributed columns.
---
Maybe this is still too clever and will need to be reverted in the
future, but in the meantime it provides a nice default for the common
use case of repeated subplots.
These just expose the low-level w/hpad and w/hspace controls available
in matplotlib's constrained_layout.
---
I think something funky might be going on with matplotlib's
constrained_layout. I've noticed a relatively annoying amount of padding
as the number of plots in the grid grow quite large. Though as is usual
with plotmpl.py, this may just be my own fault with the amount of hacks
being applied.
--w/hpad and --w/hspace provide a temporary workaround by overriding the
low-level padding controls in matplotlib's constrained_layout (--w/hpad
should probably be preferred, --w/hspace seems to be a legacy option).
Though, while a temporary solution, these are probably a good idea to
keep around for easy tweaking of plot padding.
This was completely broken due to the renderer ignoring s.xspan. As a
result, subplots could end up rendered multiple times if they spanned
neighboring subplots.
Quick example:
./scripts/plot.py test.csv -xx \
--subplot="-ya" \
--subplot-right="-yb" \
--subplot-below="-yc"
Fortunately the fix is easy, just make sure to increment x_ += s.xspan
as we render subplots across the x-axis.
Curiously the behavior was already correct for the y-axis, I guess
because the y-axis is quite a bit more complicated with how it crosses
multiple lines.
A small tweak, but this resolves some confusing interactions between
subplot subplots and regular subplots.
Consider:
./scripts/plot.py test.csv -xx \
--subplot=" \
-ya \
--subplot-below=\" \
-yab\"" \
--subplot-right=" \
-yb \
--subplot-below=\" \
-ybb\""
You would normally expect -yab and -ybb to end up side-by-side. But
because regular subplots were parsed fully before subplot subplots, -yab
confusingly ended up beneath the sum of -ya + (-yb + -ybb).
The small tweak of prioritizing subplot subplots fixes this, and
results in the expected 2x2 grid of plots.
A slightly different syntax I found while exploring generic/template
syntax in other languages. Instead of multiple brackets/parens for
specialization, just deliminate by (or type) fields from regular fields
with a semicolon:
Before:
-fx=enumerate()
-fy=enumerate[a,b]()
-fz=accumulate[a,b](z)
After:
-fx=enumerate()
-fy=enumerate(a,b;)
-fz=accumulate(a,b;z)
The result is a flexible call syntax that avoids overloading operators
future exprs may want to use.
And if we ever want type specialization, we can always add more
semicolons:
-fx=enumerate(int;;)
-fy=enumerate(float;a,b;)
-fz=accumulate(frac;a,b;z)
By fields are very different from normal fields in exprs (no type
checking, restricted subexprs, etc), so it makes sense to give them
separate syntaxes to clarify this distinction and improve readability.
This commit adopts optional square brackets for by fields, mimicking
generic/template specialization found in other languages:
Before:
-fx=enumerate()
-fy=enumerate(a,b)
-fz=accumulate(z,a,b)
After:
-fx=enumerate()
-fy=enumerate[a,b]()
-fz=accumulate[a,b](z)
Hopefully the readability argument is pretty obvious.
I went with square brackets to avoid parser ambiguities with <>. To be
honest I've never understood why C++ went with <>, array/function
confusion seems easier to resolve than ambiguous binary/index syntaxes,
but what do I know.
Just by hiding -C/--context, -W/--width, --color from argparse unless
a related flag (-h/--help, -A/--annotate, etc) is found in sys.argv.
This is the same trick we use in test.py/bench.py/perf.py.
---
In other news my litmus test that the scripts work was broken.
This does _not_ error if a script errors:
$ for f in scripts/*.py ; do $f --help ; done
An alternative that works is piping stdout to /dev/null, Python's
exceptions go to stderr by default:
$ for f in scripts/*.py ; do $f --help >/dev/null ; done
This better matches the runners' new -Q/--query-define flag, and, thanks
to some argparse trickery, is simpler implementation wise.
Example:
$ ./scripts/code.py lfs3.o -Qsize
66570
$ ./scripts/stack.py -Qlimit lfs3.ci
3312
The only downside is this takes the --small-table shortform flag, but
--small-table doesn't really need a shortform flag.
The fact that we don't include implicit defines in bench/test output
means we need to query the runner for these surprisingly often. So it'd
be nice to have an easier API than sedding the list output.
Some examples:
$ ./scripts/test.py -QBLOCK_SIZE
4096
32768
$ ./scripts/test.py --query-implicit-define=BLOCK_SIZE
4096
$ ./scripts/test.py --query-permutation-define=BLOCK_SIZE
32768
$ ./scripts/test.py -QBLOCK_SIZZLE
(errors)
Unlike --list-*defines, --query-*defines:
- Separates by newline
- Errors if define is not found
Other than that, --query-*defines uses more-or-less the same code
internally.
For consistent ordering in later scripts. The previous
-F=min(enumerate()) trick mostly worked, but would get messed up by
running things in parallel (-j).
I've already confused myself a couple times looking at script output,
which is never a good sign.
This little per-process counters weren't updated in the move to
cumulative-by-default probes, and were summing already cumulative
results.
I was looking at something like 3 trillion bytes read and was thinking
there was no way that could be right.
This reworks -P/--powerloss to be more consistent with other flexible
flags (-D/--define, -S/--probe, etc):
- Tweaks -P/--powerloss to accept multiple flags (-Pnone -Plinear)
instead of a comma-separated list (-Pnone,linear)
- Adopts an expr-like grammar similar to -Dx='range(3)', -Sx=123shz, etc
(see below)
- Generalizes run_powerloss_linear and run_powerloss_log to accept
start/stop/step conditions, allowing for range and logrange exprs
with minimal work
---
The new expr-like grammar follows what's worked well for -D/--define,
-S/--probe, etc, in which parens can be used to parameterize some of the
more complex scenarios. This makes the -P/--powerloss grammar more
consistent, less ad-hoc, easier to parse, while also providing
flexibility for future powerloss exprs.
As an example, bounded range/logrange variants of linear/log were easy
to add without each needing their own little syntax:
- none -> none - Run with no powerlosses
- linear -> linear - Run with linearly-decreasing powerlosses
- log -> log - Run with exponentially-decreasing pls
- n -> permute(n) - Run all permutations of n powerlosses
- exhaustive -> exhaustive - Run all powerloss permutations
- {1,2,3} -> list(1,2,3) - Run explicit list of powerlosses
- added range(a,b,s) - Run explicit range of powerlosses
- added logrange(a,b,s) - Run explicit range of 2^n powerlosses
- :1248g1 -> :1248g1 - Run custom leb128-encoded set of pls
Note we still keep :-prefixed leb128-encoded powerlosses as is. This is
enough of its own syntax that trying to map it to an expr doesn't really
make sense. And is humorously compatible with most future grammars.
- -S/--probe - Specify a probe to sample.
- -x/--probe-step - Sample probes every n steps.
- --probe-runfreq - Sample probes at this frequency in hz.
- -X/--probe-simfreq - Sample probes at this frequency in simulated hz.
Also:
- --trace-simfreq - Sample trace output at this frequency in
simulated hz.
These give finer grain control over which probes we measure during
benching, and how we measure them.
These also introduce several exciting bench features:
- -S/--probe provides the ability to easily filter which probes you're
interested in at runtime.
This should replace the growing use of MASK defines in the benches.
- -x/--probe-step makes it easy to relax sampling rate when the amount
of data overwhelms later scripts.
This should replace the growing use of STEP defines in the benches.
- The additional concept of simfreq, which allows perf-esque sampling in
simtime. This provides another option for intuitively relaxing probe
sampling rate without sacrificing reproducibility.
(runfreq depends on wall time, so good bye reproducibility, though may
still be useful in interactive contexts.)
Note -S/--probe and -x/--probe-step replace MASK/STEP defines, which
have already proved their usefulness, but required reimplementation in
every bench case. An obvious contender to move into the bench_runner!
---
Note note that -S/--probe also supports some simple sample expressions,
allowing flexible step/simfreq/runfreq at the per-probe level:
- -Swrite=100 - Sample probe "write" every 100 steps
- -Swrite=100rhz - Sample probe "write" 100 times a runtime second
- -Swrite=100shz - Sample probe "write" 100 times a simulated second
Though I wonder how long it will take before I forget this feature
exists.
Adds a set of flags to query the bench_runner for available probes:
- --list-probes - List estimated probes
- --list-suite-probes - List estimated probes for each bench suite
- --list-case-probes - List estimated probes for each bench case
What's fun though, is we don't actually know the bench probes at compile
time, since the BENCH_* macros take a C string. But we're already
preprocessing bench_*.toml with Python, so guessing what probes are
available is easy with a bit of regex:
BENCH_(?:STOP|F?RESULT)\( *"((?:\\.|[^"])*)"
This does make the --list*probes flags best effort, but I think unlikely
to break in practice.
Mainly to make space for some planned bench flags, while also preferring
"step" over "period" (for consistency), and "runfreq" over "freq" (to
differentiate from "simfreq" in the future).
In runners:
- -s/--step -> --step
- --trace-period -> --trace-step
- --trace-freq -> --trace-runfreq
In scripts:
- --record -> -e/--record
- --perf-period -> --perf-step
- --perf-freq -> --perf-runfreq
- --include -> -i/--include
---
One thing that makes this work is the new sys.argv regex trick, where we
try to predict what mode the script will run in by prematching known
mode-switch flags before handing things off to argparse.
Note:
- Hiding flags from argparse risks confusing help-text, so we include
all flags if we see -h/--help in sys.argv.
This doesn't work for the help-text printed if argparse errors, but we
can only do so much. Maybe argparse only showing relevant flags for
the given mode is ok?
- We use -[^-]*[hf].* for shortform flags, which should also match
multiple shortform flags in a single arg (-fhfhfh).
- This requires the conflict_handler='ignore' hack to work, but these
scripts already needed it anyways.
The litmus benches are really only intended for introspection/debugging/
cool plots/etc. They're interesting to poke around with and cover a wide
range of littlefs's data-structures, but are not very rigorous.
To make this more clear for new users, added a new litmus flag for
benches:
litmus = true
This doesn't change anything about how the bench is run, but serves as a
marker to hint that the bench is intended for non-rigorous benchmarking.
---
In the makefile, litmus tests are disabled by default at runtime
(--no-litmus). This is to limit `make bench` to benches that are useful
for performance comparisons.
With --no-litmus at runtime, the litmus benches are at least compiled
into the bench_runner, which should hopefully encourage keeping them up
to date with code changes. Eventually we should also run them in CI, but
only to check for runtime errors.
Unlike our tests, we're not really worried about compile time at the
moment due to how few/small our benches are.
Not sure when this was introduced, but it looks like we were
unintentionally double spacing columns in our table renderer.
The problem is we add spaces for both fields and notes:
a b c d
the_thing 100 (+10%) 200 (+20%) 300 (+30%)
But unconditionally, so if there are no notes (the common case), the
fields end up double-spaced:
a b c d
the_thing 100 200 300
Fixed by checking x[1], and only adding the second space if we have any
notes:
a b c d
the_thing 100 200 300
---
The funny thing is, after using this table renderer for so long, I
assumed the double spacing was intentional.
And maybe it should be? Double-spacing does help visually separate
neighboring columns at the cost of horizontal density. The only problem
being that we really _don't_ have much horizontal density to play with.
Many of our table scripts already run past the 80-col mark just due to
how much data we want to show.
If we do want to double space in the future, we should at least double
space after notes as well for consistency. The current impl appears to
not be able to make up its mind!
This adds -U/--undefine as an inverse -D/--define, allowing you to
select results where a given field does _not_ match a set of
values/globs.
For example, make bench-marks, which need to ignore stack/heap/usage
probes as a special case, can easily filter like so:
$ ./scripts/csv.py test.csv -Uprobe=stack,heap,usage
---
One thing globbing is pretty bad at is inverse matches. This is
_usually_ easy enough to work around, but has been an annoyance enough
times that I think _some_ option to inverse filter is warranted.
I'm not sure -U/--undefine is the best name for this, since field isn't
really "undefined" as a result (well kinda? if you're relying on
implicit by/field rules?), but it gets the job done.
How long has there been a whole key dedicated to percentages sitting on
my keyboard!?
There's some funky business with format strings in argparse, but this
was already worked around for dbgbmap.py's -%/--usage flag.
These are useful for customizing the table renderer's header and total
labels:
./scripts/csv.py test.csv -ba -fc \
-Hc='c(MiB/s)' \
--tlabel='wow so many (%(c)s)'
a c(MiB/s)
x 6
wow so many (6) 6
In theory header labels could be controlled by the field names
themselves, but our use of Python's namedtuples internal is quite
limiting.
The immediate use case is -Hprobe=bench+probe in the bench-related
rules, but it may also be useful for adding units such as in the above
example.
---
I considered adding these to all csv scripts, but decided that was too
much. punescape modifiers are probably a good line for what should be
limited to csv.py.
This flag has proven useful in external scripts, might as well give it a
short form.
-t is also an infrequently used flag, so I think the risk of collision
is low even across all csv scripts. The only existing use is in
test/bench.py for -t/--trace (and apparently in gcov for -t/--stdout?).
The lowercase -c/--compare felt clunky. I think because we tend towards
using uppercase for flags that operate on csv rows, such as -D/--define,
-L/--add-label (plot.py), etc.
This is useful for enforcing an upper-bound on fracs based on their
total component.
Technically possible via decomposing + min + recomposing, but... Well
which one do you think is easier?
- saturate(x)
- frac(min(max(int(x), 0), total(x)), total(x))
And this assumes x is easily available and not some other expr (though
chaining csv.py could work around that).
---
The motivation for this was `make bench-widths`, where one read
benchmark could ruin the entire column due to introducing infinities.
Now:
make bench # (squished a bit)
probe readed progged erased
b_wt_seq+w 1.0/1.0 (100.0%) 31.7/256.0 (12.4%) 4096.0/4096.0 (100.0%)
b_wt_random+w 1.0/1.0 (100.0%) 15.3/256.0 (6.0%) 4096.0/4096.0 (100.0%)
b_wt_logging+w 1.0/1.0 (100.0%) 15.4/256.0 (6.0%) 4096.0/4096.0 (100.0%)
b_wt_many+w 1.0/1.0 (100.0%) 16.1/256.0 (6.3%) 4096.0/4096.0 (100.0%)
b_rt_seq+r 1.0/1.0 (100.0%) 256.0/256.0 (100.0%) 4096.0/4096.0 (100.0%)
b_rt_random+r 1.0/1.0 (100.0%) 256.0/256.0 (100.0%) 4096.0/4096.0 (100.0%)
b_rt_many+r 1.0/1.0 (100.0%) 256.0/256.0 (100.0%) 4096.0/4096.0 (100.0%)
TOTAL 1.0/1.0 (100.0%) 149.0/256.0 (58.2%) 4096.0/4096.0 (100.0%)
# ^- notably not infinity
A simple float variant of the CsvFrac type:
- frac(1.5,2) => 1/2 (50.0%)
- ffrac(1.5,2) => 1.5/2.0 (75.0%)
Useful for `make bench-widths` (previously make bench-bus), where we
want to find the average buffer utilization:
probe readed progged erased
b_rbyd+create 1.0/1.0 (100.0%) 13.8/256.0 (5.4%) ∞/4096.0 (∞%)
b_rbyd+delete ∞/1.0 (∞%) ∞/256.0 (∞%) ∞/4096.0 (∞%)
b_rbyd+fetch 1.0/1.0 (100.0%) ∞/256.0 (∞%) ∞/4096.0 (∞%)
b_rbyd+lookup 1.0/1.0 (100.0%) ∞/256.0 (∞%) ∞/4096.0 (∞%)
b_rbyd+usage ∞/1.0 (∞%) ∞/256.0 (∞%) ∞/4096.0 (∞%)
b_wt_seq+w 1.0/1.0 (100.0%) 31.7/256.0 (12.4%) 4096.0/4096.0 (100.0%)
b_wt_random+w 1.0/1.0 (100.0%) 15.3/256.0 (6.0%) 4096.0/4096.0 (100.0%)
b_wt_logging+w 1.0/1.0 (100.0%) 15.4/256.0 (6.0%) 4096.0/4096.0 (100.0%)
b_wt_many+w 1.0/1.0 (100.0%) 16.1/256.0 (6.3%) 4096.0/4096.0 (100.0%)
TOTAL ∞/1.0 (∞%) ∞/256.0 (∞%) ∞/4096.0 (∞%)
Now that we have 4 types, the cast matrix gets a bit complicated, but
this is side-stepped a bit by a custom __frac__ hook.
---
Some other tweaks to csv.py:
- Added CsvFold.type to typecheck folds _after_ we know the expr's final
type.
- Adopted CsvFfrac as an output for most of the math functions/folds
- Stopped early termination of typechecking if we change type!
This was broken: int(float(1.5) + int(1))
This adds __hash__ to CsvFrac, and tweakes __eq__ to be more strict
about equality.
Previously CsvFrac only considered the relevant ratio for equality,
making hashing difficult:
- before: 1/2 == 2/4 => true
- after: 1/2 == 2/4 => false
But now that we have csv.py, with the explicit ratio function, it's
probably a good idea to be strict by default.
Note comparison is unchanged:
- 1/2 < 2/4 => false
- 1/2 > 2/3 => false
---
This popped up during debugging, and would be useful to have around.
Note CsvInt/CsvFloat already implicitly define __hash__ through
namedtuple's implicit __eq__ and friends. But this is disabled in
CsvFrac due to the explicit __eq__.
Which is good because otherwise it would've been wrong with the ratio
comparison!
--no-internal has already proven useful for skipping internal tests for
refactoring, so it makes sense to add --no-reentrant/fuzz flags as well.
--no-fuzz seems particularly useful for when you want to skip the less
targeted fuzz tests:
- with fuzz tests: 634616/634616 passed, in 1239.90s
- with --no-fuzz: 85434/85434 passed, in 423.41s
I also added runtime variants to test/bench_runner and test/bench.py.
These may be useful to skip tests without needing to recompile the
runner.
---
Also tweaked -s/--step to filter permutations in any --list-* flags, for
consistency.
This adds an explicit:
internal = true
As an alternative to:
in = 'lfs3.c'
For marking tests/benches as internal without actually placing them in a
specific source file.
The internal flag and --no-internal have proven suprisingly useful for
running a subset of tests when refactoring, as internal tests break much
more frequently than the high-level API. However, placing all the
internal tests in lfs3.c _has_ put a big strain on compilation/link
times.
`internal = true` now lets you mark tests/benches as internal, without
the extra compile/link overhead. You don't get access to any internal
things, but the flag can still be useful for filtering.
---
The original motivation for this was in the test_fwrite_clip_* tests,
but they ended up using lfs3_bptr_size to check leaf sizes, so oh well.
At least it's a good flag to have around. (In theory these could be made
"fake internal" with a manual bitmask, but it doesn't seem worth the
potential maintenance headache for saving a bit of link time. Though
maybe in the future priorities will change.)
Also cleaned up the handling of None in test/bench config a bit. Now
None should be equivalent to missing config fields, at the cost of more
noise in the Python code. None vs missing always feels unusually clunky
in Python.
Turns out mt.isinf is happy to accept non-primitive floats (such as
CsvFloat) as long as __float__ is defined. But CsvInt expects a float
inf, not a CsvFloat, so things explode later.
Fixed by explicitly casting to float if mt.isinf, instead of passing
as-is.
Also tweaked CsvInt/CsvFloat constructors to not bother checking
isinstance, unconditional int/float casts are probably cheaper than the
condition in Python.
This mirrors test_runtime in test.py, which has been useful for finding
test cases that are slowing down our tests.
Though note bench.py's output is per-probe, so summing bench_runtime
would be longer than the total runtime of the bench if multiple probes
are involved. Probes can be nested, so I'm not sure this is avoidable. I
guess it's the worst-case runtime if all probes were run independently?
Also note, confusingly, bench_runtime is cumulative while bench_simtime
remains per-sample. Maybe this will help prevent interchanging the two?
This is the inverse of accumulate, returning the difference between
subsequent results. In theory accumulate(delta(x)) and
delta(accumulate(x)) are noops.
This is particularly useful for normalizing our bench n value in
scripts. It's the only value still returned as a cumulative measurement,
which is a bit inconsistent, but necessary for uniquely identifying
probe steps.
Note this matches the behavior of mods, e.g. I would expect this to not
break if ORDER is missing:
./scripts/csv.py \
-bcase='%(case)s+%(probe)s+%(ORDER)s' \
-ft=accumulate(bench_simtime, case, probe, ORDER)
Normally the expr compiler would force typechecking of ORDER, giving it
a default value of int(0) if missing, but we intentionally bypass
typechecking in enumerate/accumulate's by fields since they may be
strings.
I think this is currently only possible with overlapping by/field
fields, but hiding results with conflicting by fields is not ideal.
Especially since this function is central to so many scripts:
cat test.csv
a,b,c
x,2,1
x,1,2
x,1,3
Before:
./scripts/csv.py test.csv -ba -bb -fb -fc
warning: by fields are unstable
a,b b c
x,2 2 1
TOTAL 4 6
After:
./scripts/csv.py test.csv -ba -bb -fb -fc
a,b b c
x,2 2 5
x,2 2 1
TOTAL 4 6
This solves the main issue with unstable by fields, so no more warning.
Note that some features rely on by being unique to work (added/removed
numbers, compare fields, etc). They shouldn't error, but may be
incorrect/unintuitive with conflicting by fields, so avoiding
conflicting by fields is still a good idea.
So it turns out this _can_ happen, without an in-script coding error.
Consider the behavior of a script with overlapping by/field fields:
$ cat test.csv
a,b
x,2
x,1
x,1
$ ./scripts/csv.py test.csv -ba -bb -fb
During the first fold, rows 2 and 3 will contain b=1, but during the
second fold they will have been merged, resulting in b=2.
So, relaxing to a warning for now. Maybe the table renderer should be
rewritten to avoid folding? (note diffing results may be tricky)
The main changes:
- Added TEST_DEFINES and BENCH_DEFINES to allow overriding the default
test/bench define header:
-DTEST_DEFINES=my_test_defines.h
Note these are VERY different from LFS_DEFINES upstream. They aren't a
typical header file, and are included multiple times with various
query macros.
It's hacky, but works surprisingly well.
Or maybe I'll just do anything to avoid having to write multiline
macros. Ugh, backslashes.
- Moved more logic into bench/test_defines.h, including everything
needed to integrate other filesystems out-of-tree.
This mostly meant moving all of the cfg initialization logic into its
own query macro (replacing the BENCH/TEST_CFG field macros).
But this also includes a bunch of small tweaks encountered while trying
to get external benchmarks running again.
The external benchmarks include several other filesystems (littlefs2,
SPIFFS, Yaffs2), and I'm hoping this injectable/queryable header thing
will do a good job at avoiding a maintenance headache. (At least a
better job than forking bench_runner.c, which was the previous
solution.)
Now -l/--list-fields includes however many results fit in 36 chars:
$ ./scripts/csv.py --list-fields test.csv
i int # 16,17,14,18,19,15,20,13,12,29,27,28,...
suite ? # bench_p26_wt
case ? # bench_p26_wt_linear,bench_p26_wt_ran...
NO_FRUNCATE int # 0
SIZE int # 2097152
SEED int # 42
The whole point of -l/--list-fields is to give a quick information dump
about what's inside a csv file, and we're already parsing everything to
try to figure out types, so why not?
Much easier to read than head:
$ head -n5 test.csv
i,suite,case,NO_FRUNCATE,SIZE,SEED,BLOCK_SIZE,FILE_SIZE,SIM_...
16,bench_p26_wt,bench_p26_wt_linear,0,2097152,42,65536,64,36...
16,bench_p26_wt,bench_p26_wt_linear,0,2097152,42,65536,64,36...
16,bench_p26_wt,bench_p26_wt_linear,0,2097152,42,65536,64,36...
16,bench_p26_wt,bench_p26_wt_linear,0,2097152,42,65536,64,36...
This extends csv.py's enumerate/accumulate exprs with optional by field
arguments. Each set of by fields gets its own state, allowing multiple
parallel enumerates/accumulates to be processed simultaneously.
This is especially useful when the number of by field sets is unknown.
In theory you could split/merge each by field set with a separate csv.py
call, but it'd be a real pain.
Consider some bench results:
case,n,simtime
bench_rbyd,1,100
bench_rbyd,2,10
bench_rbyd,3,100
bench_btree,1,200
bench_rbyd,4,10
bench_btree,2,20
bench_btree,3,2000
bench_btree,4,200
It was a bit awkward to handle these with csv.py's accumulate, as
accumulate operated strictly per-row, ignoring the case field.
But now with optional by fields:
$ ./scripts/csv.py test.csv \
-bcase -bn \
-fsimtime='accumulate(simtime, case)'
case,n simtime
bench_btree,1 200
bench_btree,2 220
bench_btree,3 2220
bench_btree,4 2420
bench_rbyd,1 100
bench_rbyd,2 110
bench_rbyd,3 210
bench_rbyd,4 220
TOTAL 5700
Note that these by fields are a bit special in csv.py's grammar. So far,
they are the only fields in field exprs that aren't typechecked. The
alternative would be string types in csv.py, but I'm not sure I want to
go that far.
---
It's tempting to try to invert this logic (accumulate(simtime, n)), but
I'm not sure how it would work internally. The duplicate by fields
("case") do get annoying, but specifying them in the expr helps make the
relevant state explicit.
Keep in mind we don't evaluate the actual by fields until much later in
csv.py. Entangling these stages risks confusion (-ba='%(b)s'
-c='enumerate(n)'? hidden by fields? overlapping by+field fields?).
I mean, what would you expect this to do?
max(a) + sum(b)
Whatever your answer is, it's wrong (the way csv.py works, we always
compute folds after expr evaluation). The best option is to error,
matching the behavior of mismatched types.
csv.py's -L/--list-computed was returning some confusing types:
$ ./scripts/csv.py /dev/null -fa='float(1)' -L
a int sum
^-- huh!?
Turns out csv.py's fold typechecking was all broken. Folds can change
the type, but only at the invocation:
$ ./scripts/csv.py /dev/null -fa='sum(float(1))' -L
a int sum
$ ./scripts/csv.py /dev/null -fa='avg(int(1))' -L
a float avg
$ ./scripts/csv.py /dev/null -fa='int(avg(1))' -L
a float avg
This is maybe defensible for explicit folds, since their evaluation is
also lifted, but not so much for things like literals/fields/etc.
---
Fixed by allowing None to indicate a generic fold, and allowing types to
be lazily figured out in csv.compile.
By "simple" I mean any non-globbing suite/case ids.
Non-globbing suite/case ids can be filtered early in the test_runner.
But globbing ids require, surprise, globbing, which is currently handled
by test/bench.py.
---
This greatly speeds up valgrind testing of specific suites/cases,
otherwise things get bogged down during the initial --list-cases due to
the sheer number of test permutations we've accumulated.
Maybe we shouldn't be running the initial --list-cases under Valgrind,
but oh well. This mostly solves the problem without too many changes.
This needed a different name, and "bench probe" is sort of reminiscent
of the "debug probes" you can use to measure things in the real world.
Maybe this is just my embedded engineering background poking through,
but honestly anything is better than a single char m, especially for a
non-integer field.
These seem useful enough to have shortform flags:
- -l/--list-fields - Input fields before processing
- -L/--list-computed - Computed fields and expr dependencies
Note while -L/--list-computed has more information, it's also more
likely to trigger an assert/error due to poorly implemented field exprs.
On one hand, only inferring the used input fields is conceptually
correct because that's how csv.py works. On the other, it doesn't really
make sense for --list-computed to show _less_ information than
--list-fields.
So, showing all inferred types now:
$ ./scripts/csv.py --list-computed test.csv \
-bcase='%(case)s+%(m)s' \
-fsimtime='float(bench_simtime)/1.0e9' \
-fsimthroughput='float(n)/max(float(bench_simtime)/1.0e9,1.0e-9)'
i int .--> case ? ?
suite ? |.-> simtime int sum
case ? -+|.> simthroughput int sum
SKIP_WARMUP int |||
FILE_SIZE int |||
SEED int |||
...
m ? -'||
n int ---+
bench_reads int ||
bench_progs int ||
bench_erases int ||
bench_readed int ||
bench_progged int ||
bench_erased int ||
bench_simtime int ---'
I think this makes --list-computed a strict superset of --list-fields
now.
I was expecting -ba -Fa to sort numerically, but it was not. Turns out
hidden field fields (-F/--hidden-field) without exprs were never
typechecked.
This is not an issue for non-hidden field fields (-f/--field), because
we typecheck these explicitly in compile.
Found from some confusing behavior when by/from fields overlap. It turns
out when this happens (-bhi -Fhi, for example), the generated getattr
for the by field would trigger the __getattribute__ for the overlapping
field field, resulting in a fold on _every add operation_.
Hopefully you can see where this is a bit of a problem when summing a
large number of results (O(n^2)?).
---
Fixed by switching getattr to object.__getattribute__ and reconsidering
csv.py's entire design.
Christopher Haster