So now crystal_thresh only controls when fragments are compacted into
blocks, while fragment_thresh controls when blocks are broken into
fragments. Setting fragment_thresh=-1 will follow crystal_thresh and
keeps the previous behavior.
These were already two separate pieces of logic, so it makes sense to
provide two separate knobs for tuning.
Setting fragment_thresh lower than crystal_thresh has some potential to
reduce hysteresis in cases where random writes push blocks close to
crystal_thresh. It will be interesting to explore this more when
benchmarking.
---
The additional config option adds a bit of code/ctx, but hopefully that
will go away in the future config rework:
code stack ctx
before: 35584 2480 636
after: 35600 (+0.0%) 2480 (+0.0%) 640 (+0.6%)
And the related config options:
- cfg->file_buffer_size -> cfg->file_cache_size
- file->cfg->buffer_size -> file->cfg->cache_size
- file->cfg->buffer -> file->cfg->cache_buffer
The original motivation to rename this to file->buffer was to better
align with what other filesystems call this, but I think this is a case
where internal consistency is more important than external consistency.
file->cache better matches lfs->pcache and lfs->rcache, and makes it
easier to read code involving both file->cache and other user-provided
buffers.
Keeping the upstream name also helps with continuity.
While I think shrub_size is probably the more correct name at a
technical level, inline_size is probably more what users expect and
doesn't require a deeper understanding of filesystem details.
The only risk is that users may think inline_size has no effect on large
files, when in fact it still controls how much of the btree root can be
inlined.
There's also the point that sticking with inline_size maintains
compatibility with both the upstream version and any future version that
has other file representations.
May revisit this, but renaming to lfs->cfg->inline_size for now.
Now that we no longer have bmoss files, inline_size and shrub_size are
effectively the same thing.
We weren't using this, so no code change, but it does save a word of
ctx:
code stack ctx
before: 36280 2576 640
after: 36280 (+0.0%) 2576 (+0.0%) 636 (-0.6%)
Bptrs really are a file concept, despite the name (bptr =>
block-pointer). Other bshrubs/btrees do not have bptrs.
Returning decoded bptrs from lfsr_bshrub_lookupnext and friends was a
bit of a hack to make bsprouts (single bptrs) work, but now that we
don't support bsprouts, we don't need this hack anymore.
To avoid code duplication, this does reroute mtree traversal through
lfsr_file_traverse_. Which is a bit weird, but not the worst thing this
codebase has ever done.
Code changes:
code stack ctx
before: 36492 2608 640
after: 36460 (-0.1%) 2608 (+0.0%) 640 (+0.0%)
This moves all of the shrub tracking logic from lfsr_obshrub_t into
lfsr_bshrub_t, completely drops the lfsr_obshrub_t type, and changes all
lfsr_bshrub_* functions to take lfsr_bshrub_t instead of the mdir+shrub
pair.
This makes the lfsr_bshrub_* functions <-> lfsr_bshrub_t relationship
more consistent with other APIs, such as lfsr_btree_t:
- lfsr_bshrub_lookupnext(lfs, &file->o.o.mdir, &file->o.bshrub, ...)
+ lfsr_bshrub_lookupnext(lfs, &file->b, ...)
I think the reason why this design wasn't obvious before is because, at
least conceptually, having the lfsr_mdir_t live inside the lfsr_bshrub_t
is a bit weird. It's only thanks to lfsr_file_t invasively using the
internal lfsr_mdir_t that we can avoid duplicate lfsr_mdir_t objects.
This also reorganizes the structs in lfs.h a bit, and renames the
related file.o -> file.b fields (much needed because lfs->gc.t.o.o.mdir.
rbyd.blocks was starting to get _real_ confusing).
---
Unfortunately, reducing the number of arguments to lfsr_bshrub_*
functions did not save nearly as much code as I thought it would. It
even ended up with a net _increase_ of code, apparently due to needing
to recalculate the bshrub->shrub offset more often:
code stack ctx
before: 36476 2608 640
after: 36484 (+0.0%) 2608 (+0.0%) 640 (+0.0%)
Strange, but this rework is still worthwhile if only for the code
readability.
While they are a bit more annoying to call, init functions give the
compiler a chance to deduplicate common struct initialization logic. So
we should probably prefer init functions for any structs larger than a
couple words.
The cost of each init is small, but it really adds up!
code stack ctx
before: 38036 2608 752
after: 37844 (-0.5%) 2608 (+0.0%) 752 (+0.0%)
I think the assumption was that since these errors are trivially noops,
they shouldn't change any file state. But this doesn't match the
behavior of other errors, which is inconsistent and probably not what
users expect.
Also added a couple tests around FBIG that should catch this in the
future.
Curiously this actually saved a word of code, I guess because of
rerouting all errors through the same function epilogues:
code stack
before: 36416 2616
after: 36412 (-0.0%) 2616 (+0.0%)
This is mainly to solve the weird check-hole where passing CKPROGS/
CKREADS as mount flags has no effect on lfsr_format (I mean, it'd be a
bit silly if it did somehow):
LFS_F_RDWR 0 // Format the filesystem as read and write
LFS_F_CKPROGS 0x00000010 // Check progs by reading back progged data
LFS_F_CKREADS 0x00000020 // Check reads via parity bits/checksums
This makes lfsr_format a more cumbersome interface, but I don't know if
this is necessarily a bad thing. There's always risk of data loss when
calling lfsr_format, so maybe it should be a pain to call.
At the very least, format flags may be useful in the future for
enabling/disabling format-time things such as the planned block-map,
parity-tree, etc. Though it's unclear if such significant settings
should be format flags or somehow encoded as fields in our config
struct.
---
The LFS_F_* format flags of course ended up conflicting with our
internal LFS_F_* flags, so I renamed most of the internal flags to match
the closest flag set they participate in:
- LFS_F_TYPE -> LFS_O_TYPE
- LFS_F_UNFLUSH -> LFS_O_UNFLUSH
- LFS_F_UNSYNC -> LFS_O_UNSYNC
- LFS_F_ORPHAN -> LFS_O_ORPHAN
- LFS_F_ZOMBIE -> LFS_O_ZOMBIE
- LFS_F_ORPHANS -> LFS_I_ORPHANS
- LFS_F_UNCOMPACTED -> LFS_I_UNCOMPACTED
- LFS_F_TSTATE -> LFS_T_TSTATE
- LFS_F_BTYPE -> LFS_T_BTYPE
- LFS_F_DIRTY -> LFS_T_DIRTY
- LFS_F_MUTATED -> LFS_T_MUTATED
This may make it a bit less clear which flags are a part of the public
API, vs intended only for internal use, but at the very least our asserts
in format/mount/open/etc should catch most of these mistakes.
---
Code cost ended up being pretty minimal. Actually negative. This is the
second time we're _adding_ a feature that somehow saves code, though the
reality for this one is we're really just pushing constants up into the
user's stack frame. Still, it's a good indication the cost of format
flags is small:
code stack
before: 36452 2680
after: 36448 (-0.0%) 2680 (+0.0%)
This was only noticed when forcing btrees for other unrelated tests
(INLINED_SIZE=0, CRYSTAL_THRESH=-1), where even simple file writes would
end up with some unaligned fragments the size of our file buffer.
It was hard to notice without forcing btrees, since our crystallization
algorithm has a tendency to fix alignment issues.
The problem was that we weren't bypassing the file buffer correctly when
buffer.size == 0. We relied on the LFS_F_UNFLUSH flag to know if we
could do a bypassing write, but inlined files set the LFS_F_UNFLUSH flag
even for empty files. This led to blocked bypassing writes, attempts
to merge with empty buffers, and unaligned fragments.
To avoid this, lfsr_file_write now checks for buffer.size == 0
explicitly. There may be a better solution, but for now this gets the
job done.
---
To make sure we don't end up with unaligned fragments again in the
future, I've extend the fwrite litmus tests to check for well-aligned
fragments in addition to blocks:
- test_fwrite_simple_litmus_fragments
- test_fwrite_incr_litmus_fragments
These fixes end up adding a bit of code, as checking for both the
unflushed flag and buffer.size == 0 has a cost:
code stack
before: 36424 2680
after: 36452 (+0.1%) 2680 (+0.0%)
But hey, file aren't stuck with unaligned fragments anymore.
It's expected for our crystal boundary calculation to underflow, but
when checking for holes we were using the wrong signed/unsigned
comparison, so lfsr_file_carve thought there was a hole when there
wasn't:
-crs pos pos -crs
.-------| <-- this lookup ------| .--
'---. | +crs --. | +crs '--
. |---|---. ended up |---|---. .
. v v v looking --> v v v .
. .---. like this .---. .
. |dat| |dat| .
. '---' '---' .
. 0 . n 0 . n .
'---.---' '---.---'
no hole clearly a hole
This led to unoptimal block compaction and weird block alignment for
even relatively simple files.
The crystallization threshold is only a heuristic so this didn't exactly
break anything, but it was causing block-aligned files to waste a bit of
of space which wasn't great.
---
To hopefully protect against this in the future, I've added a couple
*_litmus tests to check that at least some simple block-aligned files
end up with the correct number of branches/blocks. This should at least
give us some confidence our crystallization algorithm is working as
intended.
We don't have all that many tests (any?) over the exact topology of
files, mainly because of how many heuristics are involved. Maybe we
should look into adding a couple more.
No code changes:
code stack
before: 36396 2664
after: 36396 (+0.0%) 2664 (+0.0%)
This has been a long-time coming, mount flags are just too useful for
configuring a filesystem at runtime.
Currently this is limited to LFS_M_RDONLY and LFS_M_CKPROGS, but there
are a few more planned in the future:
LFS_M_RDWR = 0x0000, // Mount the filesystem as read and write
LFS_M_RDONLY = 0x0001, // Mount the filesystem as readonly
LFS_M_STRICT* = 0x0002, // Error if on-disk config does not match
LFS_M_FORCE* = 0x0004, // Ignore compat flags, mount readonly
LFS_M_FORCEWITHRECKLESSABANDON*
= 0x0008, // Ignore compat flags, mount read write
LFS_M_CKPROGS = 0x0010, // Check progs by reading back progged data
LFS_M_CKREADS* = 0x0020, // Check reads via checksums
* Hypothetical
As a convenience, we also return mount flags in the struct lfs_fsinfo's
flags field as their relevant LFS_I_* variants. Though only to match
statvfs, and only because it's cheap, littlefs's API is low-level and we
should expect users to know what flags they passed to lfsr_mount.
As for the new mount flags:
- LFS_M_RDONLY - For consistency with existing APIs, this just asserts
on write operations, which makes it a bit useless... But the info flag
LFS_I_RDONLY may be useful for falling back to a readonly mode if
we encounter on-disk compat issues.
At least if implement the theoretical LFS_UNTRUSTED_USER mode
LFS_M_RDONLY could become a runtime error.
- LFS_M_RDWR - This really just exists to compliment LFS_M_RDONLY and to
match LFS_O_RDONLY/LFS_O_RDWR. It's just an alias for 0, and I don't
think there will ever be a reason to make it non-0 (but I can always
be wrong!).
- LFS_M_CKPROGS - This replaces the check_progs config option and avoids
using a full byte to store a bool.
We should probably also have a compile-time option to compile this out
(LFS_NO_CKPROGS?), but that's a future thing to do.
This ended up adding a surprising bit of code, considering we're just
moving flags around, and noise in lfs_alloc added a bit of stack again:
code stack
before: 35880 2672
after: 35932 (+0.1%) 2680 (+0.3%)
We don't actually need these, all we need are utils defined for the
largest integer size we operate on, currently uint32_t.
Counterintuitively this should make it easier to adopt different integer
widths in the future.
Or maybe this will bite us when lfs_off_t >> lfs_size_t? Oh well, if
that's the case we can fix it then.
No code changes:
code stack
before: 33886 2560
after: 33886 (+0.0%) 2560 (+0.0%)
The main test additions are the test_powerloss tests, intended to be
high-level tests over difficult/weird powerloss environments (such as
out-of-order writes!):
- test_powerloss_dir_many - 2242 pls
- test_powerloss_file_many - 8856 pls
- test_powerloss_file_pl_fuzz - 384508 pls
- test_powerloss_filedir_pl_fuzz - 268339 pls
But there was also a bunch of other test movement in the late-stage/
high-level tests. I'm trying to keep the core of these tests somewhat
consistent so we have a nice template to extend for future testing, in
case we want to test other environmentalish concerns, but not all of
these tests make sense in all of these contexts:
badblocks powerloss relocations exhaustion
dir_many y y y
dir_fuzz y y y
file_many y y y
file_fuzz y y y
fwrite_fuzz y y
orphanzombie_fuzz y y y
orphanzombiedir_fuzz y y y
file_pl_fuzz y y
filedir_pl_fuzz y y
Why not:
- dir/file_many+exhaustion? - Needs to be unbounded
- dir/file_fuzz+powerloss? - Takes O(n^2)
- fwrite_fuzz+powerloss? - Takes O(n^2)
- fwrite_fuzz+relocations? - Doesn't really test anything
- orphanzombie*_fuzz+powerloss? - Powerloss kills zombies
- file*_pl_fuzz+badblocks? - PL + Badblocks currently incompactible
- file*_pl_fuzz+exhaustion? - PL + Badblocks currently incompactible
---
Of course, in order to actually get out-of-order write testing working,
we need to implement out-of-order write syncing.
Fortunately this was a simple exercise in placing lfsr_bd_sync calls
before any mdir commits where we may have unsynced data:
- in lfsr_file_sync, to sync any pending file data
- in lfsr_mdir_commit, to sync any mroot/mtree changes
We also call lfsr_bd_sync _after_ mdir commits in case users expect to
sequence any filesystem-external operations such as network, UI, etc. In
theory this could be optional, but no users have really requested it
yet, so leave that for a potential future improvement:
- in lfsr_mdir_commit
- in lfsr_formatinited (really just because we don't go through
lfsr_mdir_commit)
Note that lfsr_rbyd_commit has been relaxed in the scheme. It only
flushes caches, and does _not_ call lfsr_bd_sync. This is useful for
allowing multiple B-tree nodes to be written out-of-order, also long as
the whole thing is synchronized before any mdir commit.
All of these lfsr_bd_sync calls add a bit of code, but not really an
amount to care about:
code stack
before: 33678 2600
after: 33766 (+0.3%) 2600 (+0.0%)
This acts as a marker to indicate a fuzz test. It should reference a
define, usually SEED, that can be randomized to get interesting test
permutations.
This is currently unused, but could lead to some interesting uses such
as time-based fuzz testing. It's also just useful for inspecting the
tests (make test-list).
We've been wasting a lot of test cycles thanks to REMOUNT. Using a test
define for this effectively duplicates the test, when we really just
want to run more post-test code without additional mutation.
The main reason for REMOUNT has been to save typing, which, well, is not
a bad reason, these tests involve a lot of typing...
But this is probably a hammer/nail situation. If we replace these with a
small post-test loop, we can save quite a bit of time:
make test -j before: 5791.9s
make test -j after: 5123.8s (-11.5%)
Some tests still use a REMOUNT define, but these should be limited to
cases where remount actually changes the test's behavior.
A much requested feature, this allows much finer control of how RAM is
allocated for the system.
It was difficult to introduce this in previous versions of littlefs due
to how we steal caches during certain file operations, but now we don't
do that and treat the caches much more transparently.
Managing separate cache sizes does add a bit of code, but this is well
worth the potential for RAM savings due to increased flexibility:
code stack
before: 33656 2632
after: 33714 (+0.2%) 2640 (+0.3%)
Also interesting to note this reduces alignment requirements for the
rcache/pcache, since they don't need to share alignment, and completely
removes any alignment requirement from the file buffers.
The main idea here is that diverse tests are better than many similar
tests.
Sure, if we throw fuzz tests at the system all day we'll eventually find
more bugs, but if a developer is in the loop that time is going to be
better spent writing specific tests targeting the fragile parts of the
system.
And don't worry, we can still throw fuzz tests at the system all day by
specifying explicit seeds with -DSEED=blah.
Changes:
- Limited dir-related powerloss fuzz testing to N <= 16.
These tests were the biggest culprit of excessive test runtime,
requiring O(n^2) redundant operations to recover from powerlosses
(they just replay the full sequence on powerloss).
- As a tradeoff, bumped most fuzz tests to a minimum of 20 seeds.
The big exception being the test_fwrite tests, which are heavily
parameterized and already take the most time to run. Each parameter
combination also multiplies the effective number of seeds, so
increasing the number of base seeds will probably have diminishing
returns.
- Limited test_fwrite_reversed to SIZE <= 4*1024*CHUNK.
Writing a file backwards is just about the worst way you could write a
file, since all buffering/coalescing expect writes to eventually make
forward progress. On the flip side, because it's uncommon, writing a
file backwards is also a great way to find bugs. But at some point a
compromise needs to be made.
Impacted test runtimes:
case otime ntime dtime
test_btree_push_fuzz 0.3 0.5 +0.2 (+60.2%)
test_btree_push_sparse_fuzz 0.4 3.3 +2.9 (+720.4%)
test_btree_update_fuzz 0.4 0.9 +0.6 (+141.6%)
test_btree_update_sparse_fuzz 0.5 4.5 +4.1 (+857.4%)
test_btree_pop_fuzz 0.6 2.3 +1.7 (+314.7%)
test_btree_pop_sparse_fuzz 1.2 5.7 +4.4 (+356.2%)
test_btree_split_fuzz 0.5 1.4 +0.8 (+150.2%)
test_btree_split_sparse_fuzz 0.4 5.6 +5.1 (+1163.2%)
test_btree_find_fuzz 0.5 0.7 +0.2 (+50.7%)
test_btree_find_sparse_fuzz 1.0 3.0 +2.0 (+189.8%)
test_btree_traversal_fuzz 0.6 2.3 +1.6 (+260.4%)
test_dirs_mkdir_many 3.3 2.1 -1.3 (-37.8%)
test_dirs_mkdir_many_backwards 3.5 2.1 -1.4 (-39.9%)
test_dirs_mkdir_fuzz 115.3 106.4 -8.9 (-7.7%)
test_dirs_rm_many 283.9 76.8 -207.0 (-72.9%)
test_dirs_rm_many_backwards 216.1 80.6 -135.5 (-62.7%)
test_dirs_rm_fuzz 647.0 68.5 -578.5 (-89.4%)
test_dirs_mv_many 14.2 15.4 +1.1 (+7.9%)
test_dirs_mv_many_backwards 16.5 14.5 -2.1 (-12.5%)
test_dirs_mv_fuzz 1932.5 156.7 -1775.8 (-91.9%)
test_dirs_general_fuzz 561.9 74.5 -487.4 (-86.7%)
test_dread_recursive_rm 336.6 46.2 -290.4 (-86.3%)
test_dread_recursive_mv 55.5 44.6 -11.0 (-19.8%)
test_fsync_rrrr_fuzz 0.4 0.3 -0.1 (-18.4%)
test_fsync_wrrr_fuzz 8.0 12.4 +4.5 (+56.0%)
test_fsync_wwww_fuzz 13.2 33.4 +20.2 (+152.6%)
test_fsync_wwrr_fuzz 5.4 50.9 +45.5 (+841.6%)
test_fsync_rwrw_fuzz 2.4 8.4 +6.0 (+253.9%)
test_fsync_rwrw_sparse_fuzz 3.2 7.5 +4.2 (+129.9%)
test_fsync_rwtfrwtf_sparse_fuzz 6.1 8.5 +2.4 (+39.3%)
test_fsync_drrr_fuzz 11.8 9.2 -2.6 (-21.8%)
test_fsync_wddd_fuzz 9.3 11.9 +2.6 (+28.0%)
test_fsync_rwdrwd_fuzz 1.6 33.1 +31.5 (+1963.4%)
test_fsync_rwdrwd_sparse_fuzz 0.3 1.8 +1.4 (+418.8%)
test_fsync_rwtfdrwtfd_sparse_fuzz 0.3 1.1 +0.8 (+260.2%)
test_fwrite_reversed 728.5 345.2 -383.3 (-52.6%)
TOTAL 7587.5 3792.3 -3795.2 (-50.0%)
We really had ~2 duplicate bd layers for a bit there.
This also involved a sort of rewrite of these low-level functions to see
if there were simplifications that could be made.
A couple tweaks:
- Added small low-level lfsr_bd_read/prog/erase/sync_ functions to
only wrap the bd callbacks and apply any relevant asserts.
These should be the only place we call the bd callbacks to make it
easy to read/audit/insert hooks in the future.
- Changed pcache flush lazily, rather than eagerly flushing when full.
This isn't for any real performance reason, it just makes the code
simpler. It's not like we can shove more data into the pcache once
full.
It's _probably_ a good idea to flush eagerly, to avoid delay more work
until sync, but I couldn't figure out how to make this work cleanly
without code duplication...
- Deduplicated read pcache overwrites via lfsr_bd_read__.
This logic is a bit annoying, but we need the pcache to take priority
whenever we read from disk, which happens when we both fill our
rcache, and bypass our rcache. Since these code paths go different
places, another internal function was the only way I could think to
deduplicate this.
It may appear that our pcache/rcache prioritization loop will make
this happen naturally, as it does in lfs_file_read for example, but
this doesn't quite work as read-alignment requirements may force us to
read past the pcache... Keep in mind read_size may be > prog_size.
- Dropped LFS_BLOCK_NULL, now using cache.size=0 to indicate a cache is
unused.
This avoids a special lfs_block_t value.
- Dropped lfsr_bd_readcksum, we never used this.
We can always add it back if necessary.
In total, the caching bd prog/read functions now look quite a bit more
like our file read/write functions, so hopefully that's a good thing.
By the virtue of not have ~2 duplicate bd layers, this saves a bit of
code:
code stack
before: 33700 2800
after: 33560 (-0.4%) 2808 (+0.3%)
The size field in lfs_info doesn't really make sense for stat/dir_read
when the file is a directory. Still, we should probably set it to 0 os
it's not uninitialized.
Fortunately we were already setting size=0 in _most_ cases, this commit
is mostly just checking for size=0 in more test cases.
This requires two things:
1. Any opened file handles need to have their mid/mdir updated after the
rename succeeds.
2. Any shrubs/sprouts need to be copied over to the new mdir, even if
they aren't in-tree.
The LFSR_TAG_MOVE operation is starting to look an awfully lot like
lfsr_mdir_compact... Unfortunately lfsr_mdir_compact, uh, compacts,
whereas LFSR_TAG_MOVE appends to the rbyd like normal, so it's not clear
exactly _how_ to deduplicate.
Now mixing in truncate/fruncate, along with desync<->sync state
transitions.
Found bugs:
- Fixed propagating LFS_F_UNSYNCED/LFS_F_UNFLUSHED state during sync
broadcasts. This is important for tracking small files correctly.
- We were not clearing the btree erased-state of other opened file
handles when we started using it, leading other file handles to have
out-of-date erased-state.
I considered moving this into lfsr_btree_commit, but file btrees are
really the only place where shared references make sense, and it feels
weird to scan file btrees every time we commit to the mtree.
- Fixed syncs not propagating to other file handles when file is synced
with disk.
It's interesting that lfsr_file_sync can actually have an effect on
the system when the disk in is-sync.
- Added O_FLUSH/O_SYNC support to lfsr_file_truncate/fruncate. This
omission was just an oversight.
Unfortunately this did add quite a bit more complexity to both
functions.
You may notice in the fix for that last bug, that lfsr_file_ftruncate
sort of drops the ball with regards to error-idempotency. This is
because, as I was trying to figure out how to recoverably move the
buffer around when fruncating small files, I realized we don't handle
small files in lfsr_file_write correctly w.r.t. error-idempotency, and
that fixing this may be intractable...
The issue is how handle overwrites for unflushed buffers.
In general, the correct thing to do when an incoming write overlaps our
file buffer, is to just write over the buffer with the new data.
Ah, but if we do this, how do we get the old data back if we run into an
error writing the data to disk? It's gone!
For normal files, this is not an issue. We can always flush to disk to
reclaim our buffer, and since a flush doesn't change the file contents,
it's fine to make this our new fallback state.
But for small files, flush is a noop, we keep these entirely in RAM.
There are some possible workarounds:
- Flush small files to disk before overwriting, sort of defeats the
purpose of caching these in RAM...
- Reread small files from disk, because that's definitely what you want
to do when you hit an error...
Also, to always have something we can read from disk implies flush
on overwrite, see above.
- Sacrificing half our buffer for staging small files. Because RAM cost
is totally not a priority...
Long story short, rethinking idempotent errors.
Much like the erased-state checksums in our rbyds (ecksums), these
block-level erased-state checksums (becksums) allow us to detect failed
progs to erased parts of a block and are key to achieving efficient
incremental write performance with large blocks and frequent power
cycles/open-close cycles.
These are also key to achieving _reasonable_ write performance for
simple writes (linear, non-overwriting), since littlefs now relies
solely on becksums to efficiently append to blocks.
Though I suppose the previous block staging logic used with the CTZ
skip-list could be brought back to make becksums optional and avoid
btree lookups during simple writes (we do a _lot_ of btree
lookups)... I'll leave this open as a future optimization...
Unlike in-rbyd ecksums, becksums need to be stored out-of-band so our
data blocks only contain raw data. Since they are optional, an
additional tag in the file's btree makes sense.
Becksums are relatively simple, but they bring some challenges:
1. Adding becksums to file btrees is the first case we have for multiple
struct tags per btree id.
This isn't too complicated a problem, but requires some new internal
btree APIs.
Looking forward, which I probably shouldn't be doing this often,
multiple struct tags will also be useful for parity and content ids
as a part of data redundancy and data deduplication, though I think
it's uncontroversial to consider this both heavier-weight features...
2. Becksums only work if unfilled blocks are aligned to the prog_size.
This is the whole point of crystal_size -- to provide temporary
storage for unaligned writes -- but actually aligning the block
during writes turns out to be a bit tricky without a bunch of
unecesssary btree lookups (we already do too many btree lookups!).
The current implementation here discards the pcache to force
alignment, taking advantage of the requirement that
cache_size >= prog_size, but this is corrupting our block checksums.
Code cost:
code stack
before: 31248 2792
after: 32060 (+2.5%) 2864 (+2.5%)
Also lfsr_ftree_flush needs work. I'm usually open to gotos in C when
they improve internal logic, but even for me, the multiple goto jumps
from every left-neighbor lookup into the block writing loop is a bit
much...
Christopher Haster