From b09e933e1ef5dcb48c443dc9dc5c353c896fc88b Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Thu, 25 Jan 2024 01:41:53 -0600 Subject: [PATCH] Eagerly discard attr-list in lfsr_mdir_commit__ This is an interesting optimization made possible by our attr-lists now only operating on one mid. We can now discard the entire attr-list based on if that mid is in the commit's filter range. Unfortunately, while I had hoped this would lead to more simplifications, we can't really push this up through many functions: - While this eager discard works for splits, lfsr_btree_commit can also merge, which affects two separate bids. The attrs on these bids need to be split over the new btree inner-nodes, so we end up still needing filtering in lfsr_rbyd_appendattrs. We could move the filtering up into lfsr_btree_commit, but would that really gain anything? - lfsr_mdir_commit_ needs the filter range because we leverage this in higher-layers to for lfsr_mdir_commit_ to omit non--1 attrs when committing to newly hollow mroots. In theory it might still be possible to push this up into lfsr_mdir_commit, but we would still need to unconditionally commit during mdir splits to append the cksum. This ends up with duplicate function calls which ends up annoyingly expensive. Though maybe there is a conditional count trick that could avoid this? At least the code changes are ok: code stack before: 33884 2896 after: 33808 (-0.2%) 2896 (+0.0%) --- lfs.c | 40 ++++++++++++---------------------------- 1 file changed, 12 insertions(+), 28 deletions(-) diff --git a/lfs.c b/lfs.c index b2c9f44a..7908cf10 100644 --- a/lfs.c +++ b/lfs.c @@ -5498,16 +5498,18 @@ static int lfsr_mdir_commit__(lfs_t *lfs, lfsr_mdir_t *mdir, lfsr_smid_t mid, const lfsr_attr_t *attrs, lfs_size_t attr_count) { // try to append a commit lfsr_rbyd_t rbyd_ = mdir->rbyd; - lfsr_srid_t rid = lfsr_mid_rid(lfs, mid); // mark as erased in case of failure mdir->rbyd.eoff = -1; - for (lfs_size_t i = 0; i < attr_count; i++) { - // don't write tags outside of the requested range - if (rid >= start_rid - // note the use of rid+1 and unsigned comparison here to - // treat end_rid=-1 as "unbounded" in such a way that rid=-1 - // is still included - && (lfs_size_t)(rid + 1) <= (lfs_size_t)end_rid) { + + // since we only ever commit to one mid or split, we can ignore the + // entire attr-list if our mid is out of range + lfsr_srid_t rid = lfsr_mid_rid(lfs, mid); + if (rid >= start_rid + // note the use of rid+1 and unsigned comparison here to + // treat end_rid=-1 as "unbounded" in such a way that rid=-1 + // is still included + && (lfs_size_t)(rid + 1) <= (lfs_size_t)end_rid) { + for (lfs_size_t i = 0; i < attr_count; i++) { // adjust for inserts if (!lfsr_tag_isgrow(attrs[i].tag) && attrs[i].delta > 0) { rid -= 1; @@ -5703,26 +5705,8 @@ static int lfsr_mdir_commit__(lfs_t *lfs, lfsr_mdir_t *mdir, } } - // adjust for inserts - if (!lfsr_tag_isgrow(attrs[i].tag) && attrs[i].delta > 0) { - rid += 1; - } - } - - // we need to make sure we keep start_rid/end_rid updated with - // weight changes - if (rid < start_rid) { - start_rid += attrs[i].delta; - } - if (rid < end_rid) { - end_rid += attrs[i].delta; - } - - // adjust rid - rid += attrs[i].delta; - // adjust for inserts - if (!lfsr_tag_isgrow(attrs[i].tag) && attrs[i].delta > 0) { - rid -= 1; + // adjust rid + rid += attrs[i].delta; } }