kv: Implemented one-commit lfs3_set

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 commit is contained in:
Christopher Haster
2025-06-18 16:46:41 -05:00
parent a75537faff
commit 0772d10dbc
+238 -110
View File
@@ -11321,7 +11321,7 @@ static inline lfs3_size_t lfs3_file_cachesize(lfs3_t *lfs3,
: lfs3->cfg->file_cache_size; : lfs3->cfg->file_cache_size;
} }
static inline lfs3_off_t lfs3_file_weight(const lfs3_file_t *file) { static inline lfs3_off_t lfs3_file_weight_(const lfs3_file_t *file) {
return lfs3_max( return lfs3_max(
file->leaf.pos + file->leaf.weight, file->leaf.pos + file->leaf.weight,
file->b.shrub.weight); file->b.shrub.weight);
@@ -11330,7 +11330,7 @@ static inline lfs3_off_t lfs3_file_weight(const lfs3_file_t *file) {
static inline lfs3_off_t lfs3_file_size_(const lfs3_file_t *file) { static inline lfs3_off_t lfs3_file_size_(const lfs3_file_t *file) {
return lfs3_max( return lfs3_max(
file->cache.pos + file->cache.size, file->cache.pos + file->cache.size,
lfs3_file_weight(file)); lfs3_file_weight_(file));
} }
@@ -11342,24 +11342,14 @@ static void lfs3_file_init(lfs3_file_t *file, uint32_t flags,
file->cfg = cfg; file->cfg = cfg;
file->b.o.flags = lfs3_o_typeflags(LFS3_TYPE_REG) | flags; file->b.o.flags = lfs3_o_typeflags(LFS3_TYPE_REG) | flags;
file->pos = 0; file->pos = 0;
// default to no cache lfs3_file_discardcache(file);
file->cache.size = 0; lfs3_file_discardleaf(file);
lfs3_file_discardbshrub(file);
} }
static int lfs3_file_fetch(lfs3_t *lfs3, lfs3_file_t *file, uint32_t flags) { static int lfs3_file_fetch(lfs3_t *lfs3, lfs3_file_t *file, uint32_t flags) {
// default data state
lfs3_file_discardbshrub(file);
// discard the current cache
lfs3_file_discardcache(file);
// discard the current leaf
lfs3_file_discardleaf(file);
// don't bother reading disk if we're not created or truncating // don't bother reading disk if we're not created or truncating
if (lfs3_o_isuncreat(flags) || lfs3_o_istrunc(flags)) { if (!lfs3_o_isuncreat(flags) && !lfs3_o_istrunc(flags)) {
// but do mark as unsync
file->b.o.flags |= LFS3_o_UNSYNC;
} else {
// lookup the file struct, if there is one // lookup the file struct, if there is one
lfs3_tag_t tag; lfs3_tag_t tag;
lfs3_data_t data; lfs3_data_t data;
@@ -11531,15 +11521,15 @@ int lfs3_file_opencfg(lfs3_t *lfs3, lfs3_file_t *file,
return LFS3_ERR_NOTDIR; return LFS3_ERR_NOTDIR;
} }
// create a stickynote entry if we don't have one, this reserves the // check that name fits
// mid until first sync lfs3_size_t name_len = lfs3_path_namelen(path);
if (!exists) { if (name_len > lfs3->name_limit) {
// check that name fits return LFS3_ERR_NAMETOOLONG;
lfs3_size_t name_len = lfs3_path_namelen(path); }
if (name_len > lfs3->name_limit) {
return LFS3_ERR_NAMETOOLONG;
}
if (!exists) {
// create a stickynote entry if we don't have one, this
// reserves the mid until first sync
lfs3_alloc_ckpoint(lfs3); lfs3_alloc_ckpoint(lfs3);
err = lfs3_mdir_commit(lfs3, &file->b.o.mdir, LFS3_RATTRS( err = lfs3_mdir_commit(lfs3, &file->b.o.mdir, LFS3_RATTRS(
LFS3_RATTR_NAME( LFS3_RATTR_NAME(
@@ -11559,8 +11549,8 @@ int lfs3_file_opencfg(lfs3_t *lfs3, lfs3_file_t *file,
} }
} }
// mark as uncreated // mark as uncreated + unsync
file->b.o.flags |= LFS3_o_UNCREAT; file->b.o.flags |= LFS3_o_UNCREAT | LFS3_o_UNSYNC;
#endif #endif
} else { } else {
// wanted to create a new entry? // wanted to create a new entry?
@@ -11577,9 +11567,14 @@ int lfs3_file_opencfg(lfs3_t *lfs3, lfs3_file_t *file,
} }
#ifndef LFS3_RDONLY #ifndef LFS3_RDONLY
// if stickynote, mark as uncreated // if stickynote, mark as uncreated + unsync
if (tag == LFS3_TAG_STICKYNOTE) { if (tag == LFS3_TAG_STICKYNOTE) {
file->b.o.flags |= LFS3_o_UNCREAT; file->b.o.flags |= LFS3_o_UNCREAT | LFS3_o_UNSYNC;
}
// if truncating, mark as unsync
if (lfs3_o_istrunc(file->b.o.flags)) {
file->b.o.flags |= LFS3_o_UNSYNC;
} }
#endif #endif
} }
@@ -11912,7 +11907,7 @@ lfs3_ssize_t lfs3_file_read(lfs3_t *lfs3, lfs3_file_t *file,
} }
// any data in our btree? // any data in our btree?
if (pos_ < lfs3_file_weight(file)) { if (pos_ < lfs3_file_weight_(file)) {
// bypass cache? // bypass cache?
if ((lfs3_size_t)d >= lfs3_file_cachesize(lfs3, file)) { if ((lfs3_size_t)d >= lfs3_file_cachesize(lfs3, file)) {
lfs3_ssize_t d_ = lfs3_file_read_(lfs3, file, lfs3_ssize_t d_ = lfs3_file_read_(lfs3, file,
@@ -12261,7 +12256,7 @@ static int lfs3_file_crystallize_(lfs3_t *lfs3, lfs3_file_t *file,
lfs3->cfg->block_size), lfs3->cfg->block_size),
lfs3_max( lfs3_max(
pos + size, pos + size,
lfs3_file_weight(file))); lfs3_file_weight_(file)));
// do we need to allocate a new block? // do we need to allocate a new block?
if (!lfs3_bptr_isbptr(&file->leaf.bptr) if (!lfs3_bptr_isbptr(&file->leaf.bptr)
@@ -12333,7 +12328,7 @@ static int lfs3_file_crystallize_(lfs3_t *lfs3, lfs3_file_t *file,
} }
// any data on disk? // any data on disk?
if (pos_ < lfs3_file_weight(file)) { if (pos_ < lfs3_file_weight_(file)) {
lfs3_bid_t bid__; lfs3_bid_t bid__;
lfs3_bid_t weight__; lfs3_bid_t weight__;
lfs3_bptr_t bptr__; lfs3_bptr_t bptr__;
@@ -12601,7 +12596,7 @@ static int lfs3_file_flush_(lfs3_t *lfs3, lfs3_file_t *file,
0); 0);
if (crystal_end - crystal_start < lfs3->cfg->crystal_thresh if (crystal_end - crystal_start < lfs3->cfg->crystal_thresh
&& crystal_start > 0 && crystal_start > 0
&& poke < lfs3_file_weight(file) && poke < lfs3_file_weight_(file)
// don't bother looking up left after the first block // don't bother looking up left after the first block
&& !aligned) { && !aligned) {
lfs3_bid_t bid; lfs3_bid_t bid;
@@ -12634,9 +12629,9 @@ static int lfs3_file_flush_(lfs3_t *lfs3, lfs3_file_t *file,
// find right crystal neighbor // find right crystal neighbor
poke = lfs3_min( poke = lfs3_min(
crystal_start + (lfs3->cfg->crystal_thresh-1), crystal_start + (lfs3->cfg->crystal_thresh-1),
lfs3_file_weight(file)-1); lfs3_file_weight_(file)-1);
if (crystal_end - crystal_start < lfs3->cfg->crystal_thresh if (crystal_end - crystal_start < lfs3->cfg->crystal_thresh
&& crystal_end < lfs3_file_weight(file)) { && crystal_end < lfs3_file_weight_(file)) {
lfs3_bid_t bid; lfs3_bid_t bid;
lfs3_bid_t weight; lfs3_bid_t weight;
lfs3_bptr_t bptr; lfs3_bptr_t bptr;
@@ -12827,7 +12822,7 @@ fragment:;
// is already full // is already full
if (fragment_end - fragment_start < lfs3->cfg->fragment_size if (fragment_end - fragment_start < lfs3->cfg->fragment_size
&& fragment_start > 0 && fragment_start > 0
&& fragment_start <= lfs3_file_weight(file) && fragment_start <= lfs3_file_weight_(file)
// don't bother to lookup left after first fragment // don't bother to lookup left after first fragment
&& !aligned) { && !aligned) {
lfs3_bid_t bid; lfs3_bid_t bid;
@@ -12876,7 +12871,7 @@ fragment:;
// //
// note this may the same as our left sibling // note this may the same as our left sibling
if (fragment_end - fragment_start < lfs3->cfg->fragment_size if (fragment_end - fragment_start < lfs3->cfg->fragment_size
&& fragment_end < lfs3_file_weight(file)) { && fragment_end < lfs3_file_weight_(file)) {
lfs3_bid_t bid; lfs3_bid_t bid;
lfs3_bid_t weight; lfs3_bid_t weight;
lfs3_bptr_t bptr; lfs3_bptr_t bptr;
@@ -13183,9 +13178,8 @@ static int lfs3_file_sync_(lfs3_t *lfs3, lfs3_file_t *file) {
// this only works if the file is entirely in our cache // this only works if the file is entirely in our cache
LFS3_ASSERT(file->cache.pos == 0); LFS3_ASSERT(file->cache.pos == 0);
LFS3_ASSERT(file->cache.size == lfs3_file_size_(file)); LFS3_ASSERT(file->cache.size == lfs3_file_size_(file));
// bshrub should be discarded here
// reset the bshrub LFS3_ASSERT(lfs3_file_weight_(file) == 0);
lfs3_file_discardbshrub(file);
// build a small shrub commit // build a small shrub commit
if (file->cache.size > 0) { if (file->cache.size > 0) {
@@ -13295,55 +13289,6 @@ static int lfs3_file_sync_(lfs3_t *lfs3, lfs3_file_t *file) {
} }
} }
return 0;
}
#endif
int lfs3_file_sync(lfs3_t *lfs3, lfs3_file_t *file) {
(void)lfs3;
LFS3_ASSERT(lfs3_omdir_isopen(lfs3, &file->b.o));
// removed? ignore sync requests
if (lfs3_o_iszombie(file->b.o.flags)) {
return 0;
}
#ifndef LFS3_RDONLY
// first flush any data in our cache, this is a noop if already
// flushed
//
// note that flush does not change the actual file data, so if
// flush succeeds but mdir commit fails it's ok to fall back to
// our flushed state
//
// though don't flush quite yet if our file is small and can be
// combined with sync in a single commit
int err;
if (file->cache.size == lfs3_file_size_(file)
&& file->cache.size <= lfs3->cfg->inline_size
&& file->cache.size <= lfs3->cfg->fragment_size
&& file->cache.size < lfs3->cfg->crystal_thresh) {
if (lfs3_o_isungraft(file->b.o.flags)) {
file->b.o.flags |= LFS3_o_UNFLUSH;
}
lfs3_file_discardleaf(file);
} else {
err = lfs3_file_flush(lfs3, file);
if (err) {
goto failed;
}
}
// commit any pending metadata to disk
//
// the use of a second function here is mainly to isolate the stack
// costs of lfs3_file_flush and lfs3_file_sync_
//
err = lfs3_file_sync_(lfs3, file);
if (err) {
goto failed;
}
// update in-device state // update in-device state
for (lfs3_omdir_t *o = lfs3->omdirs; o; o = o->next) { for (lfs3_omdir_t *o = lfs3->omdirs; o; o = o->next) {
if (lfs3_o_type(o->flags) == LFS3_TYPE_REG if (lfs3_o_type(o->flags) == LFS3_TYPE_REG
@@ -13422,15 +13367,73 @@ int lfs3_file_sync(lfs3_t *lfs3, lfs3_file_t *file) {
lfs3_traversal_clobber(lfs3, (lfs3_traversal_t*)o); lfs3_traversal_clobber(lfs3, (lfs3_traversal_t*)o);
} }
} }
#endif
// mark as synced // mark as synced
file->b.o.flags &= ~LFS3_o_UNSYNC file->b.o.flags &= ~LFS3_o_UNSYNC
& ~LFS3_o_UNFLUSH & ~LFS3_o_UNFLUSH
& ~LFS3_o_UNCRYST & ~LFS3_o_UNCRYST
& ~LFS3_o_UNGRAFT & ~LFS3_o_UNGRAFT
& ~LFS3_o_UNCREAT & ~LFS3_o_UNCREAT;
& ~LFS3_O_DESYNC; return 0;
}
#endif
int lfs3_file_sync(lfs3_t *lfs3, lfs3_file_t *file) {
(void)lfs3;
LFS3_ASSERT(lfs3_omdir_isopen(lfs3, &file->b.o));
// removed? ignore sync requests
if (lfs3_o_iszombie(file->b.o.flags)) {
return 0;
}
#ifndef LFS3_RDONLY
// first flush any data in our cache, this is a noop if already
// flushed
//
// note that flush does not change the actual file data, so if
// flush succeeds but mdir commit fails it's ok to fall back to
// our flushed state
//
// though don't flush quite yet if our file is small and can be
// combined with sync in a single commit
int err;
if (lfs3_o_isunflush(file->b.o.flags)
&& file->cache.size == lfs3_file_size_(file)
&& file->cache.size <= lfs3->cfg->inline_size
&& file->cache.size <= lfs3->cfg->fragment_size
&& file->cache.size < lfs3->cfg->crystal_thresh) {
// size == size implies pos == 0
LFS3_ASSERT(file->cache.pos == 0);
// map any weird bshrub/btree states to unflush
if (lfs3_o_isuncryst(file->b.o.flags)
|| lfs3_o_isungraft(file->b.o.flags)) {
file->b.o.flags |= LFS3_o_UNFLUSH;
}
// discard any lingering bshrub state
lfs3_file_discardleaf(file);
lfs3_file_discardbshrub(file);
} else {
err = lfs3_file_flush(lfs3, file);
if (err) {
goto failed;
}
}
// commit any pending metadata to disk
//
// the use of a second function here is mainly to isolate the
// stack costs of lfs3_file_flush and lfs3_file_sync_
//
err = lfs3_file_sync_(lfs3, file);
if (err) {
goto failed;
}
#endif
// clear desync flag
file->b.o.flags &= ~LFS3_O_DESYNC;
return 0; return 0;
#ifndef LFS3_RDONLY #ifndef LFS3_RDONLY
@@ -13467,6 +13470,11 @@ int lfs3_file_resync(lfs3_t *lfs3, lfs3_file_t *file) {
// do nothing if already in-sync // do nothing if already in-sync
if (lfs3_o_isunsync(file->b.o.flags)) { if (lfs3_o_isunsync(file->b.o.flags)) {
// discard cached state
lfs3_file_discardbshrub(file);
lfs3_file_discardcache(file);
lfs3_file_discardleaf(file);
// refetch the file struct from disk // refetch the file struct from disk
err = lfs3_file_fetch(lfs3, file, err = lfs3_file_fetch(lfs3, file,
// don't truncate again! // don't truncate again!
@@ -13477,7 +13485,7 @@ int lfs3_file_resync(lfs3_t *lfs3, lfs3_file_t *file) {
} }
#endif #endif
// mark as resynced // clear desync flag
file->b.o.flags &= ~LFS3_O_DESYNC; file->b.o.flags &= ~LFS3_O_DESYNC;
return 0; return 0;
@@ -13909,33 +13917,153 @@ lfs3_ssize_t lfs3_size(lfs3_t *lfs3, const char *path) {
} }
#ifndef LFS3_RDONLY #ifndef LFS3_RDONLY
int lfs3_set(lfs3_t *lfs3, const char *path, LFS3_NOINLINE
static int lfs3_file_openset(lfs3_t *lfs3, lfs3_file_t *file,
const char *path,
const void *buffer, lfs3_size_t size) { const void *buffer, lfs3_size_t size) {
// we just use the file API here, but with no cache so all writes // prepare our filesystem for writing
// bypass the cache int err = lfs3_fs_mkconsistent(lfs3);
lfs3_file_t file;
int err = lfs3_file_opencfg(lfs3, &file, path,
LFS3_O_WRONLY | LFS3_O_CREAT | LFS3_O_TRUNC,
&lfs3_file_kvconfig);
if (err) { if (err) {
return err; return err;
} }
lfs3_ssize_t size_ = lfs3_file_write(lfs3, &file, buffer, size); // setup file state
if (size_ < 0) { lfs3_file_init(file, LFS3_O_WRONLY | LFS3_O_CREAT | LFS3_O_TRUNC,
err = size_; &lfs3_file_kvconfig);
// lookup our parent
lfs3_tag_t tag;
lfs3_did_t did;
err = lfs3_mtree_pathlookup(lfs3, &path,
&file->b.o.mdir, &tag, &did);
if (err && !(err == LFS3_ERR_NOENT && lfs3_path_islast(path))) {
return err;
}
bool exists = (err != LFS3_ERR_NOENT);
// creating a new entry?
if (!exists || tag == LFS3_TAG_ORPHAN) {
// we're a file, don't allow trailing slashes
if (lfs3_path_isdir(path)) {
return LFS3_ERR_NOTDIR;
}
// check that name fits
lfs3_size_t name_len = lfs3_path_namelen(path);
if (name_len > lfs3->name_limit) {
return LFS3_ERR_NAMETOOLONG;
}
// mark as uncreated + unsync
file->b.o.flags |= LFS3_o_UNCREAT | LFS3_o_UNSYNC;
if (!exists) {
// small file? can we atomically commit everything? currently
// this is only possible via lfs3_set
if (size <= lfs3->cfg->inline_size
&& size <= lfs3->cfg->fragment_size
&& size < lfs3->cfg->crystal_thresh) {
// TODO build commit?
// TODO test size=0?
// commit create + flush atomically
lfs3_alloc_ckpoint(lfs3);
err = lfs3_mdir_commit(lfs3, &file->b.o.mdir, LFS3_RATTRS(
LFS3_RATTR_NAME(
LFS3_TAG_REG, +1,
did, path, name_len),
(size > 0)
? LFS3_RATTR_SHRUBCOMMIT(
(&(lfs3_shrubcommit_t){
.bshrub=&file->b,
.rid=0,
.rattrs=&LFS3_RATTR_DATA(
LFS3_TAG_DATA, +size,
&LFS3_DATA_BUF(buffer, size)),
.rattr_count=1}))
: LFS3_RATTR_NOOP(),
(size > 0)
? LFS3_RATTR_SHRUB(
LFS3_TAG_BSHRUB, 0,
&file->b.shrub_)
: LFS3_RATTR_NOOP()));
if (err) {
return err;
}
// TODO better way to structure this?
// clear the uncreated + unsync flags
file->b.o.flags &= ~LFS3_o_UNCREAT & ~LFS3_o_UNSYNC;
} else {
// create a stickynote entry if we don't have one, this
// reserves the mid until first sync
lfs3_alloc_ckpoint(lfs3);
err = lfs3_mdir_commit(lfs3, &file->b.o.mdir, LFS3_RATTRS(
LFS3_RATTR_NAME(
LFS3_TAG_STICKYNOTE, +1,
did, path, name_len)));
if (err) {
return err;
}
}
// TODO move this to lfs3_mdir_commit
// update dir positions
for (lfs3_omdir_t *o = lfs3->omdirs; o; o = o->next) {
if (lfs3_o_type(o->flags) == LFS3_TYPE_DIR
&& ((lfs3_dir_t*)o)->did == did
&& o->mdir.mid >= file->b.o.mdir.mid) {
((lfs3_dir_t*)o)->pos += 1;
}
}
}
} else {
// wrong type?
if (tag == LFS3_TAG_DIR) {
return LFS3_ERR_ISDIR;
}
if (tag == LFS3_TAG_UNKNOWN) {
return LFS3_ERR_NOTSUP;
}
// if stickynote, mark as uncreated + unsync
if (tag == LFS3_TAG_STICKYNOTE) {
file->b.o.flags |= LFS3_o_UNCREAT | LFS3_o_UNSYNC;
}
// if truncating, mark as unsync
file->b.o.flags |= LFS3_o_UNSYNC;
} }
// unconditionally close // add to tracked mdirs
int err_ = lfs3_file_close(lfs3, &file); lfs3_omdir_open(lfs3, &file->b.o);
if (err_) { return 0;
// we didn't allocate anything, and write failing would set the }
// desync flag, so only one of write/close can fail #endif
LFS3_ASSERT(!err);
err = err_; #ifndef LFS3_RDONLY
int lfs3_set(lfs3_t *lfs3, const char *path,
const void *buffer, lfs3_size_t size) {
lfs3_file_t file;
int err = lfs3_file_openset(lfs3, &file, path,
buffer, size);
if (err) {
return err;
} }
return err; // if not small, pretend the buffer is our cache
if (lfs3_o_isunsync(file.b.o.flags)) {
file.b.o.flags |= LFS3_o_UNFLUSH;
file.cache.buffer = (uint8_t*)buffer;
file.cache.pos = 0;
file.cache.size = size;
}
// let close do all the remaining work
//
// this includes grafting our cache into the bshrub/btree, and
// broadcasting the changes to any open files
return lfs3_file_close(lfs3, &file);
} }
#endif #endif