Github/littlefs
1
Fork 0
Code Actions Activity

Implemented 1-commit shrubs for small in-cache files

Browse Source 
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.
This commit is contained in:
Christopher Haster
2025-05-16 11:10:52 -05:00
parent c44c43ac74
commit b6a0b7afe2
1 changed files with 51 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+51 -8
View File
@@ -12715,15 +12715,25 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
// flush succeeds but mdir commit fails it's ok to fall back to
// our flushed state
//
err = lfsr_file_flush(lfs, file);
if (err) {
goto failed;
// though don't flush quite yet if our file is small and can be
// combined with sync in a single commit
if (file->cache.size < lfsr_file_size_(file)
|| file->cache.size > lfs->cfg->inline_size
|| file->cache.size > lfs->cfg->fragment_size
|| file->cache.size >= lfs->cfg->crystal_thresh) {
err = lfsr_file_flush(lfs, file);
if (err) {
goto failed;
}
}
// build a commit of any pending file metadata
lfsr_rattr_t rattrs[3];
lfsr_rattr_t rattrs[4];
lfs_size_t rattr_count = 0;
lfsr_data_t name_data;
lfsr_rattr_t shrub_rattrs[1];
lfs_size_t shrub_rattr_count = 0;
lfsr_shrubcommit_t shrub_commit;
// not created yet? need to convert to normal file
if (lfsr_o_isuncreat(file->b.o.flags)) {
@@ -12744,6 +12754,31 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
&name_data);
}
// pending small file flush?
if (lfsr_o_isunflush(file->b.o.flags)) {
// this only works if the file is entirely in our cache
LFS_ASSERT(file->cache.pos == 0);
LFS_ASSERT(file->cache.size == lfsr_file_size_(file));
// reset the bshrub
lfsr_bshrub_init(&file->b);
// build a small shrub commit
if (file->cache.size > 0) {
shrub_rattrs[shrub_rattr_count++] = LFSR_RATTR_DATA(
LFSR_TAG_DATA, +file->cache.size,
(const lfsr_data_t*)&file->cache);
LFS_ASSERT(shrub_rattr_count
<= sizeof(shrub_rattrs)/sizeof(lfsr_rattr_t));
shrub_commit.bshrub = &file->b;
shrub_commit.rid = 0;
shrub_commit.rattrs = shrub_rattrs;
shrub_commit.rattr_count = shrub_rattr_count;
rattrs[rattr_count++] = LFSR_RATTR_SHRUBCOMMIT(&shrub_commit);
}
}
// pending file changes?
if (lfsr_o_isunsync(file->b.o.flags)) {
// make sure data is on-disk before committing metadata
@@ -12753,15 +12788,19 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
}
// zero size files should have no bshrub/btree
LFS_ASSERT(file->b.shrub.weight > 0
LFS_ASSERT(lfsr_file_size_(file) > 0
|| lfsr_bshrub_isbnull(&file->b));
// no bshrub/btree?
if (lfsr_bshrub_isbnull(&file->b)) {
if (lfsr_bshrub_isbnull(&file->b)
&& !(lfsr_o_isunflush(file->b.o.flags)
&& file->cache.size > 0)) {
rattrs[rattr_count++] = LFSR_RATTR(
LFSR_TAG_RM | LFSR_TAG_MASK8 | LFSR_TAG_STRUCT, 0);
// bshrub?
} else if (lfsr_bshrub_isbshrub(&file->b)) {
} else if (lfsr_bshrub_isbshrub(&file->b)
|| (lfsr_o_isunflush(file->b.o.flags)
&& file->cache.size > 0)) {
rattrs[rattr_count++] = LFSR_RATTR_SHRUB(
LFSR_TAG_MASK8 | LFSR_TAG_BSHRUB, 0,
// note we use the staged trunk here
@@ -12900,7 +12939,11 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
}
// mark as synced
file->b.o.flags &= ~(LFS_o_UNSYNC | LFS_o_UNCREAT | LFS_O_DESYNC);
file->b.o.flags &= ~(
LFS_o_UNSYNC
| LFS_o_UNFLUSH
| LFS_o_UNCREAT
| LFS_O_DESYNC);
return 0;
failed:;