Github/littlefs
1
Fork 0
Code Actions Activity

Created an explicit local copy of the mdir's mid in lfsr_mdir_commit

Browse Source 
Knowing C's issues with pointer aliasing, I was wondering if this might
save some code cost by avoiding unnecessary indirect loads of the mid.

But, as is often the case, the compiler is smarter than it first appears:

            code          stack
  before:  21052           1744
  after:   21048 (-0.0%)   1744 (+0.0%)

Still, sometimes an optimization is better when written out explicitly,
so I'll keep this for now.
This commit is contained in:
Christopher Haster
2023-09-10 13:18:52 -05:00
parent ba571cf83f
commit f9bd2c56e3
1 changed files with 28 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+28 -28
View File
@@ -5153,10 +5153,11 @@ compact:;
// //
static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
const lfsr_attr_t *attrs, lfs_size_t attr_count) { const lfsr_attr_t *attrs, lfs_size_t attr_count) {
LFS_ASSERT(mdir->mid == -1 const lfs_ssize_t mid = mdir->mid;
LFS_ASSERT(mid == -1
|| lfsr_mtree_isinlined(lfs) || lfsr_mtree_isinlined(lfs)
|| mdir->u.m.weight > 0); || mdir->u.m.weight > 0);
LFS_ASSERT(mdir->mid == -1 LFS_ASSERT(mid == -1
|| (lfs_size_t)(mdir->mid & lfsr_mridmask(lfs)) || (lfs_size_t)(mdir->mid & lfsr_mridmask(lfs))
<= mdir->u.m.weight); <= mdir->u.m.weight);
@@ -5187,7 +5188,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
// mark the mdir as unerased in case we fail // mark the mdir as unerased in case we fail
lfsr_mdir_unerase(mdir); lfsr_mdir_unerase(mdir);
// and all related copies flying around // and all related copies flying around
if (mdir->mid == -1 || lfsr_mtree_isinlined(lfs)) { if (mid == -1 || lfsr_mtree_isinlined(lfs)) {
lfsr_mdir_unerase(&lfs->mroot); lfsr_mdir_unerase(&lfs->mroot);
} }
for (unsigned type = 0; type < 2; type++) { for (unsigned type = 0; type < 2; type++) {
@@ -5195,7 +5196,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
opened; opened;
opened = opened->next) { opened = opened->next) {
if ((opened->mdir.mid & lfsr_mbidmask(lfs)) if ((opened->mdir.mid & lfsr_mbidmask(lfs))
== (lfs_smax32(mdir->mid, 0) & lfsr_mbidmask(lfs))) { == (lfs_smax32(mid, 0) & lfsr_mbidmask(lfs))) {
lfsr_mdir_unerase(&opened->mdir); lfsr_mdir_unerase(&opened->mdir);
} }
} }
@@ -5211,14 +5212,14 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
// handle possible mtree updates, this gets a bit messy // handle possible mtree updates, this gets a bit messy
lfsr_mdir_t msibling_ = {.u.m.weight=0}; lfsr_mdir_t msibling_ = {.u.m.weight=0};
lfsr_mdir_t mroot_ = (mdir->mid == -1 || lfsr_mtree_isinlined(lfs) lfsr_mdir_t mroot_ = (mid == -1 || lfsr_mtree_isinlined(lfs)
? mdir_ ? mdir_
: lfs->mroot); : lfs->mroot);
lfsr_btree_t mtree_ = lfs->mtree; lfsr_btree_t mtree_ = lfs->mtree;
// need to split? // need to split?
if (err == LFS_ERR_RANGE) { if (err == LFS_ERR_RANGE) {
// this should not happen unless we can't fit our mroot's metadata // this should not happen unless we can't fit our mroot's metadata
LFS_ASSERT(mdir->mid != -1 || lfsr_mtree_isinlined(lfs)); LFS_ASSERT(mid != -1 || lfsr_mtree_isinlined(lfs));
// if we're the mroot, create a new mtree, assume the upper layers // if we're the mroot, create a new mtree, assume the upper layers
// will take care of grafting our mtree into the mroot as needed // will take care of grafting our mtree into the mroot as needed
if (lfsr_mtree_isinlined(lfs)) { if (lfsr_mtree_isinlined(lfs)) {
@@ -5247,7 +5248,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
// compact into new mdir tags < split_rid // compact into new mdir tags < split_rid
int err = lfsr_mdir_alloc(lfs, &mdir_, int err = lfsr_mdir_alloc(lfs, &mdir_,
lfs_smax32(mdir->mid, 0)); lfs_smax32(mid, 0));
if (err) { if (err) {
return err; return err;
} }
@@ -5267,7 +5268,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
// compact into new mdir tags >= split_rid // compact into new mdir tags >= split_rid
err = lfsr_mdir_alloc(lfs, &msibling_, err = lfsr_mdir_alloc(lfs, &msibling_,
lfs_smax32(mdir->mid, 0)); lfs_smax32(mid, 0));
if (err) { if (err) {
return err; return err;
} }
@@ -5292,8 +5293,8 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
"0x{%"PRIx32",%"PRIx32"} " "0x{%"PRIx32",%"PRIx32"} "
"-> 0x{%"PRIx32",%"PRIx32"}, " "-> 0x{%"PRIx32",%"PRIx32"}, "
"0x{%"PRIx32",%"PRIx32"}", "0x{%"PRIx32",%"PRIx32"}",
mdir->mid & lfsr_mbidmask(lfs), mid & lfsr_mbidmask(lfs),
mdir->mid & lfsr_mridmask(lfs), mid & lfsr_mridmask(lfs),
mdir->u.m.blocks[0], mdir->u.m.blocks[1], mdir->u.m.blocks[0], mdir->u.m.blocks[1],
mdir_.u.m.blocks[0], mdir_.u.m.blocks[1], mdir_.u.m.blocks[0], mdir_.u.m.blocks[1],
msibling_.u.m.blocks[0], msibling_.u.m.blocks[1]); msibling_.u.m.blocks[0], msibling_.u.m.blocks[1]);
@@ -5381,8 +5382,8 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
} else if (err == LFS_ERR_NOENT) { } else if (err == LFS_ERR_NOENT) {
LFS_DEBUG("Dropping mdir %"PRId32".%"PRId32" " LFS_DEBUG("Dropping mdir %"PRId32".%"PRId32" "
"0x{%"PRIx32",%"PRIx32"}", "0x{%"PRIx32",%"PRIx32"}",
mdir->mid & lfsr_mbidmask(lfs), mid & lfsr_mbidmask(lfs),
mdir->mid & lfsr_mridmask(lfs), mid & lfsr_mridmask(lfs),
mdir->u.m.blocks[0], mdir->u.m.blocks[1]); mdir->u.m.blocks[0], mdir->u.m.blocks[1]);
// consume gstate so we don't lose any info // consume gstate so we don't lose any info
@@ -5405,11 +5406,11 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
// need to relocate? // need to relocate?
} else if (lfsr_mdir_cmp(mdir, &mdir_) != 0 } else if (lfsr_mdir_cmp(mdir, &mdir_) != 0
&& !(mdir->mid == -1 || lfsr_mtree_isinlined(lfs))) { && !(mid == -1 || lfsr_mtree_isinlined(lfs))) {
LFS_DEBUG("Relocating mdir %"PRId32".%"PRId32" " LFS_DEBUG("Relocating mdir %"PRId32".%"PRId32" "
"0x{%"PRIx32",%"PRIx32"} -> 0x{%"PRIx32",%"PRIx32"}", "0x{%"PRIx32",%"PRIx32"} -> 0x{%"PRIx32",%"PRIx32"}",
mdir->mid & lfsr_mbidmask(lfs), mid & lfsr_mbidmask(lfs),
mdir->mid & lfsr_mridmask(lfs), mid & lfsr_mridmask(lfs),
mdir->u.m.blocks[0], mdir->u.m.blocks[1], mdir->u.m.blocks[0], mdir->u.m.blocks[1],
mdir_.u.m.blocks[0], mdir_.u.m.blocks[1]); mdir_.u.m.blocks[0], mdir_.u.m.blocks[1]);
@@ -5462,14 +5463,14 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
// fix our grm // fix our grm
for (uint8_t j = 0; j < 2; j++) { for (uint8_t j = 0; j < 2; j++) {
if ((grm->rms[j] & lfsr_mbidmask(lfs)) if ((grm->rms[j] & lfsr_mbidmask(lfs))
== (lfs_smax32(mdir->mid, 0) & lfsr_mbidmask(lfs))) { == (lfs_smax32(mid, 0) & lfsr_mbidmask(lfs))) {
if ((grm->rms[j] & lfsr_mridmask(lfs)) if ((grm->rms[j] & lfsr_mridmask(lfs))
>= (lfs_ssize_t)mdir_.u.m.weight) { >= (lfs_ssize_t)mdir_.u.m.weight) {
grm->rms[j] += lfsr_mweight(lfs) grm->rms[j] += lfsr_mweight(lfs)
- mdir_.u.m.weight; - mdir_.u.m.weight;
} }
// update mid if we had a split or drop // update mid if we had a split or drop
} else if (grm->rms[j] > mdir->mid } else if (grm->rms[j] > mid
&& lfsr_btree_weight(&mtree_) && lfsr_btree_weight(&mtree_)
!= lfsr_mtree_weight(lfs)) { != lfsr_mtree_weight(lfs)) {
grm->rms[j] += lfsr_btree_weight(&mtree_) grm->rms[j] += lfsr_btree_weight(&mtree_)
@@ -5668,7 +5669,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
for (lfs_size_t i = 0; i < attr_count; i++) { for (lfs_size_t i = 0; i < attr_count; i++) {
// adjust opened mdirs? // adjust opened mdirs?
if ((opened->mdir.mid & lfsr_mbidmask(lfs)) if ((opened->mdir.mid & lfsr_mbidmask(lfs))
== (lfs_smax32(mdir->mid, 0) & lfsr_mbidmask(lfs)) == (lfs_smax32(mid, 0) & lfsr_mbidmask(lfs))
&& opened->mdir.mid >= attrs[i].rid) { && opened->mdir.mid >= attrs[i].rid) {
// removed? // removed?
if (opened->mdir.mid < attrs[i].rid - attrs[i].delta) { if (opened->mdir.mid < attrs[i].rid - attrs[i].delta) {
@@ -5688,7 +5689,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
((lfsr_dir_t*)opened)->pos += attrs[i].delta; ((lfsr_dir_t*)opened)->pos += attrs[i].delta;
} }
} }
} else if (opened->mdir.mid > mdir->mid) { } else if (opened->mdir.mid > mid) {
// adjust dir position? // adjust dir position?
if (type == LFS_TYPE_DIR) { if (type == LFS_TYPE_DIR) {
((lfsr_dir_t*)opened)->pos += attrs[i].delta; ((lfsr_dir_t*)opened)->pos += attrs[i].delta;
@@ -5698,7 +5699,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
// update any opened mdirs if we had a split or drop // update any opened mdirs if we had a split or drop
if ((opened->mdir.mid & lfsr_mbidmask(lfs)) if ((opened->mdir.mid & lfsr_mbidmask(lfs))
== (lfs_smax32(mdir->mid, 0) & lfsr_mbidmask(lfs))) { == (lfs_smax32(mid, 0) & lfsr_mbidmask(lfs))) {
if (msibling_.u.m.weight > 0 if (msibling_.u.m.weight > 0
&& (opened->mdir.mid & lfsr_mridmask(lfs)) && (opened->mdir.mid & lfsr_mridmask(lfs))
>= (lfs_ssize_t)mdir_.u.m.weight) { >= (lfs_ssize_t)mdir_.u.m.weight) {
@@ -5710,7 +5711,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
} else { } else {
opened->mdir.u.m = mdir_.u.m; opened->mdir.u.m = mdir_.u.m;
} }
} else if (opened->mdir.mid > mdir->mid) { } else if (opened->mdir.mid > mid) {
opened->mdir.mid += lfsr_btree_weight(&mtree_) opened->mdir.mid += lfsr_btree_weight(&mtree_)
- lfsr_mtree_weight(lfs); - lfsr_mtree_weight(lfs);
} }
@@ -5722,8 +5723,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
// TODO clean this up a bit? // TODO clean this up a bit?
// adjust opened mdirs? // adjust opened mdirs?
if ((dir->bookmark & lfsr_mbidmask(lfs)) if ((dir->bookmark & lfsr_mbidmask(lfs))
== (lfs_smax32(mdir->mid, 0) == (lfs_smax32(mid, 0) & lfsr_mbidmask(lfs))
& lfsr_mbidmask(lfs))
&& dir->bookmark >= attrs[i].rid) { && dir->bookmark >= attrs[i].rid) {
// removed? // removed?
if (dir->bookmark < attrs[i].rid - attrs[i].delta) { if (dir->bookmark < attrs[i].rid - attrs[i].delta) {
@@ -5736,14 +5736,14 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
// adjust dir position? // adjust dir position?
dir->pos -= attrs[i].delta; dir->pos -= attrs[i].delta;
} }
} else if (dir->bookmark > mdir->mid) { } else if (dir->bookmark > mid) {
// adjust dir position? // adjust dir position?
dir->pos -= attrs[i].delta; dir->pos -= attrs[i].delta;
} }
} }
if ((dir->bookmark & lfsr_mbidmask(lfs)) if ((dir->bookmark & lfsr_mbidmask(lfs))
== (lfs_smax32(mdir->mid, 0) & lfsr_mbidmask(lfs))) { == (lfs_smax32(mid, 0) & lfsr_mbidmask(lfs))) {
if (msibling_.u.m.weight > 0 if (msibling_.u.m.weight > 0
&& (dir->bookmark & lfsr_mridmask(lfs)) && (dir->bookmark & lfsr_mridmask(lfs))
>= (lfs_ssize_t)mdir_.u.m.weight) { >= (lfs_ssize_t)mdir_.u.m.weight) {
@@ -5752,7 +5752,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
dir->bookmark += lfsr_mweight(lfs) dir->bookmark += lfsr_mweight(lfs)
- mdir_.u.m.weight; - mdir_.u.m.weight;
} }
} else if (dir->bookmark > mdir->mid) { } else if (dir->bookmark > mid) {
dir->bookmark += lfsr_btree_weight(&mtree_) dir->bookmark += lfsr_btree_weight(&mtree_)
- lfsr_mtree_weight(lfs); - lfsr_mtree_weight(lfs);
} }
@@ -5762,9 +5762,9 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
} }
// update mdir to follow requested rid // update mdir to follow requested rid
if (mdir->mid != -1 if (mid != -1
&& msibling_.u.m.weight > 0 && msibling_.u.m.weight > 0
&& (mdir->mid & lfsr_mridmask(lfs)) && (mid & lfsr_mridmask(lfs))
>= (lfs_ssize_t)mdir_.u.m.weight) { >= (lfs_ssize_t)mdir_.u.m.weight) {
LFS_ASSERT(lfsr_btree_weight(&mtree_) != lfsr_mtree_weight(lfs)); LFS_ASSERT(lfsr_btree_weight(&mtree_) != lfsr_mtree_weight(lfs));
mdir->mid += lfsr_mweight(lfs) - mdir_.u.m.weight; mdir->mid += lfsr_mweight(lfs) - mdir_.u.m.weight;