Github/littlefs
1
Fork 0
Code Actions Activity

t: Dropped mtinfo/btinfo, just use data/bptr for everything

Browse Source 
It's probably a bad reason, but this avoids wasting too much time
figuring out how to name things.

Now most traversal functions return an lfsr_tag_t + lfsr_bptr_t pair,
which is enough to describe the current relevant traversal objects:

  tag=LFSR_TAG_MDIR   => (lfsr_mdir_t*)bptr.data.u.buffer
  tag=LFSR_TAG_BRANCH => (lfsr_rbyd_t*)bptr.data.u.buffer
  tag=LFSR_TAG_DATA   => bptr.data
  tag=LFSR_TAG_BPTR   => bptr

This would be a bit better if lfsr_data_t's buffer field was a void*,
but that would mess with byte-level arithmetic, which is more common
with lfsr_data_ts.

This also adopts the fragmented/optional out-params used elsewhere in
the codebase. I thought this would add quite a bit more stack cost,
since we need redundant tags/bptrs to make lfsr_mtree_traverse/
lfsr_mtree_gc work, but surprisingly not:

           code          stack
  before: 35256           2680
  after:  35228 (-0.1%)   2680 (+0.0%)

It seems we make up the extra stack cost of redundant tags/bptrs by
giving the compiler more stack-alloc flexibility, tighter per-function
return types, and opting-out of tags/bptrs in most low-level traversals:
lfs_alloc mainly.

But if the fragmented/optional out-params is net harmful for code/stack
size, we should reconsider the pattern system-wide. This does probably
deserve a second look in the future...
This commit is contained in:
Christopher Haster
2024-07-05 14:54:38 -05:00
parent 96834c2460
commit 2e6a5be4e3
6 changed files with 486 additions and 384 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_traversal.toml
+80 -60
View File
@@ -6077,12 +6077,14 @@ code = '''
// we need internals to check this
// ckmeta needed for eoff
lfsr_mtraversal_t mt = LFSR_MTRAVERSAL(LFS_T_CKMETA);
lfsr_mtinfo_t mtinfo;
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_MDIR);
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_BRANCH);
if (lfsr_rbyd_eoff(mtinfo.u.rbyd) > GC_COMPACT_THRESH) {
lfsr_tag_t tag;
lfsr_bptr_t bptr;
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_MDIR);
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_BRANCH);
if (lfsr_rbyd_eoff((lfsr_rbyd_t*)bptr.data.u.buffer)
> GC_COMPACT_THRESH) {
break;
}
@@ -6135,12 +6137,14 @@ code = '''
// mtree should have been compacted
lfsr_mtraversal_t mt = LFSR_MTRAVERSAL(LFS_T_CKMETA);
lfsr_mtinfo_t mtinfo;
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_MDIR);
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_BRANCH);
assert(lfsr_rbyd_eoff(mtinfo.u.rbyd) <= GC_COMPACT_THRESH);
lfsr_tag_t tag;
lfsr_bptr_t bptr;
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_MDIR);
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_BRANCH);
assert(lfsr_rbyd_eoff((lfsr_rbyd_t*)bptr.data.u.buffer)
<= GC_COMPACT_THRESH);
// check we can still read the files
for (int remount = 0; remount < 2; remount++) {
@@ -6555,12 +6559,14 @@ code = '''
// we need internals to check this
// ckmeta needed for eoff
lfsr_mtraversal_t mt = LFSR_MTRAVERSAL(LFS_T_CKMETA);
lfsr_mtinfo_t mtinfo;
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_MDIR);
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_BRANCH);
if (lfsr_rbyd_eoff(mtinfo.u.rbyd) > GC_COMPACT_THRESH) {
lfsr_tag_t tag;
lfsr_bptr_t bptr;
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_MDIR);
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_BRANCH);
if (lfsr_rbyd_eoff((lfsr_rbyd_t*)bptr.data.u.buffer)
> GC_COMPACT_THRESH) {
break;
}
@@ -6600,17 +6606,19 @@ code = '''
// bshrub should have been compacted
lfsr_mtraversal_t mt = LFSR_MTRAVERSAL(LFS_T_CKMETA);
lfsr_mtinfo_t mtinfo;
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_MDIR);
lfsr_tag_t tag;
lfsr_bptr_t bptr;
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_MDIR);
while (true) {
int err = lfsr_mtree_traverse(&lfs, &mt, &mtinfo);
int err = lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr);
assert(!err || err == LFS_ERR_NOENT);
if (err == LFS_ERR_NOENT) {
break;
}
assert(mtinfo.tag == LFSR_TAG_BRANCH);
assert(lfsr_rbyd_eoff(mtinfo.u.rbyd) <= GC_COMPACT_THRESH);
assert(tag == LFSR_TAG_BRANCH);
assert(lfsr_rbyd_eoff((lfsr_rbyd_t*)bptr.data.u.buffer)
<= GC_COMPACT_THRESH);
}
// check we can still read the file
@@ -6668,12 +6676,14 @@ code = '''
// we need internals to check this
// ckmeta needed for eoff
lfsr_mtraversal_t mt = LFSR_MTRAVERSAL(LFS_T_CKMETA);
lfsr_mtinfo_t mtinfo;
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_MDIR);
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_BRANCH);
if (lfsr_rbyd_eoff(mtinfo.u.rbyd) > GC_COMPACT_THRESH) {
lfsr_tag_t tag;
lfsr_bptr_t bptr;
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_MDIR);
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_BRANCH);
if (lfsr_rbyd_eoff((lfsr_rbyd_t*)bptr.data.u.buffer)
> GC_COMPACT_THRESH) {
break;
}
@@ -6713,17 +6723,19 @@ code = '''
// bshrub should have been compacted
lfsr_mtraversal_t mt = LFSR_MTRAVERSAL(LFS_T_CKMETA);
lfsr_mtinfo_t mtinfo;
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_MDIR);
lfsr_tag_t tag;
lfsr_bptr_t bptr;
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_MDIR);
while (true) {
int err = lfsr_mtree_traverse(&lfs, &mt, &mtinfo);
int err = lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr);
assert(!err || err == LFS_ERR_NOENT);
if (err == LFS_ERR_NOENT) {
break;
}
assert(mtinfo.tag == LFSR_TAG_BRANCH);
assert(lfsr_rbyd_eoff(mtinfo.u.rbyd) <= GC_COMPACT_THRESH);
assert(tag == LFSR_TAG_BRANCH);
assert(lfsr_rbyd_eoff((lfsr_rbyd_t*)bptr.data.u.buffer)
<= GC_COMPACT_THRESH);
}
// check we can still read the file
@@ -6782,12 +6794,14 @@ code = '''
// we need internals to check this
// ckmeta needed for eoff
lfsr_mtraversal_t mt = LFSR_MTRAVERSAL(LFS_T_CKMETA);
lfsr_mtinfo_t mtinfo;
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_MDIR);
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_BRANCH);
if (lfsr_rbyd_eoff(mtinfo.u.rbyd) > GC_COMPACT_THRESH) {
lfsr_tag_t tag;
lfsr_bptr_t bptr;
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_MDIR);
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_BRANCH);
if (lfsr_rbyd_eoff((lfsr_rbyd_t*)bptr.data.u.buffer)
> GC_COMPACT_THRESH) {
break;
}
@@ -6825,17 +6839,19 @@ code = '''
// bshrub should have been compacted
lfsr_mtraversal_t mt = LFSR_MTRAVERSAL(LFS_T_CKMETA);
lfsr_mtinfo_t mtinfo;
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_MDIR);
lfsr_tag_t tag;
lfsr_bptr_t bptr;
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_MDIR);
while (true) {
int err = lfsr_mtree_traverse(&lfs, &mt, &mtinfo);
int err = lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr);
assert(!err || err == LFS_ERR_NOENT);
if (err == LFS_ERR_NOENT) {
break;
}
assert(mtinfo.tag == LFSR_TAG_BRANCH);
assert(lfsr_rbyd_eoff(mtinfo.u.rbyd) <= GC_COMPACT_THRESH);
assert(tag == LFSR_TAG_BRANCH);
assert(lfsr_rbyd_eoff((lfsr_rbyd_t*)bptr.data.u.buffer)
<= GC_COMPACT_THRESH);
}
// check we can still read the file
@@ -6894,12 +6910,14 @@ code = '''
// we need internals to check this
// ckmeta needed for eoff
lfsr_mtraversal_t mt = LFSR_MTRAVERSAL(LFS_T_CKMETA);
lfsr_mtinfo_t mtinfo;
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_MDIR);
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_BRANCH);
if (lfsr_rbyd_eoff(mtinfo.u.rbyd) > GC_COMPACT_THRESH) {
lfsr_tag_t tag;
lfsr_bptr_t bptr;
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_MDIR);
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_BRANCH);
if (lfsr_rbyd_eoff((lfsr_rbyd_t*)bptr.data.u.buffer)
> GC_COMPACT_THRESH) {
break;
}
@@ -6958,17 +6976,19 @@ code = '''
// bshrub should have been compacted
lfsr_mtraversal_t mt = LFSR_MTRAVERSAL(LFS_T_CKMETA);
lfsr_mtinfo_t mtinfo;
lfsr_mtree_traverse(&lfs, &mt, &mtinfo) => 0;
assert(mtinfo.tag == LFSR_TAG_MDIR);
lfsr_tag_t tag;
lfsr_bptr_t bptr;
lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr) => 0;
assert(tag == LFSR_TAG_MDIR);
while (true) {
int err = lfsr_mtree_traverse(&lfs, &mt, &mtinfo);
int err = lfsr_mtree_traverse(&lfs, &mt, &tag, &bptr);
assert(!err || err == LFS_ERR_NOENT);
if (err == LFS_ERR_NOENT) {
break;
}
assert(mtinfo.tag == LFSR_TAG_BRANCH);
assert(lfsr_rbyd_eoff(mtinfo.u.rbyd) <= GC_COMPACT_THRESH);
assert(tag == LFSR_TAG_BRANCH);
assert(lfsr_rbyd_eoff((lfsr_rbyd_t*)bptr.data.u.buffer)
<= GC_COMPACT_THRESH);
}
// check we can still read the files