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.
- lfsr_rbyd_init
- lfsr_rbyd_claim
- lfsr_btree_claim
- lfsr_mdir_claim
Saves a bit of code, but I think this is just because of a tweak to how
we check for shared btree erased-state that crept in (now only comparing
blocks instead of block + trunk):
code stack ctx
before: 37184 2288 636
after: 37172 (-0.0%) 2288 (+0.0%) 636 (+0.0%)
Not sure how the duplication here went unnoticed.
Note we can _not_ reuse block_start/end for the buffer updates, since
those depend on the crystallized/aligned result.
No code changes though. The good news is the compiler is doing a good
job with the dense math in these functions.
This commit actually does two things:
1. Opportunistically marks caches as flushed if they were included in
the crystallization region in lfsr_file_crystallize
2. Reroutes lfsr_file_read_ through lfsr_file_crystallize to minimize
stack cost
---
Digging into why lazy-crystallization adds so much stack, it seems the
main reason is because lfsr_file_read_ drags in lfsr_file_flush, which
puts the entirety of the stack hot-path under both lfsr_file_read and
lfsr_file_read_.
But why are we calling lfsr_file_flush? And not just
lfsr_file_crystallize to claim the leaf? Isn't the cache flushed in
lfsr_file_read before reading?
The one concerning case is when reads bypass the cache (read >
cache_size). With cache-bypassing reads, it's entirely possible for
lfsr_file_read_ to end up with unflushed data. lfsr_file_read's logic
gives the cache priority in this case, so it's not like we're going to
read outdated data or anything, but if we crystallize without flushing
we risk wasting erased-state that will need to be recrystallized later.
What's extra humorous is our crystallization logic _does_ correctly
write out the cache, it just doesn't clear the LFS_o_UNFLUSH bit because
it doesn't know if progress has been made.
So to avoid this, all we need to do is add an explicit check to
lfsr_file_crystallize that clears the LFS_o_UNFLUSH bit if our cache
ends up written out as a part of crystallization.
Note this is slightly more powerful than lfsr_file_flush, since we don't
_need_ to flush the cache if it's not in our crystallization region.
As an extra plus this affects all lfsr_file_crystallize calls, so now
lfsr_file_truncate/fruncate also avoid unnecessary recrystallization.
That's some good code reuse right there!
---
Long story short, rerouting lfsr_file_read_ through
lfsr_file_crystallize moves it off the stack hot-path, bringing our
stack down to almost pre-lazy-crystallization levels:
code stack ctx
before: 37140 2304 636
after: 37184 (+0.1%) 2288 (-0.7%) 636 (+0.0%)
At a code cost, but this code also allows lfsr_file_read/truncate/
fruncate to avoid recrystallization with opportunistic flushes in cases
where we need to discard file->leaf.
Gah! I'm not sure why I thought this code was so useless... Without it
we risk immediate recrystallization if the crystal heuristic pushes
crystal_start such that it overlaps the crystallizing block.
We may not make progress on our buffer, but triggering recrystallization
isn't great.
Considering this really doesn't add _that_ much code, I think this is a
case where we are better safe than sorry:
code stack ctx
before: 37092 2304 636
after: 37140 (+0.1%) 2304 (+0.0%) 636 (+0.0%)
This logic only gets hit after we decide to allocate a new block, so
there's no risk of losing erased-state to potential fragments.
---
In benchmarking it also looks like this recoups most of the extra disk
usage introduced by lazy rewrite crystallization. So that's a good
thing... I think...
Still on the fence about this, but in hindsight the code/stack
difference is not _that_ much:
code stack ctx
before: 36460 2280 636
after: 37092 (+1.7%) 2304 (+1.1%) 636 (+0.0%)
Especially with the potential to significantly speed up linear file
writes/rewrites, which are usually the most common file operation. You
ever just, you know, write a whole file at once?
Note we can still add the previous behavior as an opt-in write strategy
to save code/stack when preferred over linear write/rewrite speed.
This is actually the main reason I think we should prefer
lazy-crystallization by default. Of the theoretical/future write
strategies, lazy-crystallization was the only one trading performance
for code/stack and not vice versa (global-alignment, linear-only,
fully-fragmented, etc).
If we default to a small, but less performant filesystem, it risks users
thinking littlefs is slow when they just haven't turned on the right
flags.
That being said there's a balance here. Users will probably judge
littlefs based on its default code size for the same reason.
---
Note this includes the generalized lfsr_file_crystallize_ API, which
adds a bit of code:
code stack ctx
before gen-cryst: 37084 2304 636
after gen-cryst: 37092 (+0.0%) 2304 (+0.0%) 636 (+0.0%)
This function is actually pretty much the same in both the lazy and
eager crystallization write strategies. The main difference being the
nuances around the crystal_size parameter:
- lazy: crystal_size => rough upper bound on crystal
- eager: crystal_size => strict lower bound on crystal
If we change these to an explicit crystal_min and crystal_max, we can
use lfsr_file_crystallize_ in both write strategies without changing the
logic.
It's out of scope right now, but this will help supporting both write
strategies in the future.
---
Unfortunately this added more code/stack that I was expecting:
code stack ctx
before: 36428 2248 636
after: 36460 (+0.1%) 2280 (+1.4%) 636 (+0.0%)
I'm not exactly sure why, I guess the crystal_limit calculation is too
complex to const propagate the crystal_max=-1?
Maybe the LFS_NOINLINE is disabling certain cross-function
optimizations...
This reverts most of the lazy-grafting/crystallization logic, but keeps
the general crystallization algorithm rewrite and file->leaf for caching
read operations and erased-state.
Unfortunately lazy-grafting/crystallization is both a code and stack
heavy feature for a relatively specific write pattern. It doesn't even
help if we're forced to write fragments due to prog alignment.
Dropping lazy-grafting/crystallization trades off linear write/rewrite
performance for code and stack savings:
code stack ctx
before: 37084 2304 636
after: 36428 (-1.8%) 2248 (-2.4%) 636 (+0.0%)
But with file->leaf we still keep the improvements to linear read
performance!
Compared to pre-file->leaf:
code stack ctx
before file->leaf: 36016 2296 636
after lazy file->leaf: 37084 (+3.0%) 2304 (+0.3%) 636 (+0.0%)
after eager file->leaf: 36428 (+1.1%) 2248 (-2.1%) 636 (+0.0%)
I'm still on the fence about this, but lazy-grafting/crystallization is
just a lot of code... And the first 6 letters of littlefs don't spell
"speedy" last time I checked...
At the very least we can always add lazy-grafting/crystallization as an
opt-in write strategy later.
This adopts lazy crystallization in _addition_ to lazy grafting, managed
by separate LFS_o_UNCRYST and LFS_o_UNGRAFT flags:
LFS_o_UNCRYST 0x00400000 File's leaf not fully crystallized
LFS_o_UNGRAFT 0x00800000 File's leaf does not match bshrub/btree
This lets us graft not-fully-crystallized blocks into the tree without
needing to fully crystallize, avoiding repeated recrystallizations when
linearly rewriting a file.
Long story short, this gives file rewrites roughly the same performance
as linear file writes.
---
In theory you could also have fully crystallized but ungrafted blocks
(UNGRAFT + ~UNCRYST), but this doesn't happen with the current logic.
lfsr_file_crystallize eagerly grafts blocks once they're crystallized.
Internally, lfsr_file_crystallize replaces lfsr_file_graft for the
"don't care, gimme file->leaf" operation. This is analogous to
lfsr_file_flush for file->cache.
Note we do _not_ use LFS_o_UNCRYST to track erased-state! If we did,
erased-state wouldn't survive lfsr_file_flush!
---
Of course, this adds even more code. Fortunately not _that_ much
considering how many lines of code changed:
code stack ctx
before: 37012 2304 636
after 37084 (+0.2%) 2304 (+0.0%) 636 (+0.0%)
There is another downside however, and that's that our benchmarked disk
usage is slightly worse during random writes.
I haven't fully investigated this, but I think it's due to more
temporary fragments/blocks in the B-tree before flushing. This can cause
B-tree inner nodes to split earlier than when eagerly recrystallizing.
This also leads to higher disk usage pre-flush since we keep both the
old and new blocks around while uncrystallized, but since most rewrites
are probably going to be CoW on top of committed files, I don't think
this will be a big deal.
Note the disk usage ends up the same after lfsr_file_flush.
This should better match other relocation loops in the codebase, and is
hopefully a bit more readable.
---
Note we generally have two patterns for relocation loops:
Loops where we unconditionally allocate/relocate:
relocate:;
alloc();
compact();
if (err) goto relocate;
commit();
if (err) goto relocate;
return;
And loops where we fallback to allocation/relocation:
while (true) {
commit();
if (err) goto relocate;
return;
relocate:;
alloc();
compact();
if (err) goto relocate;
}
lfsr_mdir_commit_ falls into the latter.
No code changes.
This tweaks lfsr_mdir_commit_ to avoid overrecycling if we encounter a
bad prog (LFS_ERR_CORRUPT). This avoids compacting to the same block
twice, which risks an undetected prog error and breaks internal
invariants.
Note we still overrecycle if the relocation reason is a recycle
overflow.
---
This is an alternative solution to the previous overrecycling + shrub +
ckprog bug: Just make sure we don't compact to the same block twice!
After all, if we just got a bad prog, why are we trying to prog again?
(There are actually some arguments for multiple prog attempts, bus
errors for example, but I don't think that's a great excuse for littlefs
attempting multiple progs without user input.)
Even though this adds logic to lfsr_mdir_commit_, it ends up saving
code since we can drop the shrub discard pass:
code stack ctx
before: 37088 2304 636
after: 37056 (-0.1%) 2304 (+0.0%) 636 (+0.0%)
Not that we _really_ care about this quantity of code. The real
motivation is 1. lowering the risk of a missed prog error, and
2. maintaining the never-compact-same-block invariant in case there
are other invariant-dependent bugs lurking around.
In lfsr_mdir_compact__, we rely on shrub_.block != mdir.block to avoid
compacting shrubs multiple times. This works for the most part because
we set shrub_.block = shrub.block (the old mdir block) at the beginning
of lfsr_mdir_commit. We don't actually reset shrub_.block on a bad prog,
but in theory that was ok because we never try to compact into the same
block twice.
But this falls apart if we overrecycle the mdir!
With overrecycling, if we encounter a bad prog during a compaction and
there are no more blocks to relocate to, we try one last time to compact
into the same block (this logic is mainly for recycle overflows, where
it makes a bit more sense).
Of course, compacting into the same block breaks the above shrub_.block
!= mdir.block invariant, which causes the shrub compaction to be
skipped, uses the old shrub_.trunk (which now points to garbage), and
breaks everything.
Fortunately the solution is relatively simple: Just discard any staged
shrubs that have been committed when we relocate/overrecycle.
---
While fixing this I went ahead and renamed overcompaction ->
overrecycling. To me, overcompaction implies something _very_ different,
and I think this better describes the relationship between overrecycling
and block_recycles.
Also added test_ck_ckprogs_overrecycling to nail this down and prevent a
regression in the future. This bug _was_ caught by
test_ck_spam_fwrite_fuzz, but only after unrelated fs changes.
Adds a bit of code, but a smaller + dysfunctional filesystem is not very
useful:
code stack ctx
before: 37056 2304 (+0.0%) 636 (+0.0%)
after: 37088 (+0.1%) 2304 (+0.0%) 636 (+0.0%)
With the new crystallization logic, we have two routes for resuming
crystallization:
1. before finding our crystal heuristic, if buffer is in-block and
enough for prog alignment
2. after finding our crystal heuristic, if crystal heuristic is in-block
and enough for prog alignment
But thinking about the second case, when would this happen that isn't
caught by the first case? When there are fragments trailing our buffer?
Are you writing to the file backwards?
This corner case doesn't seem worth the extra logic.
Benchmarking didn't find a noticeable difference in performance, so
removing.
Saves a bit of code:
code stack ctx
before: 37080 2304 636
after: 37056 (-0.1%) 2304 (+0.0%) 636 (+0.0%)
This sort of abuses the bptr/data type overlap again, taking an explicit
delta along with a list of datas where:
- data_count=-1 => single bptr
- data_count>=0 => list of concatenated fragments
It's a bit of a hack, but the previous rattr argument it replaces was
an arguably worse hack. I figured if we're going to interrogate the
rattr to figure out what type it is, we might as well just make the type
explicit.
Saved a surprising amount of stack! So that's nice:
code stack ctx
before: 37192 2360 636
after: 37080 (-0.3%) 2304 (-2.4%) 636 (+0.0%)
Except for the unknown flag checks. I don't know why but they really
mess with readability there for me. Maybe because the logic matches
english grammar ("is not any of these" vs "is any not of these")?
No code changes.
This is just a bit simpler/more flexible of an API. Taking flags
directly has worked well for similar functions.
This also drops lfsr_*_mkdirty. I think we should keep the mk* names
reserved for heavy-weight filesystem operations.
That being said, this does add a surprising bit of code. I because the
flags end up in literal pools? Doesn't thumb have a bunch of fancy
single-bit immediate encodings?
code stack ctx
before: 37180 2360 636
after: 37192 (+0.0%) 2360 (+0.0%) 636 (+0.0%)
Mostly adding convenience functions to deduplicate code:
- Adopted lfsr_bptr_claim
- Renamed lfsr_file_graft -> lfsr_file_graft_
- Adopted lfsr_file_graft
- Didn't bother with lfsr_file_discardleaf
This saves a bit of code, though not that much in the context of the
file->leaf code cost:
code stack ctx
before cleanup: 37228 2328 636
after: 37180 (-0.1%) 2360 (+1.4%) 636 (+0.0%)
code stack ctx
before file->leaf: 36016 2296 636
after: 37180 (+3.2%) 2360 (+2.8%) 636 (+0.0%)
TLDR: Added file->leaf, which can track file fragments (read only) and
blocks independently from file->b.shrub. This speeds up linear
read/write performance at a heavy code/stack cost.
The jury is still out on if this ends up reverted.
---
This is another change motivated by benchmarking, specifically the
significant regression in linear reads.
The problem is that CTZ skip-lists are actually _really_ good at
appending blocks! (but only appending blocks) The entire state of the
file is contained in the last block, so file writes can resume without
any reads. With B-trees, we need at least 1 B-tree lookup to resume
appending, and this really adds up when writing extremely blocks.
To try to mitigate this, I added file->leaf, a single in-RAM bptr for
tracking the most recent leaf we've operated on. This avoids B-tree
lookups during linear reads, and allowing the leaf to fall out-of-sync
with the B-tree avoids both B-tree lookups and commits during writes.
Unfortunately this isn't a complete win for writes. If we write
fragments, i.e. cache_size < prog_size, we still need to incrementally
commit to the B-tree. Fragments are a bit annoying for caching as any
B-tree commit can discard the block they reside on.
For reading, however, this brings read performance back to roughly the
same as CTZ skip-lists.
---
This also turned into more-or-less a full rewrite of the lfsr_file_flush
-> lfsr_file_crystallize code path, which is probably a good thing. This
code needed some TLC.
file->leaf also replaces the previous eblock/eoff mechanism for
erased-state tracking via the new LFSR_BPTR_ISERASED flag. This should
be useful when exploring more erased-state tracking mechanisms (ddtree).
Unfortunately, all of this additional in-RAM state is very costly. I
think there's some cleanup that can be done (the current impl is a bit
of a mess/proof-of-concept), but this does add a significant chunk of
both code and stack:
code stack ctx
before: 36016 2296 636
after: 37228 (+3.4%) 2328 (+1.4%) 636 (+0.0%)
file->leaf also increases the size of lfsr_file_t, but this doesn't show
up in ctx because struct lfs_info dominates:
lfsr_file_t before: 116
lfsr_file_t after: 136 (+17.2%)
Hm... Maybe ctx measurements should use a lower LFS_NAME_MAX?
Maybe it's just habit, but the trailing underscores_ felt far more
useful serving only as a out-pointer/new/biproduct hint. Having trailing
underscores_ serve dual purposes as both a new/biproduct hint and
optional hint just muddies things and makes the hint much less useful.
No code changes.
This adds LFS_NOINLINE, and forces lfsr_file_sync_ (the commit logic in
lfsr_file_sync) off the stack hot-path.
This adds a bit of code, function calls are surprisingly expensive, but
saves a nice big chunk of stack:
code stack ctx
before: 35992 2408 636
after: 36016 (+0.1%) 2296 (-4.7%) 636 (+0.0%)
Well, maybe not _real_ stack. The fact that this worked suggests the
real stack usage is less than our measured value.
The reason is because our stack.py script is relatively simple. It just
adds together stack frames based on the callgraph at compile time, which
misses shrinkwrapping and similar optimizations. Unfortunately that sort
of information is simply not available via GCC short of parsing the
disassembly.
But this is the number that will be used for statically allocated stacks,
and of course the number that will probably end up associated with
littlefs, so it still seems like a worthwhile number to "optimize" for.
Maybe in the future this will be different as tooling around stack
measurements improves.
---
The other benefit of moving lfsr_file_sync_ off the hot-path is that we
now no longer incorrectly include the sync commit context in the
hot-path. This tells a much different story for the cost of 1-commit
shrubs:
code stack ctx
before 1c-shrubs: 35848 2296 636
after 1c-shrubs: 36016 (+0.5%) 2296 (+0.0%) 636 (+0.0%)
This adds an alternative sync path for small in-cache files, where we
combine the shrub commit with the file sync commit, potentially writing
everything out in a single prog.
This is reminiscent of bmoss (old inlined) files, but notably avoids the
additional on-disk data-structure and extra code necessary to manage it.
---
The motivation for this comes from ongoing benchmarking, where we're
seeing a fairly significant regression in small-file performance on NAND
flash. Especially curious since the whole goal of this work was to make
NAND flash tractable.
But it makes sense: 2 commits are more than 1.
While the separate shrub + sync commits are barely noticeable on NOR
flash, on NAND flash, with its huge >512B prog sizes, the extra commit
is hard to miss.
In theory, the most performant solution would be to merge all bshrub
commits with sync commits whenever possible. This is technically doable,
and may make sense for a more performance-focused littlefs driver, but
it would 1. require an invasive code rewrite, 2. entangle lfsr_file_sync
-> lfsr_file_flush -> lfsr_file_carve, and 3. add even more code.
If we only merge shrub + sync commits when the file fits in the cache,
we can skip lfsr_file_flush, craft a simple shrubcommit by hand, and
avoid all of this mess. While still speeding up the most common write
path for small files.
And sure enough, our bench-many benchmark, which creates ~1000 4 byte
files, shows a ~2x speed improvement on bs=128KiB NAND (basically just
because we compact/split ~5 times instead of ~10 times).
---
Unfortunately the shrub commit requires quite a bit of state to set up,
and in the middle of lfsr_file_sync, one of the more critical functions
on our stack hot-path. So this does have a big cost:
code stack ctx
before: 35836 2368 636
after: 35992 (+0.4%) 2408 (+1.7%) 636 (+0.0%)
Though this is also a perfect contender to be compile-time ifdefed. It
may be worth adding something like LFS_NO_MERGESHRUBCOMMITS (better
name?) to claw back some of the cost if you don't care about
performances as much.
This could also probably be a bit cheaper if our file write configs were
organized differently... At the moment we need to check inline_size,
fragment_size, _and_ crystal_thresh since these can sometimes overlap.
But this is waiting on the future config rework.
---
Actually... Looking at this closer, I'm not sure the added commit logic
should really be included in the hot-path cost...
lfsr_file_flush is the hot path, and flush -> sync are sequential
operations that don't really share stack (with the shrub commit we
humorously _never_ call flush). The commit logic is only being dragged
in because our stack measurements are pessimistic about shrinkwrapping,
which is a bit frustrating.
I've explored shrinkwrapping in stack.py before, but the idea pretty
much failed. Unfortunately GCC simply doesn't make this info available
short of parsing the per-arch disassembly.
- codemapd3.py -> codemapsvg.py
- dbgbmapd3.py -> dbgbmapsvg.py
- treemapd3.py -> treemapsvg.py
Originally these were named this way to match plotmpl.py, but these
names were misleading. These scripts don't actually use the d3 library,
they're just piles of Python, SVG, and Javascript, modelled after the
excellent d3 treemap examples.
Keeping the *d3.py names around also felt a bit unfair to brendangregg's
flamegraph SVGs, which were the inspiration for the interactive
component. With d3 you would normally expect a rich HTML page, which is
how you even include the d3 library.
plotmpl.py is also an outlier in that it supports both .svg and .png
output. So having a different naming convention in this case makes
sense to me.
So, renaming *d3.py -> *svg.py. The inspiration from d3 is still
mentioned in the top-level comments in the relevant files.
This adds --xlim-stddev and --ylim-stddev as alternatives to -X/--xlim
and -Y/--ylim that define the plot limits in terms of standard
deviations from the mean, instead of in absolute values.
So want to only plot data within +-1 standard deviation? Use:
$ ./scripts/plot.py --ylim-stddev=-1,+1
Want to ignore outliers >3 standard deviations? Use:
$ ./scripts/plot.py --ylim-stddev=3
This is very useful for plotting the amortized/per-byte benchmarks,
which have a tendency to run off towards infinity near zero.
Before, we could truncate data explicitly with -Y/--ylim, but this was
getting very tedious and doesn't work well when you don't know what the
data is going to look like beforehand.
So:
$(filter-out %.t.c %.b.c %.a.c,$(wildcard bd/*.c))
Instead of:
$(filter-out $(wildcard bd/*.t.* bd/*.b.*),$(wildcard bd/*.c))
The main benefit is we no longer need to explicitly specify all
subdirectories, though the single wildcard is a bit less flexible if
test.py/bench.py ever end up with other non-C artifacts.
Unfortunately only a single wildcard is supported in filter-out.
This adds mattr_estimate, which is basically the same as rattr_estimate,
but assumes weight <= 1:
rattr tag:
.---+---+---+- -+- -+- -+- -+---+- -+- -+- -. worst case: <=11 bytes
| tag | weight | size | rattr est: <=3t + 4
'---+---+---+- -+- -+- -+- -+---+- -+- -+- -' <=37 bytes
mattr tag:
.---+---+---+---+- -+- -+- -. worst case: <=7 bytes
| tag | w | size | mattr est: <=3t + 4
'---+---+---+---+- -+- -+- -' <=25 bytes
This may seem like only a minor improvement, but with 3 tags for every
attr, this really adds up. And with our compaction estimate overheads we
need every byte of shaving we can get.
---
This ended up necessary to get littlefs running with 512 byte blocks
again. Now that our compaction overheads are so high, littlefs is having
a hard time fitting even just the filesystem config in a single block:
mroot estimate 512B before: 246/256
mroot estimate 512B after: 162/256 (-34.1%)
Whether or not it makes sense to run littlefs with 512 byte blocks is
still an open question, even after this tweak.
Note that even if 512 byte blocks ends up intractable, this doesn't mean
littlefs won't be able to run on SD/eMMC! The configured block_size can
always be a multiple, >=, of the physical block_size, and choosing a
larger block_size completely side-steps this problem.
The new design of littlefs is primarily focused on devices with very
large block sizes, so you may want to use larger block sizes on SD/eMMC
for performance reasons anyways.
---
Code changes were pretty minimal. This does add an additional field to
lfs_t, but it's just a byte and fits into padding with the other small
precomputed constants:
code stack ctx
before: 35824 2368 636
after: 35836 (+0.0%) 2368 (+0.0%) 636 (+0.0%)
This prevents runaway O(n^2) behavior on devices with extremely large
block sizes (NAND, bs=~128KiB - ~1MiB).
The whole point of shrubs is to avoid this O(n^2) runaway when inline
files become necessarily large. Setting FRAGMENT_SIZE to a factor of the
BLOCK_SIZE humorously defeats this.
The 512 byte cutoff is somewhat arbitrary, it's the natural BLOCK_SIZE/8
FRAGMENT_SIZE on most NOR flash (bs=4096), but it's probably worth
tuning based on actual device performance.
Whoops, looks like cumulative results were overlooked when multiple
bench measurements per bench were added. We were just adding all
cumulative results together!
This led to some very confusing bench results.
The solution here is to keep track of per-measurement cumulative results
via a Python dict. Which adds some memory usage, but definitely not
enough to be noticeable in the context of the bench-runner.
This should be floor (rounds towards -inf), not int (rounds towards
zero), otherwise sub-integer results get funky:
- floor si(0.00001) => 10u
- int si(0.00001) => 0.01m
- floor si(0.000001) => 1u
- int si(0.000001) => m (???)
This was a simple typo. Unfortunately went unnoticed because the
lingering dataset assigned in the above for loop made the results look
mostly correct. Yay.
Before this, the only option for ordering the legend was by specifying
explicit -L/--add-label labels. This works for the most part, but
doesn't cover the case where you don't know the parameterization of the
input data.
And we already have -s/-S flags in other csv scripts, so it makes sense
to adopt them in plot.py/plotmpl.py to allow sorting by one or more
explicit fields.
Note that -s/-S can be combined with explicit -L/--add-labels to order
datasets with the same sort field:
$ ./scripts/plot.py bench.csv \
-bBLOCK_SIZE \
-xn \
-ybench_readed \
-ybench_proged \
-ybench_erased \
--legend \
-sBLOCK_SIZE \
-L'*,bench_readed=bs=%(BLOCK_SIZE)s' \
-L'*,bench_proged=' \
-L'*,bench_erased='
---
Unfortunately this conflicted with -s/--sleep, which is a common flag in
the ascii-art scripts. This was bound to conflict with -s/--sort
eventually, so a came up with some alternatives:
- -s/--sleep -> -~/--sleep
- -S/--coalesce -> -+/--coalesce
But I'll admit I'm not the happiest about these...
Whoops! A missing splat repetition here meant we only ever accepted
floats with a single digit of precision and no e/E exponents.
Humorously this went unnoticed because our scripts were only
_outputting_ single digit floats, but now that that's fixed, float
parsing also needs a fix.
Fixed by allowing >1 digit of precision in our CsvFloat regex.
This adds __csv__ methods to all Csv* classes to indicate how to write
csv/json output, and adopts Python's default float repr. As a plus, this
also lets us use "inf" for infinity in csv/json files, avoiding
potential unicode issues.
Before this we were reusing __str__ for both table rendering and
csv/json writing, which rounded to a single decimal digit! This made
float output pretty much useless outside of trivial cases.
---
Note Python apparently does some of its own rounding (1/10 -> 0.1?), so
the result may still not be round-trippable, but this is probably fine
for our somewhat hack-infested csv scripts.
So now the hidden variants of field specifiers can be used to manipulate
by fields and field fields without implying a complete field set:
$ ./scripts/csv.py lfs.code.csv \
-Bsubsystem=lfsr_file -Dfunction='lfsr_file_*' \
-fcode_size
Is the same as:
$ ./scripts/csv.py lfs.code.csv \
-bfile -bsubsystem=lfsr_file -Dfunction='lfsr_file_*' \
-fcode_size
Attempting to use -b/--by here would delete/merge the file field, as
cvs.py assumes -b/-f specify all of the relevant field type.
Note that fields can also be explicitly deleted with -D/--define's new
glob support:
$ ./scripts/csv.py lfs.code.csv -Dfile='*' -fcode_size
---
This solves an annoying problem specific to csv.py, where manipulating
by fields and field fields would often force you to specify all relevant
-b/-f fields. With how benchmarks are parameterized, this list ends up
_looong_.
It's a bit of a hack/abuse of the hidden flags, but the alternative
would be field globbing, which 1. would be a real pain-in-the-ass to
implement, and 2. affect almost all of the scripts. Reusing the hidden
flags for this keeps the complexity limited to csv.py.
Globs in CLI attrs (-L'*=bs=%(bs)s' for example), have been remarkably
useful. It makes sense to extend this to the other flags that match
against CSV fields, though this does add complexity to a large number of
smaller scripts.
- -D/--define can now use globs when filtering:
$ ./scripts/code.py lfs.o -Dfunction='lfsr_file_*'
-D/--define already accepted a comma-separated list of options, so
extending this to globs makes sense.
Note this differs from test.py/bench.py's -D/--define. Globbing in
test.py/bench.py wouldn't really work since -D/--define is generative,
not matching. But there's already other differences such as integer
parsing, range, etc. It's not worth making these perfectly consistent
as they are really two different tools that just happen to look the
same.
- -c/--compare now matches with globs when finding the compare entry:
$ ./scripts/code.py lfs.o -c'lfs*_file_sync'
This is quite a bit less useful that -D/--define, but makes sense for
consistency.
Note -c/--compare just chooses the first match. It doesn't really make
sense to compare against multiple entries.
This raised the question of globs in the field specifiers themselves
(-f'bench_*' for example), but I'm rejecting this for now as I need to
draw the complexity/scope _somewhere_, and I'm worried it's already way
over on the too-complex side.
So, for now, field names must always be specified explicitly. Globbing
field names would add too much complexity. Especially considering how
many flags accept field names in these scripts.
I don't know how I completely missed that this doesn't actually work!
Using del _does_ work in Python's repl, but it makes sense the repl may
differ from actual function execution in this case.
The problem is Python still thinks the relevant builtin is a local
variables after deletion, raising an UnboundLocalError instead of
performing a global lookup. In theory this would work if the variable
could be made global, but since global/nonlocal statements are lifted,
Python complains with "SyntaxError: name 'list' is parameter and
global".
And that's A-Ok! Intentionally shadowing language builtins already puts
this code deep into ugly hacks territory.
This was broken:
$ ./scripts/plotmpl.py -L'*=bs=%(bs)s'
There may be a better way to organize this logic, but spamming if
statements works well enough.
Christopher Haster