If only for consistency with DISK_GEOMETRY.
The main reason to keep BENCH_PERBYTE around is to help debug/sanity
check the more complex bus+buffer sim. For that purpose it makes sense
to be able to easily switch modes.
The only downside is if it's more difficult to introduce -DDISK_SIM=1 at
runtime vs compile-time, but eh. Consistency wins.
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 we were using a slightly wrong condition for when to discard
file leaves in lfs3_file_flush_. An unsurprising mistake given size vs
weight subtleties. As a result, it was possible for a write to bypass
the leaf, leaving it with an outdated weight, resulting in an unexpected
hole in the file.
This was surprisingly hard to find as most writes don't leave the leaf
with hole information, only reads.
Fortunately a solution is easy. Just don't use the bptr size here,
instead use the full leaf weight to decide when to discard tracked file
leaves.
Code changes humorously canceling out the Valgrind fix:
code stack ctx
before: 35260 2136 660
after: 35256 (-0.0%) 2136 (+0.0%) 660 (+0.0%)
---
This was found by test_fsync_rwtfrwtf_sparse_fuzz, but only by luck
after the CRYSTAL_THRESH/8 -> CRYSTAL_THRESH/16 tweak.
To prevent a regression, and hopefully catch other bugs like this
(something something cache coherency hard problem), I added a couple
"clip" tests that try to force the cache/leaf bypassing behavior:
- test_fwrite_clip_cache - try clipping the file cache
- test_fwrite_clip_leaf - try clipping the file leaf
- test_fwrite_clip_hole - try clipping the file leaf+hole
test_fwrite_clip_hole does reproduce the bug.
Valgrind was reporting a conditional move on uninitialized read here,
which is correct. If we fetch a data fragment during a read, the
cksize/cksum is meaningless and may be uninitialized.
This was somewhat intentional as both lfs3_bptr_claim and LFS3_o_UNCRYST
are inconsequential when file->leaf is a data fragment. Why bother
checking for a condition that doesn't matter?
But keeping Valgrind happy is significantly more important for
everyone's mental health.
Costs an extra 4 bytes of code:
code stack ctx
before: 35256 2136 660
after: 35260 (+0.0%) 2136 (+0.0%) 660 (+0.0%)
This has been adopted in external benchmarks for a while, as it manages
to push sequential write performance into a much better region of the
diminishing-returns curve.
But hey! Don't take my word for it, let's see the results from our new
bench_runner for the first time:
NOR throughput cs=1/8 cs=1/16
bench_wt_seq+write 15180.0 29402.6 (+93.7%)
bench_wt_random+write 876.2 957.3 (+9.3%)
bench_wt_logging+write 2001.0 2153.4 (+7.6%)
bench_wt_many+write 453.6 453.6 (+0.0%)
NAND throughput cs=1/8 cs=1/16
bench_wt_seq+write 21778.7 22330.1 (+2.5%)
bench_wt_random+write 3583.0 3637.2 (+1.5%)
bench_wt_logging+write 10855.1 10977.1 (+1.1%)
bench_wt_many+write 68.2 68.2 (+0.0%)
Though this doesn't really capture the tradeoffs related to file tails,
storage usage, etc.
In theory sequential writes are happy to start crystallizing as soon as
any data is written, but this leads to significant waste anytime you're
not going to write most of a block.
This gives us much more room for activities.
It makes sense to keep the test disk small: easier parallelization,
heavier emubd with more test features, and if you're running into space
issues in a test, that usually just means you need to be more creative
with how the test is setup.
But for benches, we're interested what happens when we throw a ton of
data at the system.
Also defaulted to noop erases. 0xff erases behave more predictably,
which is useful for testing. But for benching, less work is faster.
- make bench-ops - Show the amount readed/progged/erased
- make bench-bus - Show average readed/progged/erased per width
These just seem immediately useful for a high-level understanding of
benchmark costs. Especially while trying to understand how things
interact with the new bus+buffer sim.
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.
To hopefully make it more clear these are test/bench-dependent, and to
try to be consistent with other make bench-runner, bench-list, etc,
rules.
It felt weird to type this without a hyphen now.
Also renamed test/benchmarks-bottlenecks -> test/bench-bottlenecks,
which is considerably less of a mouthful.
Just makes it a bit easier to fiddle with.
I don't think we can adopt the same prefix => define trick for LFS3_*
defines. If we did it'd pull in things like BENCH_CFLAGS, recursively...
This shows an interesting strategy difference between the test_runner
and bench_runner.
In the test_runner we default to the least-stress configuration, to
minimize bugs unrelated to the current test. But the resulting
configuration is unrealistic, as most use cases on flash will probably
want wear-leveling.
In the bench_runner, we should use a more realistic configuration, so
setting BLOCK_RECYCLES=100 by default makes sense.
So now you can easily run multiple/specific geometries without
recompiling the bench runner:
./scripts/bench.py -DDISK_GEOMETRY=0,1
But note by default we only simulate NOR flash. Spitting out multiple
results by default is confusing.
---
Previously this was possible by either compiling multiple bench runners
(with -DBENCH_NAND), or by explicit specifying full the geometry
(-DREAD_SIZE, -DPROG_SIZE, ..., -DREAD_TIMING, ...) at runtime, but both
were clunky and annoying to parameterize.
DISK_GEOMETRY make it easy, fits well with DISK_SIZE, and adds a field
to help identify the geometry in later scripts.
I considered filling out test_defines.h with multiple geometries as
well, but decided against it. The current idea behind test_runner is to
not test specific geometries, but to instead let individual suites/cases
iterate through the specific READ_SIZEs, PROG_SIZEs, etc, that are
relevant. Still, added DISK_GEOMETRY to test_defines.h for consistency,
but it doesn't actually control anything.
After letting it sit for a bit, the previous byte+op sim comes across as
overly clever in a way that is counter-productive. This is highlighted
by erase-timing scaling in a confusing way when per-op.
Fortunately, with a bit of tweaking, we can instead model the bd sim as
separate bus+buffer timings. This seems more intuitive and is closer to
how the actual hardware works.
---
In the bus+buffer model, bd operations are simulated using two sets of
timing estimates:
buffer timings (nor) bus timings (nor)
read_timing (0) readed_timing (40 ns/B)
prog_timing (1563 ns/B) progged_timing (19 ns/B)
erase_timing (10986 ns/B) erased_timing (0)
Bus timings are a simple multiplier of the bytes read/progged/erased,
while buffer timings are rounded up + aligned to the nearest "width":
bd geometry (nor) bd buffers (nor)
read_size (1 B) read_width (1 B)
prog_size (1 B) prog_width (256 B)
erase_size (4096 B) erase_width (4096 B)
For most purposes, the width should just be the device's read/prog/erase
buffer, but I went with the name width to try to keep it generic and
avoid confusion with "buffer" elsewhere in the codebase.
Some notes:
- Like the byte+op sim, the bus+buffer sim allows penalizing small
operations without artificially limiting what operations are possible.
- Because buffer timings depend on read/prog/erase alignment, there's no
simple equation from ops+bytes to bus+buffer. But as a tradeoff, this
new sim more accurately penalizes unaligned operations.
- All timings are still kept as per-byte instead of per-width. This has
proven to be more flexible when benchmarking, as you usually what
timings to scale with the relevant operation.
- Currently this implemented by changing reads/progs/erases to track the
number of "widths" read/progged/erased after alignment. Which makes
the simtime formula roughly:
simtime = reads*read_width*read_timing + readed*readed_timing
(per-butter) (per-bus)
I considered keeping separate counters for calls (read_calls/
prog_calls/erase_calls?), but not sure there's a good reason to. The
theory behind these widths is there no functional difference between
one big call vs multiple width sized calls, though maybe they would be
useful for debugging?
We can always add these later if they turn out to be useful.
- When widths are disable (0), reads/progs/erases reverts to the number
of read/prog/erase calls.
This is the behavior when BENCH_SIMPLE is defined at compile-time.
Now that csv.py's accumulate/delta functions make it easy to switch
between delta/cumulative results, we might as well make the default
results consistent.
The previous difference between n/bench_runtime vs bench_readed/
bench_simtime risked a lot of confusion.
Note we can't use delta results for n, as it doubles as a unique index
for each probe measurement. If we want consistency the only option is
cumulative results. At least that makes the decision easy.
We're explicitly specifying bash in enough rules that adopting it at the
top-level is probably best for minimizing surprises.
Really the only bash feature we need is process substitution
(`a <(b) <(c)`), but it's a hard feature to pass on.
Though it will be interesting to see if any users run into problems with
this.
So, avg seems like a poor way to merge throughput results, at least for
bench_rbyd (avg create/delete throughputs were wildly different).
To fix this, changed make benchmarks and friends to a two step
calculation:
1. find max results: -fn='max(n)' -ft='float(bench_simtime)/1.0e9'
2. calculate throughput: -fthroughput='avg(float(n) / max(t, 1.0e-9))'
As a plus, this is probably more robust toward accidentally introducing
new by fields.
Note this does require two csv.py calls. csv.py doesn't support exprs
after folding as an intentional simplification (in theory it's always
possible to chain multiple csv.py calls together). In make this can be a
bit annoying since we need SHELL=/bin/bash for subprocess substitution
(for benchmarks-diff specifically), but that's not the end of the world.
--
Also changed -Si='min(enumerate())' -> -Si -Fi='min(enumerate())', in
case users override SUMMARYFLAGS.
This is mostly useful for finding slow test cases, but may be useful for
benches in the future?
I've been running a separate script for this, but adding a rule to the
Makefile makes it a bit easier:
TESTMARKS=1 make test -j && make testmarks-bottlenecks
- Sort by -Si='min(enumerate())'
This was an unexpectedly neat trick for ordering result based on input
csv, without disabling folding like -i/-I.
Note the min is needed because the field is summed before sorting, so
an early case with many permutations can end up after later cases by
default. Because -i/-I also disables folding, it doesn't have this
problem.
- Adopted m -> probe rename.
- Adopted delta expr, so we can now show n correctly, without
accidentally summing already cumulative results.
- Changed benchmark fields simtime+simthroughput -> n+t+throughput
- Adopted throughput=avg(throughput), so TOTAL is somewhat useful?
Unsure if this is the correct way to merge throughput results.
We already hinted PERFGEN in make perf's help text. This extends the
idea to the other rules that depend on env variables.
The default behavior of erroring because make test/bench was run without
the right env variable is confusing enough.
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.
These were copied from external benchmarks, and tweaked/simplified a
bit based on gained experience.
I mostly just wanted something to test the bench runner/scripts, with
bench_rbyd showcasing a low-level litmus benchmark, and bench_wt
showcasing a high-level throughput benchmark.
Though bench_wt has proven to be a _very_ versatile benchmark, and will
likely be the first stop for getting an understanding of high-level
performance implications.
---
Also added bench_helpers.h/c, which includes a couple helper functions:
- bench_helpers_warmup - Warm up the filesystem by writing a 1 block
file 2*block_count times. This is meant to exhaust any preerased
state, post-format lookahead buffers, etc.
- bench_helpers_usage - Find a tight bound on disk usage. This allocates
a bitmap to find the tight bound, unlike lfs3_fs_usage, which is
best-effort. However the bitmap is hidden behind BENCH_HEAP_PAUSE to
prevent messing with parallel heap measurements.
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)
Having BENCH/TEST_NAND ifdefs that enable the relevant timings, but
_not_ the relevant geometry, is certainly a choice.
Defaulting to NAND geometry when BENCH/TEST_NAND is defined is more
useful, if only for minimizing confusion.
No idea how this ended up with the wrong url! I only noticed when tSE
didn't match what was in the datasheet (expected 45ms, found 50ms).
Ugh. I've been copying this url around for a while now without noticing,
so this is not the only repo that needs fixing...
Initial results with the new timing calculations looked weird. Turns
out different block sizes perform surprisingly when they all cost the
same!
Fortunately, erases are the one operation where per-byte vs per-op
timing doesn't really matter, so reverting to only per-byte timing
solves this problem. Now, erasing 2 4KiB blocks should take the same
time as 1 8KiB block, instead of twice as long.
---
Arguably, erase timing shouldn't be _strictly_ linear w.r.t. block size.
There's a reason denser storage usually ends up with larger block sizes
after all. But preventing the block size from messing with per-byte
timings is much more interesting from a filesystem design perspective.
It also matches the behavior of artificially increasing block size to
reduce block allocator pressure.
Unfortunately, this also raises concerns with read/prog timing when
varying geometry is involved... Should we stick to the per-byte timing
in such cases? Is there a better timing model out there without too much
additional complexity?
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.)
This was already made unconditional upstream as a part of LFS_DEFINES
support.
We should try to avoid conditional definitions that depend on
unintuitive conditions (LFS3_CFG in this case). It just makes the whole
codebase more fragile.
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.
- Off-by-one in test_btree_find_general[_sparse]_fuzz
Because we can only create named btrees via splitting, these always
start with one entry. If all operations are randomly selected to be
splits, this can lead to an overflow of the sim buffer (sounds
unlikely, but relatively easily for small N).
The fix is to use a `for (lfs3_size_t i = 1; i < N; i++)` loop to
account for the initial entry. Note we already use this in the
test_btree_split_* tests.
An alternative is allocating space for N+1 entries, but this seems
unintuitive with N usually being associated with the upper bound on
btree size.
- Off-by-one in our sim rename pattern
When renaming, we don't bother to update sim_size, because after the
rename the sim_size size will be unchanged. But this means the
sim_size is out-of-date during the memmove that reinserts the renamed
entry. Buffer overflow!
To fix we just need to use sim_size-1 to account for the temporarily
deleted entry.
This is messy C code, so not surprised it went unnoticed, even though
this pattern ended up in quite a few tests.
Found while running with HEAP=1. This was just intended to test HEAP=1,
but I guess the injected heap hooks result in a more fragile heap? They
increase all allocations by one word, and maybe this reduces alignment
padding? Not exactly sure.
But it's a good argument for maybe adding heap canaries in the future.
Previously we ran Valgrind on all tests, but it's unclear if this will
still be reasonable with the number of tests we have now.
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.
These have been battle-tested in external benchmarks, and have proven
useful for finding a runtime estimate on stack+heap usage.
Of course, to be realistic they need to be cross-compiled and run under
QEMU (which does work!), but even on x86_64 they provide a nice insight
into RAM usage. In practice the only real difference is pointer width
anyways.
---
Enabled by default for the bench runner, these are available if
TEST/BENCH_YES_HEAP and/or TEST/BENCH_YES_STACK are defined.
(This default is provided by the Makefile. At least heap measurements
rely on linker flags, so it probably doesn't make sense to default
enable in the bench runner itself.)
Stack vs heap rely on slightly different mechanisms:
- Stack: Uses GCC's __builtin_frame_address(0) to measure the current
stack usage on entry to every bd operation.
- Heap: Relies on GCC's -Wl,--wrap flags to intercept every malloc/free
call, to track the current heap usage.
These are available via bench/test macros:
- BENCH_STACK() - Maximum stack usage of the current run
- BENCH_STACK_CURRENT() - Current stack usage
- BENCH_HEAP() - Maximum heap usage of the current run
- BENCH_HEAP_CURRENT() - Current heap usage
Note BENCH_STACK_CURRENT() can be useful for separating out the bench's
ctx from total stack usage, similarly to our static analysis.
---
One surprising outcome is that these heap hooks trivially implement a
memory leak detector. Maybe that could be useful in the test_runner as a
cheaper alternative to Valgrind?
kiwibd has been used extensively in external benchmarks, it makes sense
to make it the default bd for the bench runner:
- test_runner - defaults to emubd - more testing features
- bench_runner - defaults to kiwibd - lighter-weight disks
The benefit of kiwibd is the disk is just one big blob of RAM, so
basically no overhead. This is important when benchmarking on multi-GiB
disks.
emubd is much heavy, but as a tradeoff can do quite a bit more:
bad-block simulation, wear simulation, snapshotting, etc.
---
In theory the bd used by each runner can be controlled at compile-time
by defining -DBENCH_EMUBD, etc, but I have a feeling no one will ever
use this.
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.
Now that we have csv.py's accumulate(), this information is strictly
redundant!
$ ./scripts/csv.py test.csv \
-bspecific_permutation_here \
-fbench_creaded='accumulate(bench_readed)'
The point of adding accumulate() was to drop these. We really shouldn't
be doubling the size of the csvs with redundant/derivable data.
This was a funny issue for external benchmarking, where we've focused
mostly on throughput benchmarking so far.
The current throughput approach is to run a benchmark for a given
simtime, and record the number of bytes written after. This is great for
allowing benchmarks to fail gracefully, but doesn't really work with the
current bench runner, which expected a known n in BENCH_START.
We can work around this by calling BENCH_START/STOP a second time
(making a mess of later scripts), but it would be nice if this was fixed
in the bench runner.
---
Humorously, BENCH_START just stores n to be printed out when BENCH_STOP
is called, so this was an easy fix.
Less useful than --list-fields, but fun.
This shows more of the internal expr eval info: input fields + types,
output fields + types + folds, and a small dependency graph showing what
goes where:
$ ./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 ? .--> case ? ?
suite ? |.-> simtime int sum
case ? -+|.> simthroughput int sum
SKIP_WARMUP ? |||
FILE_SIZE ? |||
SEED ? |||
...
m ? -'||
n int ---+
bench_reads ? ||
bench_progs ? ||
bench_erases ? ||
bench_readed ? ||
bench_progged ? ||
bench_erased ? ||
bench_simtime int ---'
Maybe I was just itching to write another ascii-art renderer.
Christopher Haster