Just cleaning up a bunch of outdated TODOs and commented out code, as
well as a little bit of code formatting, and scrubbing airspace/gbatc
names as these are no longer used and will just confuse new users.
Having gbmap/bmap used in different places for the same thing was
confusing. Preferring gbmap as it is consistent with other gstate (grm
queue, gcksums), even if it is a bit noisy.
It's interesting to note what didn't change:
- The BM* range tags: LFS3_TAG_BMFREE, etc. These already differs from
the GBMAP* prefix enough, and adopting GBM* would risk confusion for
actual gstate.
- The gbmap revdbg string: "bb~r". We don't have enough characters for
anything else!
- dbgbmap.py/dbgbmapsvg.py. These aren't actually related to the gbmap,
so the name difference is a good thing.
TLDR: This drops the idea of different bmap strategies/modes, and sorts
out most of the compile-time/runtime conditional bmap interactions.
---
Motivation: Benchmarking (at least up to the 32-bit word limit) has
shown the bmap will unlikely be a significant bottleneck, even on large
disks. The largest disks tend to be NAND, and NAND's ridiculous block
size limits pressure on block allocation.
There are still concerns for areas I haven't measured yet:
- SD/eMMC/FTL - Small blocks, so more pressure on block allocation. In
theory the logical block size can be artificially increased, but this
comes with a granularity tradeoff.
- I've only measured throughput, latency is a whole other story.
However, users have reported lfs3_fs_gc is useful for mitigating this,
so maybe latency is less of a concern now?
But while there may still be room for improvement via alternative bmap
strategies, the risk a concerning amount of complexity. Yes,
configuration gets more complicated, but the real issue is any bmap
strategies that try to track _deallocations_ (the original idea being
treediffing) risk falling leaking blocks if all cases aren't covered.
The current "bmap cache" strategy strikes a really nice balance where it
reduces _amortized_ block allocation -> ~O(log n) without RAM, while
retaining the safe, bug-resistant, single-source-of-truth properties
that come with lookahead-based allocation.
---
So, long story short, dropping other strategies, and now the presence of
the bmap is a boolean flag.
This is also the first format-specific flag:
- Define LFS3_BMAP to enable the bmap logic, but note by default the
bmap will still not be used.
- Define LFS3_YES_BMAP to force the bmap to be used.
- With LFS3_BMAP, passing LFS3_F_GBMAP to lfs3_format will include the
on-disk block-map.
- No flag is needed during mount, the presence of the bmap is determined
by the on-disk wcompat flags (LFS3_WCOMPAT_GBMAP). This also prevents
rw mounting if the bmap is not supported, but rdonly mounting is
allowed.
- Users can check if the bmap is in use via lfs3_fs_stat, which reports
LFS3_I_GBMAP in the flags field.
There's still some missing pieces, but these will be a bit more
involved:
- lfs3_fs_grow needs to be made bmap aware!
- We probably want something like lfs3_fs_mkgbmap and lfs3_fs_rmgbmap to
allow converting between bmap backed/not-backed filesystem images.
Code changes minimal:
code stack ctx
before: 37172 2352 684
after: 37172 (+0.0%) 2352 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38844 2456 800
bmap after: 38852 (+0.0%) 2456 (+0.0%) 800 (+0.0%)
It makes sense, nesting directives (#ifdef) in macro arguments invites
all sort of weird parse errors. Unfortunately, this doesn't leave us
with many options for conditionally including rattrs in LFS3_RATTRS
lists... This is especially important for lfs3_format, where we can
expect many rattrs to depend on compile-time configurations.
To fix the warning, I went ahead and adopted a conditionally predefined
LFS3_RATTR_IFDEF_BMAP before the rattr list. I'm not super happy with
this fix (ugh, missing comma), but it at least avoids a warning and
non-portable behavior.
For some reason emubd had both a path argument to lfs3_emubd_create, and
a disk_path config option, with only the disk_path actually being used.
But the real curiosity is why did GCC only starting warning about it
when copied to kiwibd? path is clearly unused in lfs3_emubd_createcfg,
but no warning...
---
Anyways, not sure which one is a better API, but we definitely don't
need two APIs, so eeny meeny miny moe...
Went ahead and chose the lfs3_emubd_create path param for some
consistency with filebd.
New bmap range tags:
LFS3_TAG_BMRANGE 0x033u v--- --11 --11 uuuu
LFS3_TAG_BMFREE 0x0330 v--- --11 --11 ----
LFS3_TAG_BMINUSE 0x0331 v--- --11 --11 ---1
LFS3_TAG_BMERASED 0x0332 v--- --11 --11 --1-
LFS3_TAG_BMBAD 0x0333 v--- --11 --11 --11
Note 0x334-0x33f are still reserved for future bmap tags, but the new
encoding fits in the surprisingly common 2-bit subfield that may
deduplicate some decoding code.
Fitting in 2-bits is the main reason for this, now that in-flight ranges
look like they won't be worth exploring further. Worst case we can
always add more bm tags in the future. And it may even make sense to use
an entire bit for in-flight tags, since in theory the concept can apply
to more than just in-use blocks.
---
Another benefit of this encoding: In-use vs free is a bit check, and I
like the implication that an in-use + erased block can only be a bad
block.
No code changes:
code stack ctx
before: 37172 2352 684
after: 37172 (+0.0%) 2352 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38844 2456 800
bmap after: 38844 (+0.0%) 2456 (+0.0%) 800 (+0.0%)
So now:
lfs3.c:11482:info: Rebuilding bmap (bmap 37/256)
lfs3.c:11246:error: No more free space (lookahead 0/256)
Instead of the previously somewhat confusing:
lfs3.c:11484:info: Rebuilding bmap (bmap 62/256/256)
lfs3.c:11247:error: No more free space (lookahead 0/0/256)
While the previous info statements did have more info (window +
ckpoint + block count), usually one of these ended up redundant
(window == ckpoint == 0 during ENOSPC, for example).
If before rebuilding the bmap is a valid checkpoint, after is too.
This lets us realloc any blocks that may have been temporarily allocated
when rebuilding the bmap. This probably doesn't matter much except for
low-storage states when blocks are extremely scarce, but allocator
checkpoints are cheap so better safe than sorry.
Code changes minimal (negative?):
code stack ctx
before: 37172 2352 684
after: 37172 (+0.0%) 2352 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38852 2456 800
bmap after: 38844 (-0.0%) 2456 (+0.0%) 800 (+0.0%)
Previously this had a very naive number of parens, which led to a very
confusing night trying to debug some code that looked roughly like this:
LFS3_MAX(1, (false) ? 64 : 256) => 1 ???
Fixed by adding more parens.
I've had this trick in my back pocket for a while, but didn't think it
would be worth the code cost. Benchmarks suggested this was a
bottleneck, so gave it an impl...
But it turned out to be a red herring...
At least the code cost is ridiculously cheap?
code stack ctx
before: 37156 2352 684
after: 37172 (+0.0%) 2352 (+0.0%) 684 (+0.0%)
Oh, sidenote, this also removes shrub trunk fetching, repurposing that
bit as an internal flag for quick-fetches. I don't think fetching shrubs
makes sense anymore? This code was probably leftover from a less-correct
traversal implementation.
---
The basic idea: the most recent trunk contains all the info we need to
fetch a btree node for committing:
- We can infer the rbyd weight from one trunk: The total weight is just
the sum of alt pointer weights + the leaf weight.
- The checksum tags provide the perturb bit, ecksum, etc.
The only thing we can't find from the most recent trunk is the checksum,
but this is already implicit in our CoW branch pointers! (Technically the
weight is as well, but we have to scan the alts anyways.)
So we don't need to scan the entire rbyd if we know the checksum, just
the most recent trunk + checksum tags.
In theory, quick-fetches drop our btree commit runtime from
O(b log_b n + (log b)(log_b^2 n)) -> O((log b)(log_b^2 n)).
---
In practice, this doesn't seem to matter, even on NAND with 128KiB
blocks. We're still dominated by compaction costs, perhaps due to the
poor granularity of NAND's read size?
I'm going to keep this for now just for the peace-of-mind while
benchmarking, but it may be worth removing in the future (or maybe not?
the code size is much less than I was expecting).
At least it simplifies the runtime complexity...
Long story short: %zd != %jd!
This was a simple oversight when writing the bench printing code, and
easy to miss on x86_64 and other modern PCs, but the mistake becomes
very apparent when trying to bench under qemu in thumb mode!
After sleeping on it, allowing crystal_thresh < prog_size makes more
sense than I initially thought, if only to better support the case where
prog_size = block_size (SD/eMMC).
It's true fragments + crystal_thresh were intended to avoid needing to
write padding to raw data blocks, but this only makes sense up until
block padding is cheaper than rbyd overheads. At ~block_size/4, rbyds vs
padded data blocks have roughly the same cost, and at ~block_size/2
rbyds use ~2x the storage due to logging/splitting. At ~block_size/2 we
definitely want to crystallize even if this is still below prog_size.
And it turns out allowing crystal_thresh < prog_size fixes the 512B
block size issues on SD/eMMC we were running into earlier!
---
Implementing this required some tweaks to lfs3_file_crystallize_:
1. We intentionally do not align down partial crystallizations if we
can't satisfy prog alignment, as we risk making no progress
in this case.
2. If we can't satisfy prog alignment, don't mark the bptr as erased.
Resuming crystallization to an unaligned block is an error.
Unaligned progs should already be implicitly padded by the lower bd
caching logic, so not aligning should be all we need to do to pad data
blocks.
Oh, and also relax the crystal_thresh >= prog_size constraints.
Adds a bit of code, but the improved block usage on SD/eMMC will
hopefully be valuable:
code stack ctx
before: 37112 2352 684
after: 37156 (+0.1%) 2352 (+0.0%) 684 (+0.0%)
I confused myself a bit while benchmarking because crystal_thresh <
prog_size was showing some very confusing results. But it turns out the
relevant code was just not written well enough to support this
configuration.
And, to be fair, this configuration really doesn't make sense. The whole
point of the fragment + crystallization system is so we never have to
write unaligned data to blocks. I mean, we could explicitly write
padding in this case, but why?
---
This should probably eventually be either an assert or mutable limit,
but in the meantime I'm just adjusting crystal_thresh at runtime, which
adds a bit of code:
code stack ctx
before: 37076 2352 684
after: 37112 (+0.1%) 2352 (+0.0%) 684 (+0.0%)
On the plus side, this prevents crystal_thresh=0 issues much more
elegantly.
When crystal_thresh < fragment_size, there was a risk that repeated
write operations would oscillate between crystallizing and fragmenting
every operation. Not only would this wreck performance, it would also
violently wear down blocks as each crystallization would trigger an
erase.
Fortunately all we need to do to prevent this is check both
fragment_size and crystal_thresh before fragmenting. Note this also
affects the fragment checks in truncate/fruncate.
---
crystal_thresh < fragment_size is kind of a weird configuration, to be
honest we should probably just assert if configured this way (we never
write fragments > crystal_thresh, because at that point we would just
crystallize).
But at the moment the extra leniency is useful for benchmarking.
Adds a bit of code, but will probably either assert or mutably limit in
the future:
code stack ctx
before: 37028 2352 684
after: 37076 (+0.1%) 2352 (+0.0%) 684 (+0.0%)
There was a mismatch between the lfs3_cfg comment and the actual
crystal_thresh math where crystal_thresh=0 would break things:
- In lfs3_file_flush_, crystal_thresh=0 meant we would never resume
crystallization, leading to terrible, _terrible_, linear write
performance.
- In lfs3_file_sync and lfs3_set, it's unclear if small file commit
optimizations were working properly. I went ahead and added a
lfs3_max(lfs3->cfg->crystal_thresh, 1) just to be safe.
The other references to crystal_thresh all check for >= crystal_thresh
conditions, so shouldn't be broken (except for an unrelated bug in
lfs3_file_flushset_).
The reason for this is because crystal_thresh=1 is technically the lower
bound for this math. Allowing crystal_thresh=0 is just a convenience,
and honestly allowing it may have a been a bad idea. Maybe we should
require crystal_thresh=1 at minimum? I added a TODO.
All the new v3 config needs revisiting anyways, for defaults, etc.
---
Curiously, this actually saved code? My best guess is maybe some weird
code path in lfs3_file_flush_ was eliminated:
code stack ctx
before: 37036 2352 684
after: 37028 (-0.0%) 2352 (+0.0%) 684 (+0.0%)
Too much room for confusion, and potential flag conflicts in the future.
Note it already conflicted with -e/--error-* flags.
--exec is a rather technical flag anyways, and will probably be wrapped
in other ci/script scaffolding most of the time.
I'm not sure if this was ever implemented, or broken during a refactor,
but we were ignoring -s/-S flags when writing .csv/.json output with
-o/-O.
Curious, because the functionality _was_ implemented in fold, just
unused. All this required was passing -s/-S to fold correctly.
Note we _don't_ sort diff_results, because these are never written to
.csv/.json output.
At some point this behavior may have been a bit more questionable, since
we use to allow mixing -o/-O and table rendering. But now that -o/-O is
considered an exclusive operation, ignoring -s/-S doesn't really make
sense.
---
Why did this come up? Well imagine my frustration when:
1. In tikz/pgfplots, \addplot table only really works with sorted data
2. csv.py has a -s/-S flag for sorting!
3. -s/-S doesn't work!
This was a nasty performance hole found while benchmarking.
Basically, any time crystallization is triggered, the crystallization
algorithm tries to pack as much data into as few blocks as possible.
When fruncating (the common, and performance sensitive, use case being
logging), this can lead to the algorithm rewriting fruncated blocks.
What the crystallization algorithm doesn't realize, however, is that
when fruncating/logging, we're probably going to fruncate again on the
next call, so rewriting the block is a waste of effort.
Worst case -- a 1 block file -- this can cause littlefs to rewrite the
entire file on every append.
---
The solution implemented here, which is a bit of a hack, is to use the
actual block start for block alignment instead of the logical
start-of-block referenced by our btree/bshrub.
This solves the fruncating/logging performance hole, with the tradeoff
of using more storage than is strictly necessary. This tradeoff is
probably expected with logging however.
Code changes minimal:
code stack ctx
before: 37024 2352 684
after: 37036 (+0.0%) 2352 (+0.0%) 684 (+0.0%)
We're not currently using these (at the moment it's unclear if the
original intention behind the treediff algorithms is worth pursuing),
and they are showing up in our heap benchmarks.
The good news is that means our heap benchmarks are working.
Also saves a bit of code/ctx in bmap mode:
code stack ctx
before: 37024 2352 684
after: 37024 (+0.0%) 2352 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38752 2456 812
bmap after: 38704 (-0.1%) 2456 (+0.0%) 800 (-1.5%)
Useful for emulating much larger disks in a file (or in RAM). kiwibd
doesn't have all the features of emubd, but this allows it to prioritize
disk size and speed for benchmarking.
kiwibd still keeps some features useful for benchmarking/emulation:
- Optional erase value emulation, including nor-masking
- Read/prog/erase trackers for measuring bd operations
- Read/prog/erase sleeps for slowing down the simulation to a human
viewable speed
- Added --small-total. Like --small-header, this omits the first column
which usually just has the informative text TOTAL.
- Tweaked -Q/--small-table so it renders with --small-total if
-Y/--summary is provided.
- Added --total as an alias for --summary + --no-header + --small-total,
i.e. printing only the totals (which may be multiple columns) and no
other decoration.
This is useful for scripting, now it's possible to extract just, say,
the sum of some csv and embed with $():
echo $(./scripts/code.py lfs3.o --total)
- Tweaked total to always output a number (0) instead of a dash (-),
even if we have no results.
This relies on Result() with no args, which risks breaking scripts
where the Result type expects an argument. To hopefully catch this
early, the table renderer currently creates a Result() before trying
to fold the total result.
- If first column is empty (--small-total + --small-header, --no-header,
etc) collapse width to zero. This avoids a bunch of extra whitespace,
but still includes the two spaces normal used to separate names from
fields.
But I think those spaces are a good thing. It makes it hard to miss
the implicit padding in the table renderer that risks breaking
dependent scripts.
Found when trying to measure ctx of yaffs2, which relies on incomplete
structs to hide some internal state (yaffs_summary_tags, yaffs_DIR).
This is less common in microcontroller filesystems since almost all
structs end up statically/stack allocated, and you can't statically
allocate incomplete structs.
It's not too surprising, but incomplete structs have no associated
DW_AT_byte_size in the relevant dwarf info, which broke ctx.py and
structs.py...
As a workaround, I'm now defaulting to size=0 if DW_AT_byte_size is
missing.
---
With this fix, at least structs.py is able to pick up the later internal
definition of yaffs_summary_tags. ctx.py doesn't because it only looks
at the unique dwarf offset referenced by the function definition, but
I'm hesitant to try anything more clever here.
yaffs_DIR is noteworthy in that there is simply no complete definition.
Internally, yaffs_DIR pointers alias yaffsfs_DirSearchContext structs.
In this case I think returning size=0 is the only reasonable option.
I've been struggling to keep plots readable with --x/ylim-stddev, it may
have been the wrong tool for the job.
This adds --x/ylim-ratio as an alternative, which just sets the limit to
include x-percent of the data (I avoided "percen"t in the name because
it should be --x/ylim-ratio=0.98, not 98, though I'm not sure "ratio" is
great either...).
Like --x/ylim-stddev, this can be used in both one and two argument
forms:
$ ./scripts/plot.py --ylim-ratio=0.98
$ ./scripts/plot.py --ylim-=-0.98,+0.98
So far, --x/ylim-ratio has proven much easier to use, maybe because our
amortized results don't follow a normal distribution? --x/ylim-ratio
seems to do a good job of clipping runaway amortized results without too
much information loss.
This adds NOR-style masking emulation to emubd when erase_value is set
to -2:
erase => 0xff
prog 0xf0 => 0xf0
prog 0xcc => 0xc0
We do _not_ rely on this property in littlefs, and so this feature will
probably go unused in our tests, but it's useful for running other
filesystems (SPIFFS) on top of emubd.
It may be a bit of a scope violation to merge this into littlefs's core
repo, but it's useful to centralize emubd's features somewhere...
Counterintuitively, LFS3_RDONLY + LFS3_BMAP _does_ make sense for cases
where you want to include the bmap in things like ckmeta/ckdata scans.
Though this is another argument for a LFS3_RDONLY + LFS3_NO_TRV build.
Traversals add quite a bit of code to the rdonly build that is probably
not always needed.
---
This just required another bunch of ifdefs.
Current bmap rdonly code size:
code stack ctx
rdonly: 10616 896 532
rdonly+bmap: 10892 (+2.6%) 896 (+0.0%) 636 (+19.5%)
Just a few alloc/eoff references slipped through in the bmap work.
Current rdonly code size:
code stack ctx
default: 37024 2352 684
rdonly: 10616 (-71.3%) 896 (-61.9%) 532 (-22.2%)
This biggest change was tweaking our mtortoise again to use the unused
trunk field for the power-of-two bound. The original intention of using
eoff was an extra precaution to avoid the mtortoise looking like a valid
shrub at any point, but eoff is not available in LFS3_RDONLY.
And we definitely want our mtortoise in LFS3_RDONLY!
---
Note I haven't actually tested LFS3_RDONLY + LFS3_BMAP. Does this config
even make sense? I guess ckmeta/ckdata will need to traverse the bmap,
so, counterintuitively, yes?
Whoops, this was an oversight when readopting lazy grafting.
It turns out the crystallization refactor that led to
lfs3_file_crystallize_ operating directly on file->leaf.bptr was a bit
incompatible with lazy grafting.
If we encounter an error and need to relocate, we need to rewrite any
data in our crystal, _including data in ungrafted leaves_.
By pure luck, the previous lazy grafting implementation side-stepped
this issue by including ungrafted leaves in lfs3_file_lookupnext calls.
This implicitly included the ungrafted leaf in any recrystallizations,
as long as it wasn't modified on error.
---
The fix required two tweaks:
- Recrystallize into a copy in case we hit an error.
Instead of a full lfs3_bptr_t, I just copied the relevant
block_/off_/pos_ pieces we need.
- Include file leaves in the crystallization logic.
Fortunately the multi-data-prioritization loop we already have for
any cached data was relatively easy to adapt for this.
As a plus lfs3_file_crystallize_ can also now short-circuit a
bshrub/btree lookup if the data we're crystallizing happens to be in the
file leaf.
This adds a bit more code, but doesn't break if we hit an error. In
theory this would add stack for the recrystallization copy, but
lfs3_file_crystallize_ is just off the stack hot-path:
code stack ctx
before: 36972 2352 684
after: 37024 (+0.1%) 2352 (+0.0%) 684 (+0.0%)
Another fix I considered -- calling lfs3_file_graft on error -- may have
been a bit less code, but would have moved the stack hot-path under
lfs3_file_crystallize_. An error triggering _more_ progs/commits also
doesn't really sound like the greatest of ideas.
Found by test_ck_spam_fwrite_fuzz.
Currently LFS3_BMAP implies LFS3_YES_BMAP, which is an ugly hack because
I don't want to figure out the BMAP flag logic right now.
As a side-effect, this makes it impossible to test LFS3_BIGGEST without
LFS3_BMAP, which breaks a number of tests that have not been updated to
support >2 format blocks.
Saves a bit of code, at the cost of making this logic a bit more
difficult to read:
code stack ctx
before: 36992 2352 684
after: 36972 (-0.1%) 2352 (+0.0%) 684 (+0.0%)
Now that ungrafted leaves are much more limited in scope, I'm not sure
lfs3_file_weight_ still makes sense as a separate function.
The only call was in lfs3_file_read, where we decide if we bother
flushing things before actually flushing things. Given that we should
now generally graft before expecting lfs3_file_lookupnext to make sense,
relying on lfs3_file_weight_ too much probably hints at a logic mistake.
This logic ends up inlined anyways, so no code changes:
code stack ctx
before: 36992 2352 684
after: 36992 (+0.0%) 2352 (+0.0%) 684 (+0.0%)
We no longer need to discard the leaf, since we can just leave ungrafted
leaves around as long as LFS3_o_UNGRAFT is set.
This let's us flatten lfs3_file_crystallize_, saving a bit of code and
cleaning up the logic a bit:
code stack ctx
before: 37032 2352 684
after: 36992 (-0.1%) 2352 (+0.0%) 684 (+0.0%)
Note that lfs3_file_crystallize_ is still NOINLINE to force it off the
stack hot-path in lfs3_file_flush_, etc.
Continued benchmarking efforts are indicating this isn't really an
optional optimization.
This brings back lazy grafting, where the file leaf is allowed to fall
out-of-date to minimize bshrub/btree updates. This is controlled by
LFS3_o_UNGRAFT, which is similar, but independent from LFS3_o_UNCRYST:
- LFS3_o_UNCRYST - File's leaf not fully crystallized
- LFS3_o_UNGRAFT - File's leaf does not match disk
Note it makes sense for files to be UNGRAFT only, in the case where the
current crystal terminates at the end-of-file but future appends are
likely. And it makes sense for files to be UNCRYST only, in cases where
we graft uncrystallized blocks so the bshrub/btree makes sense.
Which brings us to the main change from the previous lazy-grafting
implementation: lfs3_file_lookupnext no longer includes ungrafted
leaves.
Instead, functions should call lfs3_file_graft if they need
lfs3_file_lookupnext to make sense.
This significantly reduces the code cost of lazy grafting, at the risk
of needing to graft more frequently. Fortunately we don't actually need
to call lfs3_file_graft all that often:
- lfs3_file_read already flushes caches/leaves before attempting any
bshrub/btree reads for simplicity (heavy are not currently considered
a priority, if you need this consider opening two file handles).
- lfs3_file_flush_ _does_ need to call lfs3_file_graft before the
crystallization heuristic pokes, but if we can't resume
crystallization, we would probably need to graft the crystal to
satisfy the flush anyways.
---
Lazy grafting, i.e. procrastinating on bshrub/btree updates during block
appends, is an optimization previously dropped due to perceived
nicheness:
- We can only lazily graft blocks, inlined data fragments always require
bshrub/btree updates since they live in the bshrub/btree.
- Sync forces bshrub/btree updates anyways, so lazy grafting has no
benefit for most logging applications.
- This performance penalty of eagerly grafting goes away if your caches
are large enough.
Note that the last argument is a non-argument in littlefs's case. They
whole point of littlefs is that you _don't_ need RAM to fix things.
However these arguments are all moot when you consider that the "niche
use case" -- linear file writes -- is the default bottleneck for most
applications. Any file operation becomes a linear write bottleneck when
the arguments are large enough. And this becomes a noticeable issue when
benchmarking.
So... This brings back lazy grafting. But with a more limited scope
w.r.t. internal file operations (the above lfs3_file_lookupnext/
lfs3_file_graft changes).
---
Long story short, lazy grafting is back again, reverting the ~3x
performance regression for linear file writes.
But now with quite a bit less code/stack cost:
code stack ctx
before: 36820 2368 684
after: 37032 (+0.6%) 2352 (-0.7%) 684 (+0.0%)
I was wrong! New bmaps containing the old bmap is _not_ sufficient for
lookahead scans, in the case where we rebuild the bmap multiple times
before an mdir commit! The previous bug with lfs3_trv_read +
lfs3_alloc_ckpoint _was_ a bug because the traversal was rdonly, but in
theory the bmap shouldn't have been corrupted.
Since this case is possible, we also need to traverse the on-disk bmap.
Fortunately only when the on-disk bmap does not match the active bmap.
This is probably safer behavior anyways, and means ckmeta/ckdata will
traverse both the in-RAM and on-disk bmaps, which is probably a good
thing in case we need to revert to the on-disk bmap due to power-loss/
error.
---
The implementation got a bit messy, since we only track the on-disk
encoding of the on-disk bmap (we need the encoding for gdeltas to work).
This ended up adding code, and an annoying (avoidable? TODO?) stack
cost, but correct behavior is better than incorrect behavior:
code stack ctx
before: 36856 2368 684
after: 36820 (-0.1%) 2368 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38452 2400 812
bmap after: 38504 (+0.1%) 2464 (+2.7%) 812 (+0.0%)
Once again the inaccuracy of our stack frame calculations strike
again...
This adds %i and %I as punescape modifiers for limited printing of
integers with SI prefixes:
- %(field)i - base-10 SI prefixes
- 100 => 100
- 10000 => 10K
- 0.01 => 10m
- %(field)I - base-2SI prefixes
- 128 => 128
- 10240 => 10Ki
- 0.125 => 128mi
These can also easily include units as a part of the punescape string:
- %(field)iops/s => 10Kops/s
- %(field)IB => 10KiB
This is particularly useful in plotmpl.py for adding explicit
x/yticklabels without sacrificing the automatic SI-prefixes.
So now these lex correctly:
- 1e9 => 1000000000
- 1e+9 => 1000000000
- 1e-9 => -1000000000
A bit tricky when you think about how these could be confused for binary
addition/subtraction. To fix we just eagerly grab any signs after the e.
These are particularly useful for manipulating simulated benchmarks,
where we need to convert things to/from nanoseconds.
This was causing a problem where the bmap was being rebuilt on every
lfs3_trv_read, even though the traversal was opened LFS3_T_RDONLY!
Also added a note on why we don't need to traverse both the active and
on-disk bmaps. Counterintuitively, we don't need to because the new bmap
always contains the entirety of the on-disk bmap.
Code changes minimal:
code stack ctx
before: 36840 2368 684
after: 36856 (+0.0%) 2368 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38440 2400 812
bmap after: 38452 (+0.0%) 2400 (+0.0%) 812 (+0.0%)
Found while benchmarking, our shrub estimates were being recalculated
much more frequently than they should be (every shrub commit). The
problem is that we never staged shrub estimates!
In theory this is fine, shrub estimates don't necessarily need staging.
We update shrub.r.eoff/estimate directly in lfs3_bshrub_commitroot_
after the mdir commit succeeds. But then we _redundantly_ sync
shrub_ -> shrub.r in lfs3_bshrub_commit. Since we never staged the
shrub estimate, we end up with garbage.
---
It's not clear to me this (staging the shrub estimate) is the best fix
for this, but the reason for the redundant shrub sync is the shared
bshrub/btree post-commit path. Added a TODO comment and should look at
this again when not in a time crunch.
Code changes minimal:
code stack ctx
before: 36836 2368 684
after: 36840 (+0.0%) 2368 (+0.0%) 684 (+0.0%)
Now that lfs3_alloc_ckpoint is more complicated, and can error, it makes
sense for lfs3_alloc_ckpoint to be implied by lfs3_mdir_commit.
Most lfs3_mdir_commit calls represent an atomic transaction from one
state -> another, so this saves a bit of code:
code stack ctx
before: 36912 2368 684
after: 36836 (-0.2%) 2368 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38512 2400 812
bmap after: 38436 (-0.2%) 2400 (+0.0%) 812 (+0.0%)
The notable exception being bshrub-related commits in
lfs3_bshrub_commitroot_. Bshrub commits are trying to resolve an
in-flight btree, so the relevant blocks are very much _not_ at rest.
---
I've been hesitant to adopt this mostly just because it makes the
lfs3_mdir_commit* names even more of a mess:
- lfs3_mdir_commit__ -> lfs3_mdir_commit___
- lfs3_mdir_commit_ -> lfs3_mdir_commit__
- lfs3_mdir_commit -> lfs3_mdir_commit_
- added lfs3_mdir_commit
- lfs3_mdir_compact -> lfs3_mdir_compact_
- add lfs3_mdir_compact
- lfs3_mdir_alloc__ -> lfs3_mdir_alloc___
- lfs3_mdir_estimate__ -> lfs3_mdir_estimate___
- lfs3_mdir_swap__ -> lfs3_mdir_swap___
The main change is error propagation in lfs3_alloc_ckpoint. Since
lfs3_alloc_ckpoint writes to disk during bmap rebuilds, it can now fail
in all sorts of ways. Fortunately lfs3_alloc_ckpoint should only ever be
called by write operations, where these errors are be expected.
With bmap rebuild errors now reported correctly, this unblocks most of
the remaining test TODOs:
- Passing test_badblocks
- Passing test_ck
- Passing test_trvs
With this, LFS3_YES_BMAP is now passing all but two tests, which are
still ifndef-disabled as a temporary measure:
- test_btree - We make some low-level assumptions about the lookahead
allocator when testing btrees. It's probably not worth trying to get
this passing with the bmap allocator.
- test_grow - This one does need fixing! We currently don't update
on-disk bmaps correctly when growing the filesystem.
Code changes minimal:
code stack ctx
before: 36912 2368 684
after: 36912 (+0.0%) 2368 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38456 2400 812
bmap after: 38512 (+0.1%) 2400 (+0.0%) 812 (+0.0%)
We need to adjust mids to ignore orphans during dir traversal, but we
shouldn't also adjust the dir position. In theory it shouldn't matter if
we use adjusted/non-adjusted dir positions, but it becomes a problem if
intermediate writes cause those orphans to be cleaned up. Now all your
dir positions are wrong.
Not entirely sure why this only started to fail with the bmap. I'm
guessing it's just due to the additional gstate causing the mdirs to
split differently.
Code changes minimal:
code stack ctx
before: 36920 2368 684
after: 36912 (-0.0%) 2368 (+0.0%) 684 (+0.0%)
Tangential, but toss this on the pile of problems with dir positions.
I'm increasingly convinced we should just remove the concept if we can
get away with it.
There's really no reason to immediately commit the bmap to disk, at
least no until the first mdir commit, when we need to at least discard
the previous bmap state.
We already do all the gstate handling in lfs3_mdir_commit anyways, and
piggybacking on mdir commit lets us get rid of the annoying extra mdir
param in lfs3_alloc_ckpoint.
This does mean a slightly higher risk of needing to re-rebuild the bmap
after a powerloss, but in theory only if the user does something weird
like writing to a file and never calling sync. Most on-disk operations
terminate in an mdir commit as that's how any state change becomes
atomically visibile in littlefs.
Saves a nice bit of stack:
code stack ctx
before: 36920 2368 684
after: 36920 (+0.0%) 2368 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38552 2472 812
bmap after: 38464 (-0.2%) 2400 (-2.9%) 812 (+0.0%)
At least at a proof-of-concept level, there's still a lot of cleanup
needed.
To make things work, lfs3_alloc_ckpoint now takes an mdir, which
provides the target for gbmap gstate updates.
When the bmap is close to empty (configurable via bmap_scan_thresh), we
opportunistically rebuild it during lfs3_alloc_ckpoints. The nice thing
about lfs3_alloc_ckpoint is we know the state of all in-flight blocks,
so rebuilding the bmap just requires traversing the filesystem + in-RAM
state.
We might still fall back to the lookahead buffer, but in theory a well
tuned bmap_scan_thresh can prevent this from becoming a bottleneck (at
the cost of more frequent bmap rebuilds).
---
This is also probably a good time to resume measuring code/ram costs,
though it's worth repeating the above note about the bmap work still
needing cleanup:
code stack ctx
before: 36840 2368 684
after: 36920 (+0.2%) 2368 (+0.0%) 684 (+0.0%)
Haha, no, the bmap isn't basically free, it's just an opt-in features.
With -DLFS3_YES_BMAP=1:
code stack ctx
no bmap: 36920 2368 684
yes bmap: 38552 (+4.4%) 2472 (+4.4%) 812 (+18.7%)
The gbmap's weight is defined by the block count stored in the geometry
config field, which should always be present in valid littlefs3 images.
But our scripts routinely try to parse _invalid_ littlefs3 images when
running in parallel with benchmarks/tests (littlefs3 does _not_ support
multiple read/writers), so this was causing exceptions to be thrown.
The fix is to just assume weight=0 when the geometry field is missing.
The image isn't valid, and the gbmap is optional anyways.
This doesn't fully replace the lookahead buffer, but at least augments
it with known bmap state when available.
To be honest, this is a minimal effort hack to try to get something
benchmarkable without dealing with all the catch-22 issues that a
self-support bmap allocator would encounter (allocating blocks for the
bmap requires a bmap, oh no).
Though now that I'm writing this, maybe this is a reasonable long-term
solution? Having the lookahead buffer to fall back on solves a lot of
problems, and, realistically, it's unlikely to be a performance
bottleneck unless the user has extreme write requests (>available
storage?).
---
Also tweaked field naming to be consistent between the bmap and
lookahead buffer.
This avoids issues with the different traversal paths with an mtree vs
inline-mtree. Previously this was broken when the mtree was inlined.
This order also makes more sense if we want to check mdirs before we
consider the gstate to be trustworthy enough for gbmap traversal.
The idea behind separate ctrled+unctrled airspaces was to try to avoid
multiple interpretations of the on-disk bmap, but I'm starting to think
this adds more complexity than it solves.
The main conflict is the meaning of "in-flight" blocks. When using the
"uncontrolled" bmap algorithm, in-flight blocks need to be
double-checked by traversing the filesystem. But in the "controlled"
bmap algorithm, blocks are only marked as "in-flight" while they are
truly in-flight (in-use in RAM, but not yet in use on disk).
Representing these both with the same "in-flight" state risks
incompatible algorithms misinterpreting the bmap across different
mounts.
In theory the separate airspaces solve this, but now all the algorithms
need to know how to convert the bmap from different modes, adding
complexity and code cost.
Well, in theory at least. I'm unsure separate airspaces actually solves
this due to subtleties between what "in-flight" means in the different
algorithms (note both in-use and free blocks are "in-flight" in the
unknown airspace!). It really depends on how the "controlled" algorithm
actually works, which isn't implemented/fully designed yet.
---
Long story short, due to a time crunch, I'm ripping this out for now and
just storing the current algorithm in the wcompat flags:
LFS3_WCOMPAT_GBMAP 0x00006000 Global block-map in use
LFS3_WCOMPAT_GBMAPNONE 0x00000000 Gbmap not in use
LFS3_WCOMPAT_GBMAPCACHE 0x00002000 Gbmap in cache mode
LFS3_WCOMPAT_GBMAPVFR 0x00004000 Gbmap in VFR mode
LFS3_WCOMPAT_GBMAPIFR 0x00006000 Gbmap in IFR mode
Note GBMAPVFR/IFR != BMAPSLOW/FAST! At least BMAPSLOW/FAST can share
bmap representations:
- GBMAPVFR => Uncontrolled airspace, i.e. in-flight blocks may or may
not be in use, need to traverse open files.
- GBMAPIFR => Controlled airspace, i.e. in-flight blocks are in use,
at least until powerloss, no traversal needed, but requires more bmap
writes.
- BMAPSLOW => Treediff by checking what blocks are in B but not in A,
and what blocks are in A but not in B, O(n^2), but minimizes bmap
updates.
Can be optimized with a bloom filter.
- BMAPFAST => Treediff by clearing all blocks in A, and then setting all
blocks in B, O(n), but also writes all blocks to the bmap twice even
on small changes.
Can be optimized with a sliding bitmap window (or a block hashtable,
though a bitmap converges to the same thing in both algorithms when
>=disk_size).
It will probably be worth unifying the bmap representation later (the
more algorithm-specific flags there are, the harder interop becomes for
users, but for now this opens a path to implementing/experimenting with
bmap algorithms without dealing with this headache.
Christopher Haster