From 507c04db70a6cc034a82794e3c0ee3e5e0ee0312 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Thu, 31 Jul 2025 14:01:19 -0500 Subject: [PATCH] Fixed clobbered shrub estimates when redundantly syncing shrubs Found while benchmarking, our shrub estimates were being recalculated much more frequently than they should be (every shrub commit). The problem is that we never staged shrub estimates! In theory this is fine, shrub estimates don't necessarily need staging. We update shrub.r.eoff/estimate directly in lfs3_bshrub_commitroot_ after the mdir commit succeeds. But then we _redundantly_ sync shrub_ -> shrub.r in lfs3_bshrub_commit. Since we never staged the shrub estimate, we end up with garbage. --- It's not clear to me this (staging the shrub estimate) is the best fix for this, but the reason for the redundant shrub sync is the shared bshrub/btree post-commit path. Added a TODO comment and should look at this again when not in a time crunch. Code changes minimal: code stack ctx before: 36836 2368 684 after: 36840 (+0.0%) 2368 (+0.0%) 684 (+0.0%) --- lfs3.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lfs3.c b/lfs3.c index 03c0ff6a..71897f86 100644 --- a/lfs3.c +++ b/lfs3.c @@ -6907,9 +6907,16 @@ static int lfs3_bshrub_commitroot_(lfs3_t *lfs3, lfs3_bshrub_t *bshrub, && h->mdir.mid == bshrub->h.mdir.mid && lfs3_bshrub_isbshrub((lfs3_bshrub_t*)h)) { ((lfs3_bshrub_t*)h)->shrub.r.eoff = estimate; + // TODO bit of a hack, is this the best way to make sure + // estimate is not clobbered on redundant shrub sync? should + // we instead let eoff/estimate survive staging in mdir + // commit? + ((lfs3_bshrub_t*)h)->shrub_.eoff = estimate; } } LFS3_ASSERT(bshrub->shrub.r.eoff == (lfs3_size_t)estimate); + // note above layers may redundantly sync shrub_ -> shrub + LFS3_ASSERT(bshrub->shrub_.eoff == (lfs3_size_t)estimate); return 0; }