Changed lfsr_rbyd/btree/bshrub_commit to _not_ be atomic, adopted more

Now that error recovery is well defined (at least in theory), and
high-level mdir/file functions create on-stack copies, the on-stack
copies for the low-level rbyd/btree/bshrub commit functions are
redundant and not useful.

Dropping the redundant on-stack copies in low-level functions saves a
bit for stack usage.

Additionally, we can adopt lfsr_rbyd_commit in more places where it was
avoided to avoid even more redundant on-stack copies.

            code          stack
  before:  31676           2776
  after:   31596 (-0.3%)   2752 (-0.9%)
This commit is contained in:
Christopher Haster
2023-12-06 22:49:06 -06:00
parent 3a6afaf1c5
commit 7e9c0fbd88
+14 -46
View File
@@ -3173,26 +3173,19 @@ static int lfsr_rbyd_appendattrs(lfs_t *lfs, lfsr_rbyd_t *rbyd,
static int lfsr_rbyd_commit(lfs_t *lfs, lfsr_rbyd_t *rbyd, static int lfsr_rbyd_commit(lfs_t *lfs, lfsr_rbyd_t *rbyd,
const lfsr_attr_t *attrs, lfs_size_t attr_count) { const lfsr_attr_t *attrs, lfs_size_t attr_count) {
// create a copy and mark rbyd as unerased in case of failure
lfsr_rbyd_t rbyd_ = *rbyd;
rbyd->eoff = -1;
// append each tag to the tree // append each tag to the tree
for (lfs_size_t i = 0; i < attr_count; i++) { int err = lfsr_rbyd_appendattrs(lfs, rbyd, -1, -1,
int err = lfsr_rbyd_appendattr(lfs, &rbyd_, attrs[i].rid, attrs, attr_count);
attrs[i].tag, attrs[i].delta, attrs[i].data); if (err) {
if (err) { return err;
return err; }
}
} // append a cksum, finalizing the commit
err = lfsr_rbyd_appendcksum(lfs, rbyd);
// append a cksum, finalizing the commit
int err = lfsr_rbyd_appendcksum(lfs, &rbyd_);
if (err) { if (err) {
return err; return err;
} }
*rbyd = rbyd_;
return 0; return 0;
} }
@@ -4487,29 +4480,16 @@ static int lfsr_btree_commit(lfs_t *lfs, lfsr_btree_t *btree,
// needs a new root? // needs a new root?
if (attr_count > 0) { if (attr_count > 0) {
// TODO do we need to be this careful with backup copies? err = lfsr_rbyd_alloc(lfs, btree);
lfsr_rbyd_t rbyd;
err = lfsr_rbyd_alloc(lfs, &rbyd);
if (err) { if (err) {
return err; return err;
} }
// TODO should we just use rbyd commit? it allocates _another_ err = lfsr_rbyd_commit(lfs, btree, attrs, attr_count);
// redundant copy which is a bit much...
err = lfsr_rbyd_appendattrs(lfs, &rbyd, -1, -1,
attrs, attr_count);
if (err) { if (err) {
LFS_ASSERT(err != LFS_ERR_RANGE); LFS_ASSERT(err != LFS_ERR_RANGE);
return err; return err;
} }
err = lfsr_rbyd_appendcksum(lfs, &rbyd);
if (err) {
LFS_ASSERT(err != LFS_ERR_RANGE);
return err;
}
*btree = rbyd;
} }
LFS_ASSERT(btree->trunk != 0); LFS_ASSERT(btree->trunk != 0);
@@ -4836,18 +4816,17 @@ static int lfsr_bshrub_commit(lfs_t *lfs,
// block_size and rbyds interact, and amortizes the estimate cost. // block_size and rbyds interact, and amortizes the estimate cost.
// figure out how much data this commit progs // figure out how much data this commit progs
lfs_size_t progged = 0;
for (lfs_size_t i = 0; i < attr_count; i++) { for (lfs_size_t i = 0; i < attr_count; i++) {
// only include tag overhead if tag is not a grow tag // only include tag overhead if tag is not a grow tag
if (!lfsr_tag_isgrow(attrs[i].tag)) { if (!lfsr_tag_isgrow(attrs[i].tag)) {
progged += LFSR_ATTR_ESTIMATE; bshrub->progged += LFSR_ATTR_ESTIMATE;
} }
progged += lfsr_data_size(&attrs[i].data); bshrub->progged += lfsr_data_size(&attrs[i].data);
} }
// does progged exceed our shrub_size? need to recalculate an // does progged exceed our shrub_size? need to recalculate an
// accurate our estimate? // accurate our estimate?
if (bshrub->progged + progged > lfs->cfg->shrub_size) { if (bshrub->progged > lfs->cfg->shrub_size) {
lfs_ssize_t estimate = lfsr_rbyd_estimate(lfs, lfs_ssize_t estimate = lfsr_rbyd_estimate(lfs,
&bshrub->rbyd, -1, -1, NULL); &bshrub->rbyd, -1, -1, NULL);
if (estimate < 0) { if (estimate < 0) {
@@ -4870,17 +4849,12 @@ static int lfsr_bshrub_commit(lfs_t *lfs,
if (err) { if (err) {
return err; return err;
} }
bshrub->progged += progged;
} }
LFS_ASSERT(bshrub->rbyd.trunk != 0); LFS_ASSERT(bshrub->rbyd.trunk != 0);
return 0; return 0;
evict:; evict:;
// TODO am I missing a simpler function here? at least use
// lfsr_rbyd_commit once it doesn't maintain a copy...
// convert to btree // convert to btree
err = lfsr_rbyd_alloc(lfs, &bshrub->rbyd_); err = lfsr_rbyd_alloc(lfs, &bshrub->rbyd_);
if (err) { if (err) {
@@ -4901,19 +4875,13 @@ evict:;
return err; return err;
} }
err = lfsr_rbyd_appendattrs(lfs, &bshrub->rbyd_, -1, -1, err = lfsr_rbyd_commit(lfs, &bshrub->rbyd_,
attrs, attr_count); attrs, attr_count);
if (err) { if (err) {
LFS_ASSERT(err != LFS_ERR_RANGE); LFS_ASSERT(err != LFS_ERR_RANGE);
return err; return err;
} }
err = lfsr_rbyd_appendcksum(lfs, &bshrub->rbyd_);
if (err) {
LFS_ASSERT(err != LFS_ERR_RANGE);
return err;
}
bshrub->rbyd = bshrub->rbyd_; bshrub->rbyd = bshrub->rbyd_;
LFS_ASSERT(bshrub->rbyd.trunk != 0); LFS_ASSERT(bshrub->rbyd.trunk != 0);
return 0; return 0;