From ae53c326d60e4c7a97611e5603a13a3908ba2b0a Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 20 Jul 2025 12:33:49 -0500 Subject: [PATCH] btree: Limited leaf discarding in mdir commit to shrub roots only What a mouthful. The unconditional bshrub leaf discarding in lfs3_mdir_commit was copied from the previous btree leaf caching implementation, but discarding _all_ bshrub leaves on _every_ mdir commit is a bit insane. Really, the only bshrub leaves that ever need to be discarded here are the shrub roots, which are already questionable leaf caching targets because they're already cached as the root rbyd. An alternative option would be to just never cache shrub roots, but tinkering around with the idea showed it would be more costly that conditionally discarding leaves in lfs3_mdir_commit. At least here we can reuse some of the logic that discards file leaves. I'm also probably overthinking what is only a small code cost: code stack ctx before: 36784 2400 684 after: 36792 (+0.0%) 2400 (+0.0%) 684 (+0.0%) This doesn't take into account how much CPU time is spent creating rbyd copies, but that is not something we are optimizing for. --- lfs3.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/lfs3.c b/lfs3.c index b9f4a665..d60b325f 100644 --- a/lfs3.c +++ b/lfs3.c @@ -9422,27 +9422,29 @@ static int lfs3_mdir_commit(lfs3_t *lfs3, lfs3_mdir_t *mdir, // update any staged bshrubs for (lfs3_handle_t *h = lfs3->handles; h; h = h->next) { - // if we moved a shrub, we also need to discard any related - // leaves that moved - #ifndef LFS3_KVONLY - if (lfs3_o_type(h->flags) == LFS3_TYPE_REG - && lfs3_bptr_block(&((lfs3_file_t*)h)->leaf.bptr) - == ((lfs3_bshrub_t*)h)->shrub.r.blocks[0] - && ((lfs3_bshrub_t*)h)->shrub_.blocks[0] - != ((lfs3_bshrub_t*)h)->shrub.r.blocks[0]) { - lfs3_file_discardleaf((lfs3_file_t*)h); - } - #endif - // update the shrub if (lfs3_o_isbshrub(h->flags)) { + // if we moved a shrub, we also need to discard any leaves + // that moved + if (((lfs3_bshrub_t*)h)->shrub_.blocks[0] + != ((lfs3_bshrub_t*)h)->shrub.r.blocks[0]) { + // discard any bshrub leaves that moved + if (((lfs3_bshrub_t*)h)->shrub.leaf.rbyd.blocks[0] + == ((lfs3_bshrub_t*)h)->shrub.r.blocks[0]) { + lfs3_bshrub_discardleaf((lfs3_bshrub_t*)h); + } + + // discard any file leaves that moved + #ifndef LFS3_KVONLY + if (lfs3_o_type(h->flags) == LFS3_TYPE_REG + && lfs3_bptr_block(&((lfs3_file_t*)h)->leaf.bptr) + == ((lfs3_bshrub_t*)h)->shrub.r.blocks[0]) { + lfs3_file_discardleaf((lfs3_file_t*)h); + } + #endif + } + ((lfs3_bshrub_t*)h)->shrub.r = ((lfs3_bshrub_t*)h)->shrub_; - // TODO do we really need to discard all shrub leaves on - // every mdir commit? shouldn't we just not cache the root? - // (which is already cached!) - // - // discard any leaves that may have moved - lfs3_bshrub_discardleaf((lfs3_bshrub_t*)h); } }