From 9d0edea7e38e277c2b2220d623b2861124971fd5 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 30 Jul 2023 17:09:59 -0500 Subject: [PATCH] Reworked lfsr_rbyd_estimate to be a bit simpler Instead of reading eagerly and retreating with the hopes of terminating early (which almost never happens when compacting, since we need to find the split_id). lfsr_rbyd_estimate now works inward from the first and last id to find both the dsize and split_id. One thing that helps this is the addition of a separate per-id lfsr_rbyd_estimate, which will be useful for checking if the quantity of file attributes overflows our mdir limitations. lfsr_rbyd_estimate also now ignores the -1 id for split_id calculation, since -1 ids are always cleaned up during splitting, though it does include it in the calculated dsize so that the condition to split is determined correctly. --- This also required rebalance changes. Fortunately, one improvement here is that we can make a simplifying assumption tha the number of tags can't exceed the maximum possible number of tags in the calculated dsize. So worst case, if every tag is empty, the maximum possible dsize becomes 4*(2*log2(dsize/4))+dsize. Though it's still unclear if rebalance is worth keeping. Current comparison: code stack rebalance: 22362 2120 no_rebalance: 21922 (-2.0%) 2120 (+0.0%) --- lfs.c | 326 ++++++++++++++++---------------------------- tests/t3_mtree.toml | 86 +++++++----- 2 files changed, 168 insertions(+), 244 deletions(-) diff --git a/lfs.c b/lfs.c index e400deda..4fe465be 100644 --- a/lfs.c +++ b/lfs.c @@ -3031,7 +3031,7 @@ failed:; // this is a bit of a hack, but ignore any gstate tags here, // these need to be handled specially by upper-layers - if (lfsr_tag_suptype(attrs[i].tag) == LFSR_TAG_GSTATE) { + if (lfsr_tag_suptype(tag) == LFSR_TAG_GSTATE) { continue; } @@ -3229,210 +3229,134 @@ failed:; // the following are mostly btree helpers, but since they operate on rbyds, // exist in the rbyd namespace -// determine if a given rbyd will be within the compaction threshold (1/2) -// after compaction, note this uses a conservative estimate so the actual -// on-disk cost may be smaller +// Calculate the maximum possible disk usage required by this id after +// compaction. This uses a conservative estimate so the actual on-disk cost +// should be smaller. // -// if rbyd does not fit, a good split_id is returned -static int lfsr_rbyd_estimate(lfs_t *lfs, const lfsr_rbyd_t *rbyd, - lfs_ssize_t start_id, lfs_ssize_t end_id, lfs_size_t threshold, - lfs_size_t *split_id_) { -#ifndef LFSR_NO_REBALANCE - lfs_size_t dsize = 0; - - lfs_ssize_t id = start_id; +static lfs_ssize_t lfsr_rbyd_estimate(lfs_t *lfs, const lfsr_rbyd_t *rbyd, + lfs_ssize_t id, + lfs_ssize_t *id_, lfs_size_t *weight_) { lfsr_tag_t tag = 0; + lfs_size_t w = 0; + lfs_size_t dsize = 0; while (true) { - lfs_size_t w; + lfs_ssize_t id__; + lfs_size_t w_; lfsr_data_t data; - int err = lfsr_rbyd_lookupnext(lfs, rbyd, id, lfsr_tag_next(tag), - &id, &tag, &w, &data); + int err = lfsr_rbyd_lookupnext(lfs, rbyd, + id, lfsr_tag_next(tag), + &id__, &tag, &w_, &data); if (err && err != LFS_ERR_NOENT) { return err; } - if (err == LFS_ERR_NOENT || (end_id >= 0 && id >= end_id)) { - return true; + if (err == LFS_ERR_NOENT || id__ > id+lfs_smax32(w_-1, 0)) { + break; } + // keep track of id and weight + id = id__; + w += w_; + // determine the upper-bound of alt pointers, tags, and data // after compaction // // note that with rebalancing during compaction, we know the number // of inner nodes is the same as the number of tags. Each node has // two alts and is terminated by a 4-byte null tag. - dsize += LFSR_TAG_DSIZE + lfsr_data_size(data) - + 2*LFSR_TAG_DSIZE + 4; - - // exceeded our compaction threshold? - if (dsize > threshold) { - // requested a split id? - if (split_id_) { - // TODO is this really worth it vs a simpler algorithm? - // - // here we ignore the cost of alt-pointers, and only use the - // tag+data cost as a heuristic - // - // we assume we already found an over-estimate of the split id - // so we only need to work backwards through the rbyd to - // correct the over-estimate. This is a very minor optimization. - // - lfs_size_t lower_id = id+1; - lfs_ssize_t upper_id = rbyd->weight-1; - lfs_size_t lower_dsize = dsize; - lfs_size_t upper_dsize = 0; - while (true) { - lfsr_tag_t tag = 0; - lfs_size_t w = 0; - lfs_size_t dsize = 0; - while (true) { - lfs_ssize_t id_; - lfs_size_t w_; - lfsr_data_t data; - int err = lfsr_rbyd_lookupnext(lfs, rbyd, - upper_id, lfsr_tag_next(tag), - &id_, &tag, &w_, &data); - if (err && err != LFS_ERR_NOENT) { - return err; - } - if (err == LFS_ERR_NOENT || id_ != upper_id) { - break; - } - - // keep track of weight to iterate backwards - w += w_; - - // determine the upper-bound of alt pointers, tags, - // and data after compaction (same as above) - dsize += LFSR_TAG_DSIZE + lfsr_data_size(data) - + 2*LFSR_TAG_DSIZE + 4; - } - - // steal dsize from lower_dsize if we start overlapping - if ((lfs_size_t)upper_id-(w-1) < lower_id) { - lower_id = upper_id-(w-1); - lower_dsize -= dsize; - } - upper_dsize += dsize; - - // done when upper/lower dsizes are close to balanced - if (upper_dsize >= lower_dsize) { - break; - } - - // iterate backwards - upper_id -= w; - } - - LFS_ASSERT(lower_id < rbyd->weight); - *split_id_ = lower_id; - } - - return false; - } - } - -#else - lfs_size_t count = 0; - lfs_size_t dsize = 0; - lfs_size_t real_dsize = sizeof(uint32_t); - - lfs_ssize_t id = start_id; - lfsr_tag_t tag = 0; - while (true) { - lfs_size_t w; - lfsr_data_t data; - int err = lfsr_rbyd_lookupnext(lfs, rbyd, id, lfsr_tag_next(tag), - &id, &tag, &w, &data); - if (err && err != LFS_ERR_NOENT) { - return err; - } - if (err == LFS_ERR_NOENT) { - return true; - } - - // Exhibit A. Why I really didn't want to estimate the rbyd threshold: - - // keep track of alt-less tag count and dsize // - // this is used as a heuristic for split, so the exactness matters less, - // but we need to be able to subtract tags from the result so we can't - // use the estimate with alt pointers - count += 1; + #ifndef LFSR_NO_REBALANCE + dsize += 2*LFSR_TAG_DSIZE + 4 + + LFSR_TAG_DSIZE + lfsr_data_size(data); + #else + // TODO is this the best way to do this? + // If we're not rebalancing, we just don't account for the alts. We + // need to know the total size to know how many alts there are, so + // just leave this up to upper layers dsize += LFSR_TAG_DSIZE + lfsr_data_size(data); - // determine the upper-bound of our alt pointers, tag, and data - // - // fortunately the self-balancing nature of rybds give us a tight - // bound on the number of alt pointers - real_dsize - += (2*lfs_nlog2(count)+1) * LFSR_TAG_DSIZE - + LFSR_TAG_DSIZE - + lfsr_data_size(data); + #endif + } - // exceeded our compaction threshold? - if (real_dsize > threshold) { - // requested a split id? - if (split_id_) { - // TODO is this really worth it vs a simpler algorithm? - // - // here we ignore the cost of alt-pointers, and only use the - // tag+data cost as a heuristic - // - // we assume we already found an over-estimate of the split id - // so we only need to work backwards through the rbyd to - // correct the over-estimate. This is a very minor optimization. - // - lfs_size_t lower_id = id+1; - lfs_ssize_t upper_id = rbyd->weight-1; - lfs_size_t lower_dsize = dsize; - lfs_size_t upper_dsize = 0; - while (true) { - lfsr_tag_t tag = 0; - lfs_size_t w = 0; - lfs_size_t dsize = 0; - while (true) { - lfs_ssize_t id_; - lfs_size_t w_; - lfsr_data_t data; - int err = lfsr_rbyd_lookupnext(lfs, rbyd, - upper_id, lfsr_tag_next(tag), - &id_, &tag, &w_, &data); - if (err && err != LFS_ERR_NOENT) { - return err; - } - if (err == LFS_ERR_NOENT || id_ != upper_id) { - break; - } + if (id_) { + *id_ = id; + } + if (weight_) { + *weight_ = w; + } + return dsize; +} - // keep track of weight to iterate backwards - w += w_; +// Calculate the maximum possible disk usage required by this id after +// compaction. This uses a conservative estimate so the actual on-disk cost +// should be smaller. +// +// This also returns a good split_id in case the rbyd needs to be split. +// +// TODO do we need to include commit overhead here? +static lfs_ssize_t lfsr_rbyd_estimateall(lfs_t *lfs, const lfsr_rbyd_t *rbyd, + lfs_ssize_t start_id, lfs_ssize_t end_id, + lfs_size_t *split_id_) { + // calculate dsize by starting from the outside ids and working inwards, + // this naturally gives us a split id + // + // note that we don't include -1 tags yet, -1 tags are always cleaned up + // during a split so they shouldn't affect the split_id + // + lfs_ssize_t lower_id = (start_id < 0 ? 0 : start_id); + lfs_ssize_t upper_id = (end_id < 0 + ? (lfs_ssize_t)rbyd->weight-1 + : end_id-1); + lfs_size_t lower_dsize = 0; + lfs_size_t upper_dsize = 0; - // assume worst-case encoding size - dsize += LFSR_TAG_DSIZE + lfsr_data_size(data); - } - - // steal dsize from lower_dsize if we start overlapping - if ((lfs_size_t)upper_id-(w-1) < lower_id) { - lower_id = upper_id-(w-1); - lower_dsize -= dsize; - } - upper_dsize += dsize; - - // done when upper/lower dsizes are close to balanced - if (upper_dsize >= lower_dsize) { - break; - } - - // iterate backwards - upper_id -= w; - } - - LFS_ASSERT(lower_id < rbyd->weight); - *split_id_ = lower_id; + while (lower_id <= upper_id) { + if (lower_dsize <= upper_dsize) { + lfs_size_t w; + lfs_ssize_t dsize = lfsr_rbyd_estimate(lfs, rbyd, lower_id, + NULL, &w); + if (dsize < 0) { + return dsize; } - return false; + lower_id += w; + lower_dsize += dsize; + } else { + lfs_size_t w; + lfs_ssize_t dsize = lfsr_rbyd_estimate(lfs, rbyd, upper_id, + NULL, &w); + if (dsize < 0) { + return dsize; + } + + upper_id -= w; + upper_dsize += dsize; } } + + // include -1 tags in our final dsize + lfs_ssize_t dsize = lfsr_rbyd_estimate(lfs, rbyd, -1, NULL, NULL); + if (dsize < 0) { + return dsize; + } + + if (split_id_) { + *split_id_ = lower_id; + } + +#ifndef LFSR_NO_REBALANCE + return dsize + lower_dsize + upper_dsize; +#else + // TODO if we are serious about providing a LFSR_NO_REBALANCE option, we + // should probably also do this in lfsr_rbyd_estimate, though that raises + // the question how should lfsr_rbyd_estimate/estimateall interact in that + // case? + // + // If we're not rebalancing, we need to account for the overhead of + // intermediary trunks, which is O(log(n)) per trunk. + // + // Assuming worst case all tags are zero-length, and tags are at minimum + // 4 bytes, the worst case total is 4*(2*log2(dsize/4)) + dsize + dsize = dsize + lower_dsize + upper_dsize; + return 4*(2*lfs_nlog2(dsize/4)) + dsize; #endif } @@ -4053,14 +3977,13 @@ static int lfsr_btree_commit(lfs_t *lfs, // check if we're within our compaction threshold, otherwise we // need to split - int fits = lfsr_rbyd_estimate(lfs, rbyd, -1, -1, - lfs->cfg->block_size/2, + lfs_ssize_t estimate = lfsr_rbyd_estimateall(lfs, rbyd, -1, -1, &split_id); - if (fits < 0) { - return fits; + if (estimate < 0) { + return estimate; } - if (!fits) { + if ((lfs_size_t)estimate > lfs->cfg->block_size/2) { // need to split goto split; } @@ -4360,15 +4283,14 @@ static int lfsr_btree_commit(lfs_t *lfs, // this is imprecise when not compacting, so we may still fail to // merge, but this at least lets us avoid wasting programming cycles // when merge failure is obvious - int fits = lfsr_rbyd_estimate(lfs, &sibling, -1, -1, - lfs->cfg->block_size/4, + lfs_ssize_t estimate = lfsr_rbyd_estimateall(lfs, &sibling, -1, -1, NULL); - if (fits < 0) { - return fits; + if (estimate < 0) { + return estimate; } // don't fit? can't merge - if (!fits) { + if ((lfs_size_t)estimate > lfs->cfg->block_size/4) { continue; } @@ -5633,15 +5555,15 @@ compact:; // can't commit, try to compact // check if we're within our compaction threshold - int fits = lfsr_rbyd_estimate(lfs, &mdir->rbyd, start_id, end_id, - lfs->cfg->block_size/2, + lfs_ssize_t estimate = lfsr_rbyd_estimateall(lfs, &mdir->rbyd, + start_id, end_id, split_id_); - if (fits < 0) { - return fits; + if (estimate < 0) { + return estimate; } - // TODO change lfsr_rbyd_estimate so !fits => err=LFS_ERR_RANGE? - if (!fits) { + // TODO do we need to include mdir commit overhead here? in rbyd_estimate? + if ((lfs_size_t)estimate > lfs->cfg->block_size/2) { return LFS_ERR_RANGE; } @@ -5887,16 +5809,6 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid, mroot_ = mdir_; } - // TODO should lfsr_rbyd_estimate just always do this? - // find estimate again, this time with start_id=0 so we - // ignore any -1 attrs - int fits = lfsr_rbyd_estimate(lfs, &mdir->rbyd, 0, -1, - lfs->cfg->block_size/2, - &split_id); - if (fits < 0) { - return fits; - } - // let lfsr_mtree_split_ do most of the work err = lfsr_mtree_split_(lfs, &mtree_, &mdir_, &msibling_, 0, -1, diff --git a/tests/t3_mtree.toml b/tests/t3_mtree.toml index 5863e2fd..9a490f24 100644 --- a/tests/t3_mtree.toml +++ b/tests/t3_mtree.toml @@ -2834,7 +2834,7 @@ code = ''' lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, NULL, 0) => 0; // assert mdir was unininlined correctly - assert(lfsr_mtree_weight(&lfs) == 1); + assert(lfsr_mtree_weight(&lfs) == 2); // assert mroot now has no entries assert(lfs.mroot.rbyd.weight == 0); @@ -2846,19 +2846,24 @@ code = ''' // assert that our entry is still in the mtree lfsr_mdir_t mdir; lfsr_mtree_lookup(&lfs, 0, &mdir) => 0; - assert(mdir.rbyd.weight == 3); - + assert(mdir.rbyd.weight == 2); lfsr_mdir_get(&lfs, &mdir, 1, LFSR_TAG_INLINED, buffer, SIZE) => SIZE; assert(memcmp(buffer, &alphas[3 % 26], 1) == 0); + // note that our current implementation splits here, which is suboptimal + // but saves on code size + lfsr_mdir_t msibling; + lfsr_mtree_lookup(&lfs, 1, &msibling) => 0; + assert(msibling.rbyd.weight == 1); + // assert that our neighbors were updated correctly assert(left_neighbor.rid == 0); assert(left_neighbor.mdir.mid == 0); assert(memcmp(&left_neighbor.mdir, &mdir, sizeof(lfsr_mdir_t)) == 0); - assert(right_neighbor.rid == 2); - assert(right_neighbor.mdir.mid == 0); - assert(memcmp(&right_neighbor.mdir, &mdir, sizeof(lfsr_mdir_t)) == 0); + assert(right_neighbor.rid == 0); + assert(right_neighbor.mdir.mid == 1); + assert(memcmp(&right_neighbor.mdir, &msibling, sizeof(lfsr_mdir_t)) == 0); lfsr_mdir_removeopened(&lfs, LFS_TYPE_REG, &left_neighbor); lfsr_mdir_removeopened(&lfs, LFS_TYPE_REG, &right_neighbor); @@ -2952,18 +2957,6 @@ code = ''' lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS( LFSR_ATTR(0, UNR, -1, NULL, 0))) => 0; - // setup our neighbors - lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS( - LFSR_ATTR(0, INLINED, +1, &alphas[0 % 26], 1), - LFSR_ATTR(1, INLINED, +1, &alphas[1 % 26], 1))) => 0; - // this test only works if these all fit in the mroot - assert(lfsr_mtree_isinlined(&lfs)); - - lfsr_openedmdir_t left_neighbor = {.rid=0, .mdir=lfs.mroot}; - lfsr_openedmdir_t right_neighbor = {.rid=1, .mdir=lfs.mroot}; - lfsr_mdir_addopened(&lfs, LFS_TYPE_REG, &left_neighbor); - lfsr_mdir_addopened(&lfs, LFS_TYPE_REG, &right_neighbor); - // create an uninlined mdir uint8_t buffer[SIZE]; memset(buffer, alphas[2 % 26], SIZE); @@ -2972,7 +2965,7 @@ code = ''' memset(buffer, alphas[3 % 26], SIZE); lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS( - LFSR_ATTR(1, INLINED, +1, buffer, SIZE))) => 0; + LFSR_ATTR(0, INLINED, +1, buffer, SIZE))) => 0; // force mroot to compact lfs.mroot.rbyd.off = BLOCK_SIZE; @@ -2983,11 +2976,27 @@ code = ''' // assert mroot now has no entries assert(lfs.mroot.rbyd.weight == 0); - // now add another large entry to the mdir, forcing a split + // setup our neighbors + // + // note we do this after uninlining! this is because uninlining may + // aggresively split the mtree if there are already neighbors in the mdir lfsr_mdir_t mdir; lfsr_mtree_lookup(&lfs, 0, &mdir) => 0; + assert(mdir.rbyd.weight == 1); + + lfsr_mdir_commit(&lfs, &mdir, &(lfs_ssize_t){0}, LFSR_ATTRS( + LFSR_ATTR(0, INLINED, +1, &alphas[0 % 26], 1), + LFSR_ATTR(2, INLINED, +1, &alphas[1 % 26], 1))) => 0; + // this test only works if these all fit in the mdir + assert(lfsr_mtree_weight(&lfs) == 1); assert(mdir.rbyd.weight == 3); + lfsr_openedmdir_t left_neighbor = {.rid=0, .mdir=mdir}; + lfsr_openedmdir_t right_neighbor = {.rid=2, .mdir=mdir}; + lfsr_mdir_addopened(&lfs, LFS_TYPE_REG, &left_neighbor); + lfsr_mdir_addopened(&lfs, LFS_TYPE_REG, &right_neighbor); + + // now add another large entry to the mdir, forcing a split memset(buffer, alphas[4 % 26], SIZE); lfsr_mdir_commit(&lfs, &mdir, &(lfs_ssize_t){2}, LFSR_ATTRS( LFSR_ATTR(2, INLINED, +1, buffer, SIZE))) => 0; @@ -3114,43 +3123,46 @@ code = ''' lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS( LFSR_ATTR(0, UNR, -1, NULL, 0))) => 0; - // setup our neighbors - lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS( - LFSR_ATTR(0, INLINED, +1, &alphas[0 % 26], 1), - LFSR_ATTR(1, INLINED, +1, &alphas[1 % 26], 1))) => 0; - // this test only works if these all fit in the mroot - assert(lfsr_mtree_isinlined(&lfs)); - - lfsr_openedmdir_t left_neighbor = {.rid=0, .mdir=lfs.mroot}; - lfsr_openedmdir_t right_neighbor = {.rid=1, .mdir=lfs.mroot}; - lfsr_mdir_addopened(&lfs, LFS_TYPE_REG, &left_neighbor); - lfsr_mdir_addopened(&lfs, LFS_TYPE_REG, &right_neighbor); - - // prepare mroot with a large attr so the next entry can not fit + // create an uninlined mdir uint8_t buffer[SIZE]; memset(buffer, alphas[2 % 26], SIZE); lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS( LFSR_ATTR(-1, UATTR(1), 0, buffer, SIZE))) => 0; - // create a large entry that needs to be uninlined (but not split!) memset(buffer, alphas[3 % 26], SIZE); lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS( - LFSR_ATTR(1, INLINED, +1, buffer, SIZE))) => 0; + LFSR_ATTR(0, INLINED, +1, buffer, SIZE))) => 0; // force mroot to compact lfs.mroot.rbyd.off = BLOCK_SIZE; lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, NULL, 0) => 0; - // assert mtree has one mdir + // assert mdir was unininlined correctly assert(lfsr_mtree_weight(&lfs) == 1); // assert mroot now has no entries assert(lfs.mroot.rbyd.weight == 0); - // force mdir to compact twice, this should relocate + // setup our neighbors + // + // note we do this after uninlining! this is because uninlining may + // aggresively split the mtree if there are already neighbors in the mdir lfsr_mdir_t mdir; lfsr_mtree_lookup(&lfs, 0, &mdir) => 0; + assert(mdir.rbyd.weight == 1); + + lfsr_mdir_commit(&lfs, &mdir, &(lfs_ssize_t){0}, LFSR_ATTRS( + LFSR_ATTR(0, INLINED, +1, &alphas[0 % 26], 1), + LFSR_ATTR(2, INLINED, +1, &alphas[1 % 26], 1))) => 0; + // this test only works if these all fit in the mdir + assert(lfsr_mtree_weight(&lfs) == 1); assert(mdir.rbyd.weight == 3); + lfsr_openedmdir_t left_neighbor = {.rid=0, .mdir=mdir}; + lfsr_openedmdir_t right_neighbor = {.rid=2, .mdir=mdir}; + lfsr_mdir_addopened(&lfs, LFS_TYPE_REG, &left_neighbor); + lfsr_mdir_addopened(&lfs, LFS_TYPE_REG, &right_neighbor); + + // force mdir to compact twice, this should relocate lfsr_mdir_t old_mdir = mdir; mdir.rbyd.off = BLOCK_SIZE;