Force lfsr_file_sync_ off the stack hot-path

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 commit is contained in:
Christopher Haster
2025-05-16 13:24:25 -05:00
parent b6a0b7afe2
commit 930fe6e67c
2 changed files with 62 additions and 40 deletions
+55 -40
View File
@@ -12696,37 +12696,9 @@ failed:;
return err; return err;
} }
int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) { // this LFS_NOINLINE is to force lfsr_file_sync_ off the stack hot-path
LFS_ASSERT(lfsr_omdir_isopen(lfs, &file->b.o)); LFS_NOINLINE
// can't write to readonly files, if you want to resync call static int lfsr_file_sync_(lfs_t *lfs, lfsr_file_t *file) {
// lfsr_file_resync
LFS_ASSERT(!lfsr_o_isrdonly(file->b.o.flags));
// removed? sync is just a noop in this case
int err;
if (lfsr_o_iszombie(file->b.o.flags)) {
return 0;
}
// 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
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 // build a commit of any pending file metadata
lfsr_rattr_t rattrs[4]; lfsr_rattr_t rattrs[4];
lfs_size_t rattr_count = 0; lfs_size_t rattr_count = 0;
@@ -12740,13 +12712,13 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
// uncreated files must be unsynced // uncreated files must be unsynced
LFS_ASSERT(lfsr_o_isunsync(file->b.o.flags)); LFS_ASSERT(lfsr_o_isunsync(file->b.o.flags));
err = lfsr_rbyd_lookup(lfs, &file->b.o.mdir.rbyd, int err = lfsr_rbyd_lookup(lfs, &file->b.o.mdir.rbyd,
lfsr_mrid(lfs, file->b.o.mdir.mid), LFSR_TAG_STICKYNOTE, lfsr_mrid(lfs, file->b.o.mdir.mid), LFSR_TAG_STICKYNOTE,
NULL, &name_data); NULL, &name_data);
if (err) { if (err) {
// orphan flag but no stickynote tag? // orphan flag but no stickynote tag?
LFS_ASSERT(err != LFS_ERR_NOENT); LFS_ASSERT(err != LFS_ERR_NOENT);
goto failed; return err;
} }
rattrs[rattr_count++] = LFSR_RATTR_DATA( rattrs[rattr_count++] = LFSR_RATTR_DATA(
@@ -12782,9 +12754,9 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
// pending file changes? // pending file changes?
if (lfsr_o_isunsync(file->b.o.flags)) { if (lfsr_o_isunsync(file->b.o.flags)) {
// make sure data is on-disk before committing metadata // make sure data is on-disk before committing metadata
err = lfsr_bd_sync(lfs); int err = lfsr_bd_sync(lfs);
if (err) { if (err) {
goto failed; return err;
} }
// zero size files should have no bshrub/btree // zero size files should have no bshrub/btree
@@ -12831,19 +12803,18 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
// lookup the attr // lookup the attr
lfsr_data_t data; lfsr_data_t data;
err = lfsr_mdir_lookup(lfs, &file->b.o.mdir, int err = lfsr_mdir_lookup(lfs, &file->b.o.mdir,
LFSR_TAG_ATTR(file->cfg->attrs[i].type), LFSR_TAG_ATTR(file->cfg->attrs[i].type),
NULL, &data); NULL, &data);
if (err && err != LFS_ERR_NOENT) { if (err && err != LFS_ERR_NOENT) {
goto failed; return err;
} }
// does disk match our attr? // does disk match our attr?
lfs_scmp_t cmp = lfsr_attr_cmp(lfs, &file->cfg->attrs[i], lfs_scmp_t cmp = lfsr_attr_cmp(lfs, &file->cfg->attrs[i],
(err != LFS_ERR_NOENT) ? &data : NULL); (err != LFS_ERR_NOENT) ? &data : NULL);
if (cmp < 0) { if (cmp < 0) {
err = cmp; return cmp;
goto failed;
} }
if (cmp != LFS_CMP_EQ) { if (cmp != LFS_CMP_EQ) {
@@ -12865,13 +12836,57 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
// commit! // commit!
LFS_ASSERT(rattr_count <= sizeof(rattrs)/sizeof(lfsr_rattr_t)); LFS_ASSERT(rattr_count <= sizeof(rattrs)/sizeof(lfsr_rattr_t));
err = lfsr_mdir_commit(lfs, &file->b.o.mdir, int err = lfsr_mdir_commit(lfs, &file->b.o.mdir,
rattrs, rattr_count); rattrs, rattr_count);
if (err) {
return err;
}
}
return 0;
}
int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
LFS_ASSERT(lfsr_omdir_isopen(lfs, &file->b.o));
// can't write to readonly files, if you want to resync call
// lfsr_file_resync
LFS_ASSERT(!lfsr_o_isrdonly(file->b.o.flags));
// removed? sync is just a noop in this case
int err;
if (lfsr_o_iszombie(file->b.o.flags)) {
return 0;
}
// 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
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) { if (err) {
goto failed; goto failed;
} }
} }
// commit any pending metadata to disk
//
// the use of a second function here is mainly to isolate the stack
// costs of lfsr_file_flush and lfsr_file_sync_
//
err = lfsr_file_sync_(lfs, file);
if (err) {
goto failed;
}
// update in-device state // update in-device state
for (lfsr_omdir_t *o = lfs->omdirs; o; o = o->next) { for (lfsr_omdir_t *o = lfs->omdirs; o; o = o->next) {
if (lfsr_o_type(o->flags) == LFS_TYPE_REG if (lfsr_o_type(o->flags) == LFS_TYPE_REG
+7
View File
@@ -274,6 +274,13 @@
#define LFS_FORCEINLINE #define LFS_FORCEINLINE
#endif #endif
// Force a function to _not_ be inlined
#if !defined(LFS_NO_BUILTINS) && defined(__GNUC__)
#define LFS_NOINLINE __attribute__((noinline))
#else
#define LFS_NOINLINE
#endif
// Builtin functions, these may be replaced by more efficient // Builtin functions, these may be replaced by more efficient
// toolchain-specific implementations. LFS_NO_BUILTINS falls back to a more // toolchain-specific implementations. LFS_NO_BUILTINS falls back to a more