trv: Reverted deduplicated mgc related lfs3_fs_fixgrm calls
This fixes a pretty egregious performance regression in bench_wt_many:
NOR throughput before after
bench_wt_seq+write 29405.7 29405.8 (+0.0%)
bench_wt_random+write 957.3 957.3 (+0.0%)
bench_wt_logging+write 2153.4 2153.4 (+0.0%)
bench_wt_many+write 453.6 6855.4 (+1411.3%)
bench_rt_seq+read 23939212.7 23939212.8 (+0.0%)
bench_rt_random+read 6461004.1 6461004.1 (+0.0%)
bench_rt_logging+read 4163.9 4164.0 (+0.0%)
bench_rt_many+read 392380.4 392368.5 (-0.0%)
NAND throughput before after
bench_wt_seq+write 22246.3 22246.3 (+0.0%)
bench_wt_random+write 3637.2 3637.2 (+0.0%)
bench_wt_logging+write 10977.0 10976.4 (-0.0%)
bench_wt_many+write 68.2 448.1 (+557.0%)
bench_rt_seq+read 2472748.7 2472748.8 (+0.0%)
bench_rt_random+read 375952.9 375952.9 (+0.0%)
bench_rt_logging+read 22312.2 22310.9 (-0.0%)
bench_rt_many+read 860.7 896.8 (+4.2%)
But shows our benchmarks are working! I was very confused why littlefs3
was suddenly performing worse than littlefs2 out of seemingly nowhere.
The fundamental problem is that lfs3_mtree_gc unconditionally traverses
the filesystem. So if we call it every lfs3_fs_mkconsistent call, we
trigger a full filesystem traversal on _every_ write operation.
Not great!
The solution for now is to just revert this change. We need the
lfs3_t_ismkconsistent(lfs3->flags) check to avoid the traversal, and if
we don't call lfs3_mtree_gc we also need a check for
lfs3_grm_count(lfs3) > 0.
Not sure there's a better solution here.
---
Code changes minimal, worth fixing:
code stack ctx
before: 35256 2136 660
after: 35292 (+0.1%) 2136 (+0.0%) 660 (+0.0%)
This commit is contained in:
@@ -10179,7 +10179,6 @@ eot:;
|
|||||||
}
|
}
|
||||||
|
|
||||||
// needed in lfs3_mtree_gc
|
// needed in lfs3_mtree_gc
|
||||||
static int lfs3_fs_fixgrm(lfs3_t *lfs3);
|
|
||||||
static int lfs3_mdir_mkconsistent(lfs3_t *lfs3, lfs3_mdir_t *mdir);
|
static int lfs3_mdir_mkconsistent(lfs3_t *lfs3, lfs3_mdir_t *mdir);
|
||||||
static inline void lfs3_alloc_ckpoint_(lfs3_t *lfs3);
|
static inline void lfs3_alloc_ckpoint_(lfs3_t *lfs3);
|
||||||
static inline bool lfs3_alloc_canlookahead(const lfs3_t *lfs3);
|
static inline bool lfs3_alloc_canlookahead(const lfs3_t *lfs3);
|
||||||
@@ -10197,22 +10196,6 @@ static int lfs3_alloc_adoptgbmap(lfs3_t *lfs3,
|
|||||||
// mutation here
|
// mutation here
|
||||||
static lfs3_stag_t lfs3_mtree_gc(lfs3_t *lfs3, lfs3_mgc_t *mgc,
|
static lfs3_stag_t lfs3_mtree_gc(lfs3_t *lfs3, lfs3_mgc_t *mgc,
|
||||||
lfs3_bptr_t *bptr_) {
|
lfs3_bptr_t *bptr_) {
|
||||||
// check for pending grms every step, just in case some other
|
|
||||||
// operation introduced new grms
|
|
||||||
#ifndef LFS3_RDONLY
|
|
||||||
if (lfs3_t_ismkconsistent(mgc->t.h.flags)
|
|
||||||
&& lfs3_grm_count(lfs3) > 0) {
|
|
||||||
// fix pending grms
|
|
||||||
uint32_t dirty = mgc->t.h.flags;
|
|
||||||
int err = lfs3_fs_fixgrm(lfs3);
|
|
||||||
if (err) {
|
|
||||||
return err;
|
|
||||||
}
|
|
||||||
// reset dirty flag
|
|
||||||
mgc->t.h.flags &= ~LFS3_t_DIRTY | dirty;
|
|
||||||
}
|
|
||||||
#endif
|
|
||||||
|
|
||||||
// start of traversal?
|
// start of traversal?
|
||||||
if (mgc->t.h.mdir.mid == LFS3_MID_MROOTANCHOR) {
|
if (mgc->t.h.mdir.mid == LFS3_MID_MROOTANCHOR) {
|
||||||
#ifndef LFS3_RDONLY
|
#ifndef LFS3_RDONLY
|
||||||
@@ -11709,6 +11692,9 @@ empty:;
|
|||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
// needed in lfs3_remove
|
||||||
|
static int lfs3_fs_fixgrm(lfs3_t *lfs3);
|
||||||
|
|
||||||
#ifndef LFS3_RDONLY
|
#ifndef LFS3_RDONLY
|
||||||
int lfs3_remove(lfs3_t *lfs3, const char *path) {
|
int lfs3_remove(lfs3_t *lfs3, const char *path) {
|
||||||
// prepare our filesystem for writing
|
// prepare our filesystem for writing
|
||||||
@@ -16591,15 +16577,9 @@ failed:;
|
|||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
// prepare the filesystem for mutation
|
|
||||||
#ifndef LFS3_RDONLY
|
#ifndef LFS3_RDONLY
|
||||||
int lfs3_fs_mkconsistent(lfs3_t *lfs3) {
|
static int lfs3_fs_fixorphans(lfs3_t *lfs3) {
|
||||||
// filesystem must be writeable
|
// LFS3_T_MKCONSISTENT really just removes orphans
|
||||||
LFS3_ASSERT(!lfs3_m_isrdonly(lfs3->flags));
|
|
||||||
|
|
||||||
// LFS3_T_MKCONSISTENT does most of the work:
|
|
||||||
// 1. fixes pending grms
|
|
||||||
// 2. fixes orphaned stickynotes
|
|
||||||
lfs3_mgc_t mgc;
|
lfs3_mgc_t mgc;
|
||||||
lfs3_mgc_init(&mgc,
|
lfs3_mgc_init(&mgc,
|
||||||
LFS3_T_RDWR | LFS3_T_MTREEONLY | LFS3_T_MKCONSISTENT);
|
LFS3_T_RDWR | LFS3_T_MTREEONLY | LFS3_T_MKCONSISTENT);
|
||||||
@@ -16615,6 +16595,36 @@ int lfs3_fs_mkconsistent(lfs3_t *lfs3) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
#endif
|
||||||
|
|
||||||
|
// prepare the filesystem for mutation
|
||||||
|
#ifndef LFS3_RDONLY
|
||||||
|
int lfs3_fs_mkconsistent(lfs3_t *lfs3) {
|
||||||
|
// filesystem must be writeable
|
||||||
|
LFS3_ASSERT(!lfs3_m_isrdonly(lfs3->flags));
|
||||||
|
|
||||||
|
// fix pending grms
|
||||||
|
if (lfs3_grm_count(lfs3) > 0) {
|
||||||
|
int err = lfs3_fs_fixgrm(lfs3);
|
||||||
|
if (err) {
|
||||||
|
return err;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// fix orphaned stickynotes
|
||||||
|
//
|
||||||
|
// this must happen after fixgrm, since removing orphaned
|
||||||
|
// stickynotes risks outdating the grm
|
||||||
|
//
|
||||||
|
if (lfs3_t_ismkconsistent(lfs3->flags)) {
|
||||||
|
int err = lfs3_fs_fixorphans(lfs3);
|
||||||
|
if (err) {
|
||||||
|
return err;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// go ahead and checkpoint the allocator
|
// go ahead and checkpoint the allocator
|
||||||
//
|
//
|
||||||
// this isn't always needed, but redundant alloc ckpoints are noops,
|
// this isn't always needed, but redundant alloc ckpoints are noops,
|
||||||
@@ -17149,6 +17159,22 @@ int lfs3_trv_read(lfs3_t *lfs3, lfs3_trv_t *trv,
|
|||||||
return LFS3_ERR_BUSY;
|
return LFS3_ERR_BUSY;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// check for pending grms every step, just in case some other
|
||||||
|
// operation introduced new grms
|
||||||
|
#ifndef LFS3_RDONLY
|
||||||
|
if (lfs3_t_ismkconsistent(trv->gc.t.h.flags)
|
||||||
|
&& lfs3_grm_count(lfs3) > 0) {
|
||||||
|
// fix pending grms
|
||||||
|
uint32_t dirty = trv->gc.t.h.flags;
|
||||||
|
int err = lfs3_fs_fixgrm(lfs3);
|
||||||
|
if (err) {
|
||||||
|
return err;
|
||||||
|
}
|
||||||
|
// reset dirty flag
|
||||||
|
trv->gc.t.h.flags &= ~LFS3_t_DIRTY | dirty;
|
||||||
|
}
|
||||||
|
#endif
|
||||||
|
|
||||||
// discard current block queue?
|
// discard current block queue?
|
||||||
if (lfs3_t_isstale(trv->gc.t.h.flags)) {
|
if (lfs3_t_isstale(trv->gc.t.h.flags)) {
|
||||||
trv->blocks[0] = -1;
|
trv->blocks[0] = -1;
|
||||||
|
|||||||
Reference in New Issue
Block a user