This tries to call lfs3_alloc_ckpoint in more correct positions, and
fixes a bug where we _never_ called lfs3_alloc_ckpoint before
finishing crystallization in lfs3_file_readnext and
lfs3_file_truncate/fruncate:
- lfs3_file_crystallize now implicitly calls lfs3_alloc_ckpoint before
both finishing crystallization and grafting.
- lfs3_file_flush_ and lfs3_file_flushonce_ now call lfs3_alloc_ckpoint
at the beginning of each loop iteration.
This may be redundant on some iterations but that's ok.
- lfs3_file_write does _not_ call lfs3_alloc_ckpoint, this is all
handled in lfs3_file_flush_ now.
- lfs3_file_truncate/fruncate still call lfs3_alloc_ckpoint, but just
before lfs3_file_graft.
This matches the lfs3_alloc_ckpoint pattern used for most
lfs3_mdir_commit calls, i.e. checkpoint just before to make it easier
to audit the logic.
- Also moved the pre-fragment crystallization out of the fragment loop,
we should only crystallize once and this makes the code a bit more
readable.
I think this is the source of the extra 8 bytes of stack, but that's
small enough to consider compiler noise.
It's not the biggest problem to not call lfs3_alloc_ckpoint everytime
all blocks are at rest, but it does risk a premature ENOSPC error when
it's still possible to make progress.
This gets more complicated with lazy crystallization/grafting, as block
allocations can end up deferred to operations you might not expect
(lfs3_file_read for example).
Adds a bit of code, but is in theory more correct:
code stack ctx
before: 37888 2416 636
after: 37920 (+0.1%) 2424 (+0.3%) 636 (+0.0%)
This actually binds our custom write/writeln functions as methods to the
file object:
def writeln(self, s=''):
self.write(s)
self.write('\n')
f.writeln = writeln.__get__(f)
This doesn't really gain us anything, but is a bit more correct and may
be safer if other code messes with the file's internals.
This was modified incorrectly for LFS3_2BONLY. We do actually end up
with non-bptr non-data tags here when we encounter btree inner nodes.
Code changes:
code stack ctx
before: 37864 2416 636
after: 37888 (+0.1%) 2416 (+0.0%) 636 (+0.0%)
In lfs3_mdir_namelookup, when compiling with LFS3_2BLOCK, there was an
uninitialized variable warning that just wouldn't go away (temporarily
disabled with the x=x hack).
So, giving up on the err < 0 compiler guidance since it apparently
doesn't work. Instead lfs3_rbyd_namelookup and lfs3_btree_namelookupleaf
unconditionally initialize the problematic variables before their main
loops.
This adds a bit of code, but fighting the compiler just isn't worth the
headache:
code stack ctx
before: 37836 2416 636
after: 37864 (+0.1%) 2416 (+0.0%) 636 (+0.0%)
Like LFS3_RDONLY and LFS3_KVONLY, LFS3_2BONLY opts-out of all of the
logic necessary for filesystems larger than 2-blocks (the mimimum size
of a mutable littlefs image).
This has potential for some pretty big savings:
- No block allocation
- No lookahead buffer
- No btrees (but yes bshrubs)
- No bptrs
- No mtree traversal
Which is I guess ~1/4 of the codebase:
code stack ctx
default: 37836 2416 636
2bonly: 27704 (-26.8%) 1872 (-22.5%) 592 (-6.9%)
This can be combined with LFS3_KVONLY for a small key-value store
compatible with the full littlefs driver:
code stack ctx
default: 37836 2416 636
kvonly: 30792 (-18.6%) 2168 (-10.3%) 636 (+0.0%)
kvonly+2bonly: 22900 (-39.5%) 1736 (-28.1%) 592 (-6.9%)
It may be possible to optimize this further, but, as is the case with
LFS3_KVONLY, balancing config-specific optimization vs maintainability
is tricky.
---
I'm not sure why, but this also reduced the default build's size a bit.
Compiler noise?
code stack ctx
before: 37860 2416 636
after: 37836 (-0.1%) 2416 (+0.0%) 636 (+0.0%)
One of the ideas behind the key-value API is that it is potentially much
cheaper than a full file API. With the key-value API, we get the
guarantee that all data must fit in RAM, and avoid headaches like
random reads/writes and needing to broadcast file state.
For an example of just how much complexity is avoided, the see the
difference between lfs3_file_flushonce_ vs the mess that is
lfs3_file_flush_ + lfs3_file_crystallize + lfs3_file_graft.
However, littlefs is designed around files, and a couple design
decisions hold back how much code saving is possible:
1. littlefs's shrubs are designed around being enrolled in the omdir
linked-list, so internally we still have most of the file open/close
code lumbering around.
2. Directories and traversals still exist, so we'd need the omdir
linked-list anyways, and we still need to broadcast _some_ changes.
3. Despite being intended for small amounts of data, lfs3_set/get can
still be used to create arbitrarily large files. So we still need all
of the bshrub/btree logic.
Which we still need for the mtree anyways, so this isn't really that
much of a downside.
It also may be possible to save more code by aggressively rewriting the
_entire_ read/write path for lfs3_set/get, to not reuse any of the
existing file logic in LFS3_KVONLY mode. But I decided against this due
to concerns around maintainability.
The duplicate lfs3_file_read + lfs3_file_readonce and lfs3_file_flush_ +
lfs3_file_flushonce_ are already enough of a concern.
Anyways, here's LFS3_KVONLY:
code stack ctx
default: 37824 2416 636
kvonly: 30936 (-18.2%) 2168 (-10.3%) 636 (+0.0%)
LFS3_RDONLY + LFS3_KVONLY is also interesting:
code stack ctx
rdonly: 10776 856 508
rdonly+kvonly: 9904 (-8.1%) 888 (+3.7%) 508 (+0.0%)
---
This also added some noise to the default build's code, mainly due to
tweaks in lfs3_file_readnext to allow better reuse in LFS3_KVONLY:
code stack ctx
before: 37824 2416 636
after: 37860 (+0.1%) 2416 (+0.0%) 636 (+0.0%)
As you might expect, this is the inverse of ifdef, and is useful for
supporting opt-out flags.
I don't think ifdef + ifndef is powerful enough to handle _all_
compile-time corner cases, but they at least provide convenient handling
for the most common flags. Worst case, tests/benches can always include
explicit #if/#ifdef/#ifndef statements in the code itself.
And tried to more consistently use lfs3_path_namelen.
In a perfect world we would just use lfs3_path_namelen everywhere and
let the compiler figure it out, but unfortunately this leads to poor
code generation in some places, even with __attribute__((pure)) hacks.
Code changes:
code stack ctx
before: 37832 2416 636
after: 37824 (-0.0%) 2416 (+0.0%) 636 (+0.0%)
- Unconditionally pass buffer as cache_buffer in lfs3_set now that we
rely on LFS3_o_WRSET
- Swapped true -> 1 for non-null don't-care buffer pointer
Saved one instruction as expected for the conditional assignment, but
added a bit of stack. Weird, but probably just compiler noise:
code stack ctx
before: 37836 2408 636
after: 37832 (-0.0%) 2416 (+0.3%) 636 (+0.0%)
This needed a second pass. Changes:
- Small file flushes are no longer limited to LFS3_o_UNFLUSH, which
should avoid bshrubs/btrees being written for small files with
complicated seek+writes. Now, any file small enough is converted
to a small file when we would need to flush.
This does _not_ flush small unsync files that don't need to be
flushed, though I'm not exactly sure how that would happen (broadcast
from file with a different cache size?)
I think this was a regression from previous logic.
- discardbshrub/discardbleaf moved into lfs3_file_sync_, otherwise
we risk discarding the bshrub/bleaf without setting UNSYNC.
This keeps all the state changing logic together.
- We now use lfs3_file_size_ == 0 as the decision for committing bnulls.
size_ == 0 implies bnull, and this avoids the extra headache of
checking for pending small file flush.
Note the ultimate decision on if the file is small is still left up to
lfs3_file_sync. lfs3_file_sync_ just relies on the UNFLUSH + UNCRYST +
UNGRAFT checks to do the last minute small file flush (aside from
asserts).
The UNFLUSH + UNCRYST + UNGRAFT checks look a bit messy, but keep in
mind these optimize to a single bitmask.
Saves a tiny bit of code:
code stack ctx
before: 37856 2416 636
after: 37836 (-0.1%) 2408 (-0.3%) 636 (+0.0%)
Unfortunately neither of these were actually deduplicatable:
1. We can't easily move dir update logic into lfs3_mdir_commit, because
lfs3_mdir_commit has no knowledge of the current did.
Maybe we can add did-related nudge functions, but the logic would
still need to be external to lfs3_mdir_commit. lfs3_mdir_commit only
understands mids.
2. lfs3_alloc_ckpoint continues to be enticing, but fortunately a
previous commit reminded me that we explicitly need to _not_ call
lfs3_alloc_ckpoint before the lfs3_mdir_commit in
lfs3_bshrub_commitroot_.
In theory we could add lfs3_mdir_commit and lfs3_mdir_commit_ to
make lfs3_alloc_ckpoint opt-out, but the lfs3_mdir_commit is already
a bit of a mess. And maybe keeping the lfs3_alloc_ckpoint calls
explicit is a good thing. It's better to ENOSPC than double alloc a
block.
Tweaked: 1400x750 -> 1125x525 (1.5x codemapsvg.py's default)
This is now derived (1.5x) from the default dimensions in codemapsvg.py.
This matches the dimensions that ended up used for the preliminary v3
benchmarks, which are a bit more convenient on devices with smaller
screens.
As for where the 750x350 resolution came from, I'm not entirely sure.
Maybe a random Matplotlib example? It approximates a 2:1 aspect ratio
but with 25 pixels carved out for margins.
Note we like wide aspect ratios over pretty aspect ratios like 16:9,
golden ratio, etc, here:
1. We often cram things into the margins (legends, stack usage, etc)
2. English text is much wider than it is tall (this commit message has
an aspect ration of ~3:1), so wider aspect ratios help readability
Jumping to prototypes in header files is extremely useful, because
that's usually where all the documentation is. But jumping to prototypes
in C files is a bit much. These are usually just uncomment definitions
to keep the compiler happy, and make navigation a bit of a pain.
Unfortunately it doesn't seem like ctags supports per-file-type tag
kinds (at least I couldn't find it in the documentation), but running
ctags twice with the --append flag seems to work.
This adds LFS3_o_WRSET as an internal-only 3rd file open mode (I knew
that missing open mode would come in handy) that has some _very_
interesting behavior:
- Do _not_ clear the configured file cache. The file cache is prefilled
with the file's data.
- If the file does _not_ exist and is small, create it immediately in
lfs3_file_open using the provided file cache.
- If the file _does_ exist or is not small, do nothing and open the file
normally. lfs3_file_close/sync can do the rest of the work in one
commit.
This makes it possible to implement one-commit lfs3_set on top of the
file APIs with minimal code impact:
- All of the metadata commit logic can be handled by lfs3_file_sync_, we
just call lfs3_file_sync_ with the found did+name in lfs3_file_opencfg
when WRSET.
- The invariant that lfs3_file_opencfg always reserves an mid remains
intact, since we go ahead and write the full file if necessary,
minimizing the impact on lfs3_file_opencfg's internals.
This claws back most of the code cost of the one-commit key-value API:
code stack ctx
before: 38232 2400 636
after: 37856 (-1.0%) 2416 (+0.7%) 636 (+0.0%)
before kv: 37352 2280 636
after kv: 37856 (+1.3%) 2416 (+6.0%) 636 (+0.0%)
---
I'm quite happy how this turned out. I was worried there for a bit the
key-value API was going to end up an ugly wart for the internals, but
with LFS3_o_WRSET this integrates quite nicely.
It also raises a really interesting question, should LFS3_o_WRSET be
exposed to users?
For now I'm going to play it safe and say no. While potentially useful,
it's still a pretty unintuitive API.
Another thing worth mentioning is that this does have a negative impact
on compile-time gc. Duplication adds code cost when viewing the system
as a whole, but tighter integration can backfire if the user never calls
half the APIs.
Oh well, compile-time opt-out is always an option in the future, and
users seem to care more about pre-linked measurements, probably because
it's an easier thing to find. Still, it's funny how measuring code can
have a negative impact on code. Something something Goodhart's law.
These are high-risk corner cases for the key-value API, so we should
test them.
At one point I was relying on an optional buffer parameter in
lfs3_file_sync_, but that would have broken if lfs3_set's buffer was
NULL.
This reworks lfs3_set to be able to write small files in a single
commit, by duplicating most of lfs3_file_opencfg.
The only real issue with the naive key-value API was the forced double
commit in lfs3_set. It may not seem like much, but on storage with large
prog sizes (NAND), the difference can be significant.
How significant? Well the difference approaches ~2x. Not because of the
inherent cost of progs, but because prog alignment will force you to
erase ~2x as often.
This small file logic matches lfs3_file_sync's small file logic, so if
you can lfs3_file_sync in one commit, you should be able to lfs3_set in
one commit.
It actually just uses lfs3_file_sync's small file logic for _existing_
files, but unfortunately we need special handling for _non-existing_
files to avoid the stickynote in lfs3_file_opencfg. Fortunately the
small file shrub commit is not too tricky to create on-demand. And as a
funny coincidence, _non-existing_ files, by definition, can't have any
opened file handles, so we don't need to worry about the missing file
broadcast logic.
---
Unfortunately, it turns out duplicating most of lfs3_file_opencfg adds a
huge chunk of code:
code stack ctx
before: 37644 2448 636
after: 38232 (+1.6%) 2400 (-2.0%) 636 (+0.0%)
before kv: 37352 2280 636
after kv: 38232 (+2.4%) 2400 (+5.3%) 636 (+0.0%)
So may need to go back to the drawing board.
This adds a couple functions that treat files as simple key-value pairs:
- lfs3_get - Read a file
- lfs3_size - Get the size of a file
- lfs3_set - Write a file
- lfs3_remove - Remove a file (this one already exists!)
The idea is the only real difference between a filesystem and key-value
store in the microcontroller space is the API, and the key-value API
_is_ much easier to use.
It also opens the door to making the file API opt-out in the future to
trade code cost for feature set. littlefs will probably never be
competitive with other microcontroller-scale key-value stores, but it
may be interesting for systems already using littlefs for other storage.
And don't worry, these are still files, so they can always be opened
with the full file API when more advanced operations are needed.
These APIs also matches the custom attribute APIs, which makes sense
because they're both key-values. Any mismatch should be considered an
API bug, because the best user interface is a consistent one.
This new API is tested in tests/test_kv.toml.
---
At the moment the implementation is naive, just sitting on top of the
file API. This works remarkably well thanks to littlefs's cache
bypassing logic, but does have some downsides:
- lfs3_set always writes two commits: one for the stickynote and one for
the file sync.
Unfortunately this is a fundamental limitation of littlefs's file API.
One nice benefit of lfs3_set is in theory we can bypass this
limitation, but not if we just sit on top of the file API.
- There may be code savings from more tightly integrating the key-value
code.
This also highlighted an awkward corner case with per-file cache
configuration in which the buffer needs to be non-null even if zero. Not
the end of the world, but just a bit awkward. Maybe this deserves
revisiting in the config API rework?
---
Code changes were relatively minimal given that this is a whole new API,
unfortunately the stack took quite a hit:
code stack ctx
before: 37352 2280 636
after: 37644 (+0.8%) 2448 (+7.4%) 636 (+0.0%)
The stack surprised me, but in hindsight it makes sense. In sitting on
top of the reset of the codebase, the key-value API adds very little
code, but every stack allocation in these functions add to the stack
hot-path.
This isn't the end of the world, and it's actually probably a good thing
to have an lfs3_file_t allocated in the stack hot-path. lfs3_file_t's
size has been a bit difficult to track thanks to struct lfs3_info
dominating ctx measurements...
So:
// blablabla this is my cool function
#ifdef LFS3_COOL
int lfs3_cool(lfs3_t *lfs3);
#endif
Mainly because this reads better and moves the compilation conditions
closer to the actual declaration.
One concern is if this will interfere with future doxygen/documentation
generation, but I think we can expect future scripts to be able to parse
relevant ifdefs. For one, we want to make sure to include any required
ifdefs in generated documentation, so if a script can't even parse
ifdefs, uhhhhh...
No code changes.
rbyd.eoff has the relatively unique property of only being useful in
rdwr mode. In rdonly mode we don't care where the next erased-state
starts because we're never going to use it.
Since rbyds are used everywhere, dropping rbyd.eoff has the potential to
save a significant amount of RAM.
---
At least on paper. We were using the field in lfs3_rbyd_fetch to keep
track of the most recent valid commit perturb/eoff, which was a bit
tricky to disentangle.
Disentangling lfs3_rbyd_fetch does add a bit of code to the default
build, but saves code, stack, and ctx in the rdonly mode:
code stack ctx
rdonly before: 10640 816 524
rdonly after: 10616 (-0.2%) 808 (-1.0%) 508 (-3.1%)
default before: 37320 2280 636
default after: 37352 (+0.1%) 2280 (+0.0%) 636 (+0.0%)
In theory we could ifdef the crap out of lfs3_rbyd_fetch to claw back
this code, but 1. 32 bytes of code is really not that much code, 2. the
more rdonly and default diverge the more likely rdonly breaks, and 3. I
think the new code is a bit more readable since it avoids masking
perturb/eoff together until the last minute.
We don't need the staging shrub if we never stage shrubs!
The only hangup was reuse of the staging shrub to load bshrubs/btrees in
lfs3_file_fetch (we need to be able to fallback to the previous shrub if
we error in lfs3_file_resync), but this can be handled with a stack
allocated shrub.
If btree-leaf-caches make a return, we would need to stack allocate this
anyways due to the lopsided cost of the main/staging btrees/bshrubs
introduced to avoid wasting space on the useless
staging-shrub-leaf-cache.
This saves some code in LFS3_RDONLY, and apparently an instruction or
two in the default build (I guess stack loads/stores are cheaper?):
code stack ctx
rdonly before: 10680 840 524
rdonly after: 10640 (-0.4%) 816 (+0.0%) 524 (+0.0%)
default before: 37324 2280 636
default after: 37320 (-0.0%) 2280 (+0.0%) 636 (+0.0%)
It's not apparent in ctx because lfs3_info.name dominates (guh), but
this does save some RAM in lfs3_file_t:
rdonly ctx
lfs3_file_t before: 136
lfs3_file_t after: 112 (-17.6%)
It does add some stack cost to lfs3_file_fetch, but because this isn't
on the stack hot-path in either build, we don't really care:
default code stack ctx
lfs3_file_fetch before: 372 416 0
lfs3_file_fetch after: 368 (-1.1%) 440 (+5.8%) 0 (+0.0%)
We were relying on the previous LFS3_TSTATE_OMDIRS logic implicitly
leaving t->ot NULL when it reaches the end of the linked-list. With the
lfs3_m_isrdonly shortcut we now need to do this explicitly.
Found by test_mount_flags
Adds a bit of code to both the default and rdonly builds, but a correct
filesystem is usually preferred over a small one:
code stack ctx
rdonly before: 10676 840 524
rdonly after: 10680 (+0.0%) 840 (+0.0%) 524 (+0.0%)
default before: 37320 2280 636
default after: 37324 (+0.0%) 2280 (+0.0%) 636 (+0.0%)
This was missing from the big lfs -> lfs3 rename, probably because it
didn't actually break testing. It just prevents prettyasserts from
making LFS3_ASSERT pretty.
There's already been a bunch of benchmarking targeting the current hash,
and we're probably going to find other missed prefixes in corners of the
codebase anyways, so I'm not going to bother rebasing.
This partially reverts the LFS3_TSTATE_OMDIRS/OBTREE ifdefs, instead
adopting lfs3_m_isrdonly checks that let the compiler prune the
unreachable code paths when compiling with LFS3_RDONLY.
This adds a bit of code to both the default and rdonly builds (the
compiler isn't perfect, but simplifies the codebase:
code stack ctx
rdonly before: 10664 840 524
rdonly after: 10676 (+0.1%) 840 (+0.0%) 524 (+0.0%)
default before: 37300 2280 636
default after: 37320 (+0.1%) 2280 (+0.0%) 636 (+0.0%)
Testing the rdonly build is difficult, so minimizing the differences in
the code is quite valuable for maintenance and reliability.
As a plus, the extra ~20 bytes of code in the default build lets us
avoid traversing the omdirs when mounted LFS3_M_RDONLY. This niche
performance optimization isn't really a goal, but it's nice for
LFS3_RDONLY and LFS3_M_RDONLY to match behavior when possible.
I did override lfs3_o_isrdonly, but missed lfs3_m_isrdonly and
lfs3_t_isrdonly.
These aren't strictly necessary (asserts force rdonly flags to be set
correctly), but can save code by trimming unreachable code paths.
That being said, currently no observable code savings:
code stack ctx
rdonly before: 10664 840 524
rdonly after: 10664 (+0.0%) 840 (+0.0%) 524 (+0.0%)
But I noticed while toying around with a different way of pruning
LFS3_TSTATE_OMDIRS/OBTREE and wanted to make sure other code savings
weren't dragged in.
If we can't write to the filesystem, we can't out out-of-sync files, so
there's no need to traverse open file handles at all.
Saves a bit of code in LFS3_RDONLY mode:
code stack ctx
rdonly before: 10776 840 524
rdonly after: 10664 (-1.0%) 840 (+0.0%) 524 (+0.0%)
In theory we could also skip this check when mounted LFS3_M_RDONLY, but
checking for that flag would add code and we don't really care about
CPU-related performance here.
No code changes in default mode.
This function is kinda ugly in that our failed label expects the
dirty/mutated flags to be swapped, but we only swap _after_ calling
lfs3_mtree_traverse to avoid messing up lfs3_mtree_traverse's eot logic.
Long story short, this goto failed after lfs3_mtree_traverse could end
up with drity/mutated in the wrong state.
Worst case, this can leave littlefs in a state where it thinks work was
accomplished, but only if lfs3_mtree_traverse encounters an exceptional
error (LFS3_ERR_IO? LFS3_ERR_CORRUPT?), which usually leads to emergency
actions anyways.
We probably need more testing around exceptional errors like these,
they're also the main limit to our line/branch coverage. But the work
will be tedious so for now that's a future thing.
I at least added a comment to hopefully prevent a similar regression.
Code changes minimal, humorously undoes the LFS3_RDONLY noise:
code stack ctx
before: 37304 2280 636
after: 37300 (-0.0%) 2280 (+0.0%) 636 (+0.0%)
This is the new readonly flag, to be consistent with LFS3_M_RDONLY and
friends.
Note this overlaps with LFS3_YES_RDONLY in a weird way, where
LFS3_YES_RDONLY is basically just an alias for LFS3_RDONLY. For most
flags, LFS3_THING enables the _option_ of using LFS3_M_THING, with
LFS3_YES_THING implying LFS3_M_THING in all mount calls. But
LFS3_RDONLY _disables_ the option of using LFS3_M_RDWR, so it's a bit
different...
Do we really need two flags for the same thing? Not sure. But most users
probably expect LFS3_RDONLY coming from other filesystems.
Worst case this can be revisited in the planned config API rework.
---
As for the readonly code size, this is just the first draft and limited
to mostly ifdefing out all prog/write logic paths. There's some TODOs in
the code that may save a bit more (rbyd.eoff, file.b.shrub_ for
example). But the results are looking ok:
code stack ctx
v2.11.0 rdonly: 6270 448 580
v3-alpha rdonly: 10776 (+71.9%) 840 (+87.5%) 524 (-9.7%)
It's interesting to note most of the additional code/stack cost come
from filesystem traversal. In v2, the threaded linked-list made rdonly
traversal _incredibly_ cheap. But the extra rdwr baggage of turning
littlefs into a fully connected graph made it something to be avoided
in v3.
This hits v3 with the double whammy of:
1. Filesystem traversal is more complicated since we need to keep track
of which btree and where in the btree we are
2. Everything needs to be tracked explicitly due to the new inverted
state-machine driven API (no callbacks)
Note that even if we disabled the traversal APIs, lfs3_fs_usage, cksum
checking, etc, we'd still need to traverse to rebuild gstate. Otherwise
we risk showing grmed files after a powerloss.
---
This did affect the default build a little bit, due to moving things
around for nicer ifdef groupings:
code stack ctx
default before: 37300 2280 636
default after: 37304 (+0.0%) 2280 (+0.0%) 636 (+0.0%)
This one was a bit more involved.
Removes utils that are no longer useful, and made sure some of the
name/API changes over time are adopted consistently:
- lfs_npw2 -> lfs_nlog2
- lfs_tole32_ -> lfs_tole32
- lfs_fromle32_ -> lfs_fromle32
Also did another pass for lfs_ prefixes on mem/str functions. The habit
to use the naked variants of these is hard to break!
Kinda. It's actually only 3 scripts. These have been replaced with the
new dbg*.py scripts:
- readblock.py -> dbgblock.py
- readmdir.py -> dbgrbyd.py
- readtree.py -> dbglfs.py
Not sure what the point of this was, I think it was copied from a d3
example svg at some point. But it forces the svg to always fit in the
window, even if this makes the svg unreadable.
These svgs tend to end up questionably large in order to fit in the most
info, so the unreadableness ends up a real problem for even modest
window sizes.
This adds lfsr_btree_commitroot_ and lfsr_bshrub_commitroot_, to contain
the root-specific commit logic such that it can be forced off the stack
hot-path if necessary.
---
Note we're not actually using LFS_NOINLINE yet, as the critical
function, lfsr_btree_commitroot_ is implicitly forced off the stack
hot-path via the multiple calls from lfsr_btree_commit and
lfsr_bshrub_commit.
And I'm not sure it makes sense to use LFS_NOINLINE here. It absolutely
wrecks lfsr_bshrub_commitroot_'s stack, which always ends up on the
stack hot-path because of the route through lfsr_mdir_commit.
Is this a big hack? Honestly yeah.
It doesn't even really save that much stack, but I figured it was worth
a try:
code stack ctx
before: 37260 2296 636
after: 37300 (+0.1%) 2280 (-0.7%) 636 (+0.0%)
At least the code organization is a bit better, with lfsr_bshrub_commit
reusing lfsr_btree_commitroot_ for bshrub -> btree migration.
This was a simple oversight, we weren't checking recipient file caches
when broadcasting sync!
Fixed by limiting the synced cache to the last n bytes that fit in the
recipient's cache. This is a bit more complicated than first n bytes,
but more intuitive/likely to be relevant to the recipient file.
Adds a bit of code/stack. In theory this shouldn't really affect the
stack, but lfsr_file_sync is a sensitive function on the stack hot-path:
code stack ctx
before: 37220 2288 636
after: 37260 (+0.1%) 2296 (+0.3%) 636 (+0.0%)
These mimic the relevant LFS_O_* flags, and allow users to assert
whether or not a traversal will mutate the filesystem:
LFS_T_MODE 0x00000001 The traversal's access mode
LFS_T_RDWR 0x00000000 Open traversal as read and write
LFS_T_RDONLY 0x00000001 Open traversal as read only
In theory, these could also change internal allocations, but littlefs
doesn't really work that way.
Note we _don't_ add related LFS_GC_RDONLY, LFS_GC_RDWR, etc flags. These
are sort of implied by the relevant LFS_M_* flags.
Adds a bit more code, probably because of the slightly more complicated
internal constants for the internal traversals. But I think the
self-documentingness is worth it:
code stack ctx
before: 37200 2288 636
after: 37220 (+0.1%) 2288 (+0.0%) 636 (+0.0%)
This time to account for the new LFS_o_UNCRYST and LFS_o_UNGRAFT flags.
This required moving the T flags out of the way, which of course
conflicted with TSTATE, so that had to move...
One thing that helped was shoving LFS_O_DESYNC up with the internal
state flags. It's definitely more a state flag than the other public
flags, it just also happens to be user toggleable.
Here's the new jenga:
8 8 8 8
.----++----++----++----.
.-..----..-..-..-------.
o_flags: |t|| f ||o||t|| o |
|-||-.--':-:|-|'--.-.--'
|-||-|.----.|-'--------.
t_flags: |t||f||tstt|| t |
'-''-''----'|----.-----'
.----..-.:-:|----|:-:.-.
m_flags: | m ||c||o|| t ||o||m|
|----||-|'-'|-.--''-''-'
|----||-|---|-|.-------.
f_flags: | m ||c| |t|| f |
'----''-'---'-''-------'
This adds a bit of code, but that's not the end of the world:
code stack ctx
before: 37172 2288 636
after: 37200 (+0.1%) 2288 (+0.0%) 636 (+0.0%)
- LFS_CKPARITY -> LFS_CKMETAPARITY
- LFS_CKDATACKSUMS -> LFS_CKDATACKSUMREADS
The goal here is to provide hints for 1. what is being checked (META,
DATA, etc), and 2. on what operation (FETCHES, PROGS, READS, etc).
Note that LFS_CKDATACKSUMREADS is intended to eventually be a part of a
set of flags that can pull off closed fully-checked reads:
- LFS_CKMETAREDUNDREADS - Check data checksums on reads
- LFS_CKDATACKSUMREADS - Check metadata redund blocks on reads
- LFS_CKREADS - LFS_CKMETAREDUNDREADS + LFS_CKDATACKSUMREADS
Also it's probably not a bad idea for LFS_CKMETAPARITY to be harder to
use. It's really not worth enabling unless you understand its
limitations (<1 bit of error detection, yay).
No code changes.
Not sure why, but this just seems more intuitive/correct. Maybe because
LFSR_TAG_NAME is always the first tag in a file's attr set:
LFSR_TAG_NAMELIMIT 0x0039 v--- ---- --11 1--1
LFSR_TAG_FILELIMIT 0x003a v--- ---- --11 1-1-
Seeing as several parts of the codebase still use the previous order,
it seems reasonable to switch back to that.
No code changes.
Originally adopted during the failed btree-leaf-cache, I just think this
is a bit more readable when mixed in with parent, sibling, etc.
Also a couple comment tweaks.
No code changes.
See the relevant commit for why. These just added surprisingly little
performance benefit for the code/stack cost.
Maybe in a future performance-preferring littlefs driver.
The idea here, is we give each lfsr_btree_t an optional leaf rbyd, in
addition to the root rbyd. This leaf rbyd acts as a cache for the most
recent leaf, allowing nearby btree lookups to skip the full btree walk.
Unfortunately, this failed on pretty much every measurable metric...
---
The motivation for this is that we often do a bunch of nearby btree
lookups:
- Btree iteration via lfsr_btree_lookupnext is a bit naive, walking from
the root every step.
- Our crystallization algorithm requires a bunch of nearby lookups to
figure out our crystallization heuristic. Currently at most 4, when
you need to lookup both crystal neighbors and then _also_ both
fragment neighbors for coalescing.
- Checksum collision resolution for dids and (FUTURE) ddkeys can require
an unbounded number of sequential lookups.
Though to be fair, this is an exceptional case if our checksum is any
good.
- Bids with multiple rattrs require nearby lookups to resolve.
Though currently this can be explicitly avoided via
lfsr_btree_lookupleaf + lfsr_rbyd_lookup.
The theory was that cases like these could explicitly keep track of the
leaf rbyd to avoid full btree walks, but in practice this never really
worked out. Tracking if we're still in the relevant leaf rbyd just adds
too much logic/code cost.
But if this leaf tracking logic was implemented once in the btree
layer...
The other theoretical benefit was being able to move more rbyds off the
stack. Sure our btrees take up more RAM, but if that results in stack
savings, that may be a win.
Oh, and this would let our btree API and rbyd API converge without
performance concerns. Internal users could in theory call
lfsr_btree_lookupnext + lfsr_btree_lookup with the same performance as
explicitly tracking the rbyd.
---
But this was a complete failure!
First the good news: There was a modest speedup of around ~2x to linear
reads.
And that's the good news.
Now the bad news:
1. There was no noticeable performance gain in any other benchmarks.
To be fair, we're at the early stages of benchmarking, so the
benchmarks may not be the most thorough, but thinking about it, there
are some explanations:
- In any benchmark that writes, fetch + erase + prog dominates. Being
able to skip fetches during lookups makes our btree lookups
surprisingly cheap!
- Any random read heavy benchmark is likely thrashing this cache,
which is to be expected.
- For small 1-block btrees, the leaf cache is useless because the
entire btree is cache in the root rbyd.
And keep in mind, our blocks are BIG. "Small" here could be on
the order of ~128KiB-1MiB for NAND flash.
- For the mtree, fetched mdirs actually already act as a sort of leaf
cache.
The extra btree leaf cache isn't doing _nothing_, but each layer of
the mtree has diminishing returns due to btree's ridiculous
branching factor.
- For file btrees, we're explicitly caching the leaf fragments/
blocks, so the extra btree leaf cache has diminishing returns for
the same reason.
2. Code cost was bad, stack cost was worse:
code stack ctx
before: 37172 2288 636
after: 38068 (+2.4%) 2416 (+5.6%) 664 (+4.4%)
Tracking the leaf required more code, that's expected. And, to be
fair, the current code has had a lot more time to congeal.
What wasn't expected was the stack cost.
Unfortunately these caches didn't really take any rbyds off the stack
hot-path:
- We _can_ get rid of the rbyd in lfsr_btree_lookup/namelookup, but
we were already hacking our way around the critical one in
lfsr_mtree_lookup/namelookup by reusing the mdir's rbyd!
- We can't even abuse the leaf rbyd in the commit logic, since the
target btree can end up iterated/traversed by lfs_alloc.
That was a fun bug.
And the addition of a second rbyd to lfsr_btree_t increases both ctx
and stack anywhere btrees are allocated.
Maybe this will make more sense when we add the auxiliary btrees, or
after more benchmarking, but for now the theoretical performance
improvements just aren't worth it.
Will probably revert this, but I wanted to commit it in case the idea is
worth resurrecting in the future, if in the future nearby btree lookups
are a bigger penalty than they are now.
Christopher Haster