Github/littlefs
1
Fork 0
Code Actions Activity

Cleanup around lfsr_btree_commit

Browse Source 
- Consistent handling of missing branches - now asserts

- Consistent short-circuiting of name-less branches - we can always pull
  these off in one lookup

- Skip validating already-fetched rbyd - this only affects the root
  rbyd, but as the most heavily accessed rbyd in the tree this is a nice
  optimization. In practice root rbyds should be validated exactly once.

- Dropped accidental redundant check of some btree merge conditions
This commit is contained in:
Christopher Haster
2023-04-12 17:31:38 -05:00
parent 4662e93c29
commit bdfe66aab2
1 changed files with 57 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+57 -60
View File
@@ -2630,7 +2630,12 @@ static lfs_ssize_t lfsr_btree_lookup(lfs_t *lfs,
// if we're validating during our lookup, we need to fetch each branch, // if we're validating during our lookup, we need to fetch each branch,
// otherwise we can get away with assuming our stored block+trunk is // otherwise we can get away with assuming our stored block+trunk is
// correct // correct
if (validate) { //
// though we assume fetched branches have already been validated, this
// generally only affects the root rbyd but note the root rbyd is the
// most heavily accessed
//
if (validate && !lfsr_rbyd_isfetched(&branch)) {
lfsr_rbyd_t branch_; lfsr_rbyd_t branch_;
int err = lfsr_rbyd_fetch(lfs, &branch_, int err = lfsr_rbyd_fetch(lfs, &branch_,
branch.block, branch.trunk, NULL); branch.block, branch.trunk, NULL);
@@ -2672,14 +2677,15 @@ static lfs_ssize_t lfsr_btree_lookup(lfs_t *lfs,
int err = lfsr_rbyd_lookup(lfs, &branch, rid, 0, int err = lfsr_rbyd_lookup(lfs, &branch, rid, 0,
&rid__, &tag__, &weight__, &data__); &rid__, &tag__, &weight__, &data__);
if (err) { if (err) {
LFS_ASSERT(err != LFS_ERR_NOENT);
return err; return err;
} }
if (lfsr_tag_suptype(tag__) == LFSR_TAG_NAME) { if (lfsr_tag_suptype(tag__) == LFSR_TAG_NAME) {
// TODO what if we don't find a struct? ENOENT?
err = lfsr_rbyd_lookup(lfs, &branch, rid__, LFSR_TAG_STRUCT, err = lfsr_rbyd_lookup(lfs, &branch, rid__, LFSR_TAG_STRUCT,
NULL, &tag__, NULL, &data__); NULL, &tag__, NULL, &data__);
if (err) { if (err) {
LFS_ASSERT(err != LFS_ERR_NOENT);
return err; return err;
} }
} }
@@ -2750,18 +2756,17 @@ static int lfsr_btree_parent(lfs_t *lfs,
lfs_ssize_t rid__; lfs_ssize_t rid__;
lfsr_tag_t tag__; lfsr_tag_t tag__;
lfs_size_t weight__; lfs_size_t weight__;
lfsr_data_t data_; lfsr_data_t data__;
int err = lfsr_rbyd_lookup(lfs, &branch, rid, 0, int err = lfsr_rbyd_lookup(lfs, &branch, rid, 0,
&rid__, &tag__, &weight__, &data_); &rid__, &tag__, &weight__, &data__);
if (err) { if (err) {
LFS_ASSERT(err != LFS_ERR_NOENT); LFS_ASSERT(err != LFS_ERR_NOENT);
return err; return err;
} }
if (lfsr_tag_suptype(tag__) == LFSR_TAG_NAME) { if (lfsr_tag_suptype(tag__) == LFSR_TAG_NAME) {
// TODO what if we don't find a struct? ENOENT?
err = lfsr_rbyd_lookup(lfs, &branch, rid__, LFSR_TAG_STRUCT, err = lfsr_rbyd_lookup(lfs, &branch, rid__, LFSR_TAG_STRUCT,
NULL, &tag__, NULL, &data_); NULL, &tag__, NULL, &data__);
if (err) { if (err) {
LFS_ASSERT(err != LFS_ERR_NOENT); LFS_ASSERT(err != LFS_ERR_NOENT);
return err; return err;
@@ -2778,10 +2783,10 @@ static int lfsr_btree_parent(lfs_t *lfs,
// fetch the next branch // fetch the next branch
uint8_t buf[LFSR_BRANCH_DSIZE]; uint8_t buf[LFSR_BRANCH_DSIZE];
lfs_ssize_t d = lfs_min(LFSR_BRANCH_DSIZE, lfsr_data_size(data_)); lfs_ssize_t d = lfs_min(LFSR_BRANCH_DSIZE, lfsr_data_size(data__));
err = lfs_bd_read(lfs, err = lfs_bd_read(lfs,
&lfs->pcache, &lfs->rcache, d, &lfs->pcache, &lfs->rcache, d,
data_.disk.block, data_.disk.off, buf, d); data__.disk.block, data__.disk.off, buf, d);
if (err) { if (err) {
return err; return err;
} }
@@ -2808,8 +2813,6 @@ static int lfsr_btree_parent(lfs_t *lfs,
} }
} }
// TODO do we really need this?
// TODO why doesn't this match lookup/get in rbyd?
static lfs_ssize_t lfsr_btree_get(lfs_t *lfs, static lfs_ssize_t lfsr_btree_get(lfs_t *lfs,
const lfsr_btree_t *btree, lfs_size_t bid, const lfsr_btree_t *btree, lfs_size_t bid,
lfs_size_t *bid_, lfsr_tag_t *tag_, lfs_size_t *weight_, lfs_size_t *bid_, lfsr_tag_t *tag_, lfs_size_t *weight_,
@@ -2924,14 +2927,15 @@ static lfs_ssize_t lfsr_btree_namelookup(lfs_t *lfs,
err = lfsr_rbyd_lookup(lfs, &branch, find.found_id, 0, err = lfsr_rbyd_lookup(lfs, &branch, find.found_id, 0,
&rid__, &tag__, &weight__, &data__); &rid__, &tag__, &weight__, &data__);
if (err) { if (err) {
LFS_ASSERT(err != LFS_ERR_NOENT);
return err; return err;
} }
if (lfsr_tag_suptype(tag__) == LFSR_TAG_NAME) { if (lfsr_tag_suptype(tag__) == LFSR_TAG_NAME) {
// TODO what if we don't find a struct? ENOENT?
err = lfsr_rbyd_lookup(lfs, &branch, rid__, LFSR_TAG_STRUCT, err = lfsr_rbyd_lookup(lfs, &branch, rid__, LFSR_TAG_STRUCT,
NULL, &tag__, NULL, &data__); NULL, &tag__, NULL, &data__);
if (err) { if (err) {
LFS_ASSERT(err != LFS_ERR_NOENT);
return err; return err;
} }
} }
@@ -3107,11 +3111,9 @@ static int lfsr_btree_commit(lfs_t *lfs,
return err; return err;
} }
// TODO wait do we need to guarantee an id boundary here? will we // try to copy over ids
// always shrink?
// copy over ids
lfsr_tag_t tag = 0;
lfs_ssize_t id = 0; lfs_ssize_t id = 0;
lfsr_tag_t tag = 0;
while (true) { while (true) {
lfsr_data_t data; lfsr_data_t data;
err = lfsr_rbyd_lookup(lfs, rbyd, id, lfsr_tag_next(tag), err = lfsr_rbyd_lookup(lfs, rbyd, id, lfsr_tag_next(tag),
@@ -3139,7 +3141,7 @@ static int lfsr_btree_commit(lfs_t *lfs,
// note we need to account for the missing weight of vestigial // note we need to account for the missing weight of vestigial
// name tags in the following branch tag, which is why we // name tags in the following branch tag, which is why we
// calculate weight like this // calculate weight like this
lfs_size_t weight = id+1 - rbyd_.weight; lfs_size_t w = id+1 - rbyd_.weight;
// keep track of worst-case encoding size in case we need to // keep track of worst-case encoding size in case we need to
// split // split
@@ -3147,8 +3149,7 @@ static int lfsr_btree_commit(lfs_t *lfs,
// append the attr // append the attr
err = lfsr_rbyd_append(lfs, &rbyd_, err = lfsr_rbyd_append(lfs, &rbyd_,
id-lfs_smax32(weight-1, 0), id-lfs_smax32(w-1, 0), lfsr_tag_setmk(tag), +w,
lfsr_tag_setmk(tag), +weight,
data); data);
if (err) { if (err) {
return err; return err;
@@ -3174,11 +3175,7 @@ static int lfsr_btree_commit(lfs_t *lfs,
// is our compacted size too small? try to merge with one of // is our compacted size too small? try to merge with one of
// our siblings // our siblings
if (rbyd_.off < lfs->cfg->block_size/4 if (rbyd_.off < lfs->cfg->block_size/4) {
// no parent? can't merge
&& pid != -1
// only child? can't merge
&& pweight != parent.weight) {
goto merge; goto merge;
merge_abort:; merge_abort:;
} }
@@ -3237,8 +3234,8 @@ static int lfsr_btree_commit(lfs_t *lfs,
// note this is the most expensive operation in lfsr_btree_commit // note this is the most expensive operation in lfsr_btree_commit
// //
lfs_ssize_t id = rbyd->weight-1; lfs_ssize_t id = rbyd->weight-1;
lfs_size_t upper_dsize = 0;
lfs_size_t split_id = rbyd_.weight; lfs_size_t split_id = rbyd_.weight;
lfs_size_t upper_dsize = 0;
while (true) { while (true) {
lfsr_tag_t tag = 0; lfsr_tag_t tag = 0;
lfs_size_t w = 0; lfs_size_t w = 0;
@@ -3334,13 +3331,13 @@ static int lfsr_btree_commit(lfs_t *lfs,
return err; return err;
} }
lfsr_tag_t tag = 0;
id = split_id; id = split_id;
lfsr_tag_t tag = 0;
while (true) { while (true) {
lfs_size_t weight; lfs_size_t w;
lfsr_data_t data; lfsr_data_t data;
err = lfsr_rbyd_lookup(lfs, rbyd, id, lfsr_tag_next(tag), err = lfsr_rbyd_lookup(lfs, rbyd, id, lfsr_tag_next(tag),
&id, &tag, &weight, &data); &id, &tag, &w, &data);
if (err && err != LFS_ERR_NOENT) { if (err && err != LFS_ERR_NOENT) {
return err; return err;
} }
@@ -3350,8 +3347,7 @@ static int lfsr_btree_commit(lfs_t *lfs,
// append the attr // append the attr
err = lfsr_rbyd_append(lfs, &sibling, err = lfsr_rbyd_append(lfs, &sibling,
id-split_id-lfs_smax32(weight-1, 0), id-split_id-lfs_smax32(w-1, 0), lfsr_tag_setmk(tag), +w,
lfsr_tag_setmk(tag), +weight,
data); data);
if (err) { if (err) {
return err; return err;
@@ -3389,11 +3385,11 @@ static int lfsr_btree_commit(lfs_t *lfs,
// note we need to do this after playing out pending attrs in case // note we need to do this after playing out pending attrs in case
// they introduce a new name! // they introduce a new name!
lfsr_tag_t stag; lfsr_tag_t stag;
lfs_ssize_t sid;
lfsr_data_t sdata; lfsr_data_t sdata;
err = lfsr_rbyd_lookup(lfs, &sibling, 0, LFSR_TAG_NAME, err = lfsr_rbyd_lookup(lfs, &sibling, 0, LFSR_TAG_NAME,
&sid, &stag, NULL, &sdata); NULL, &stag, NULL, &sdata);
if (err && err != LFS_ERR_NOENT) { if (err) {
LFS_ASSERT(err != LFS_ERR_NOENT);
return err; return err;
} }
@@ -3477,6 +3473,7 @@ static int lfsr_btree_commit(lfs_t *lfs,
} }
// last child? try the left sibling // last child? try the left sibling
lfs_ssize_t sid;
lfs_ssize_t sdelta; lfs_ssize_t sdelta;
if ((lfs_size_t)pid == parent.weight-1) { if ((lfs_size_t)pid == parent.weight-1) {
sid = pid-pweight; sid = pid-pweight;
@@ -3490,24 +3487,26 @@ static int lfsr_btree_commit(lfs_t *lfs,
// try looking up the sibling // try looking up the sibling
lfs_size_t sweight; lfs_size_t sweight;
err = lfsr_rbyd_lookup(lfs, &parent, sid, LFSR_TAG_NAME, err = lfsr_rbyd_lookup(lfs, &parent, sid, LFSR_TAG_NAME,
&sid, NULL, &sweight, NULL); &sid, &stag, &sweight, &sdata);
if (err && err != LFS_ERR_NOENT) { if (err) {
// no sibling? can't merge
if (err == LFS_ERR_NOENT) {
goto merge_abort;
}
return err; return err;
} }
// no sibling? can't merge if (stag == LFSR_TAG_NAME) {
if (err == LFS_ERR_NOENT) { err = lfsr_rbyd_lookup(lfs, &parent, sid, LFSR_TAG_STRUCT,
goto merge_abort; NULL, &stag, NULL, &sdata);
} if (err) {
LFS_ASSERT(err != LFS_ERR_NOENT);
err = lfsr_rbyd_lookup(lfs, &parent, sid, LFSR_TAG_BRANCH, return err;
NULL, &stag, NULL, &sdata); }
if (err && err != LFS_ERR_NOENT) {
return err;
} }
// no sibling? can't merge // no sibling? can't merge
if (err == LFS_ERR_NOENT || stag != LFSR_TAG_BRANCH) { if (stag != LFSR_TAG_BRANCH) {
goto merge_abort; goto merge_abort;
} }
@@ -3527,13 +3526,13 @@ static int lfsr_btree_commit(lfs_t *lfs,
// try to add our sibling's tags to our rbyd // try to add our sibling's tags to our rbyd
lfs_size_t rweight_ = rbyd_.weight; lfs_size_t rweight_ = rbyd_.weight;
tag = 0;
id = 0; id = 0;
tag = 0;
while (true) { while (true) {
lfs_size_t weight; lfs_size_t w;
lfsr_data_t data; lfsr_data_t data;
err = lfsr_rbyd_lookup(lfs, &sibling, id, lfsr_tag_next(tag), err = lfsr_rbyd_lookup(lfs, &sibling, id, lfsr_tag_next(tag),
&id, &tag, &weight, &data); &id, &tag, &w, &data);
if (err && err != LFS_ERR_NOENT) { if (err && err != LFS_ERR_NOENT) {
return err; return err;
} }
@@ -3543,8 +3542,7 @@ static int lfsr_btree_commit(lfs_t *lfs,
// append the attr // append the attr
err = lfsr_rbyd_append(lfs, &rbyd_, err = lfsr_rbyd_append(lfs, &rbyd_,
sdelta+id-lfs_smax32(weight-1, 0), sdelta+id-lfs_smax32(w-1, 0), lfsr_tag_setmk(tag), +w,
lfsr_tag_setmk(tag), +weight,
data); data);
if (err) { if (err) {
return err; return err;
@@ -3576,19 +3574,18 @@ static int lfsr_btree_commit(lfs_t *lfs,
} }
if (lfsr_tag_suptype(split_tag) == LFSR_TAG_NAME) { if (lfsr_tag_suptype(split_tag) == LFSR_TAG_NAME) {
// TODO can we avoid this? // lookup the id (weight really) of the previously-split entry
// lookup the id of the previously-split entry
lfs_ssize_t split_id; lfs_ssize_t split_id;
err = lfsr_rbyd_lookup(lfs, &rbyd_, err = lfsr_rbyd_lookup(lfs, &rbyd_,
(sdelta == 0 ? sweight : rweight_), LFSR_TAG_NAME, (sdelta == 0 ? sweight : rweight_), LFSR_TAG_NAME,
&split_id, NULL, NULL, NULL); &split_id, NULL, NULL, NULL);
if (err) { if (err) {
LFS_ASSERT(err != LFS_ERR_NOENT);
return err; return err;
} }
err = lfsr_rbyd_append(lfs, &rbyd_, err = lfsr_rbyd_append(lfs, &rbyd_,
split_id, LFSR_TAG_BNAME, 0, split_id, LFSR_TAG_BNAME, 0, split_data);
split_data);
if (err) { if (err) {
return err; return err;
} }
@@ -3728,7 +3725,6 @@ static int lfsr_btree_push(lfs_t *lfs, lfsr_btree_t *btree,
LFS_ASSERT(size <= LFSR_BTREE_INLINESIZE); LFS_ASSERT(size <= LFSR_BTREE_INLINESIZE);
memcpy(btree->inlined.buffer, buffer, size); memcpy(btree->inlined.buffer, buffer, size);
btree->inlined.size = size; btree->inlined.size = size;
return 0;
} }
return 0; return 0;
@@ -3831,8 +3827,12 @@ static int lfsr_btree_pop(lfs_t *lfs, lfsr_btree_t *btree, lfs_size_t bid) {
return degenerate; return degenerate;
} }
// revert to a null btree
if (degenerate && rweight >= rbyd.weight) {
btree->weight = lfsr_btree_setinlined(0);
// revert to an inlined btree // revert to an inlined btree
if (degenerate && rweight < rbyd.weight) { } else if (degenerate) {
lfs_ssize_t sid; lfs_ssize_t sid;
// left sibling // left sibling
if ((lfs_size_t)rid == rbyd.weight-1) { if ((lfs_size_t)rid == rbyd.weight-1) {
@@ -3848,14 +3848,15 @@ static int lfsr_btree_pop(lfs_t *lfs, lfsr_btree_t *btree, lfs_size_t bid) {
int err = lfsr_rbyd_lookup(lfs, &rbyd, sid, LFSR_TAG_NAME, int err = lfsr_rbyd_lookup(lfs, &rbyd, sid, LFSR_TAG_NAME,
&sid, &stag, &sweight, &sdata); &sid, &stag, &sweight, &sdata);
if (err) { if (err) {
LFS_ASSERT(err == LFS_ERR_NOENT);
return err; return err;
} }
if (lfsr_tag_suptype(stag) == LFSR_TAG_NAME) { if (lfsr_tag_suptype(stag) == LFSR_TAG_NAME) {
// TODO what if we don't find a struct? ENOENT?
err = lfsr_rbyd_lookup(lfs, &rbyd, sid, LFSR_TAG_STRUCT, err = lfsr_rbyd_lookup(lfs, &rbyd, sid, LFSR_TAG_STRUCT,
NULL, &stag, NULL, &sdata); NULL, &stag, NULL, &sdata);
if (err) { if (err) {
LFS_ASSERT(err == LFS_ERR_NOENT);
return err; return err;
} }
} }
@@ -3873,10 +3874,6 @@ static int lfsr_btree_pop(lfs_t *lfs, lfsr_btree_t *btree, lfs_size_t bid) {
return err; return err;
} }
btree->inlined.size = lfsr_data_size(sdata); btree->inlined.size = lfsr_data_size(sdata);
// revert to a null btree
} else if (degenerate) {
btree->weight = lfsr_btree_setinlined(0);
} }
return 0; return 0;