diff --git a/lfs.c b/lfs.c index 6dc1bed8..9038a063 100644 --- a/lfs.c +++ b/lfs.c @@ -1913,13 +1913,10 @@ static lfs_ssize_t lfsr_rbyd_bisect(lfs_t *lfs, const lfsr_rbyd_t *rbyd) { bsize += LFSR_TAG_DSIZE + size; if (bsize >= dsize/2) { + // well this shouldn't happen unless attr limits have gone wrong + LFS_ASSERT((lfs_size_t)id + 1 < rbyd->weight); // round up so that we always include at least one id in the // first rbyd - if ((lfs_size_t)id + 1 >= rbyd->weight) { - // well this shouldn't happen unless attr limits have gone wrong - return LFS_ERR_RANGE; - } - return id + 1; } } @@ -2382,7 +2379,7 @@ static int lfsr_rbyd_append(lfs_t *lfs, lfsr_rbyd_t *rbyd, lfs_swap32(&jump, &branch_); lfs_swap16(&p_alts[0], &alt); - lfs_swaps32(&p_weights[0], &weight); + lfs_sswap32(&p_weights[0], &weight); lfs_swap32(&p_jumps[0], &jump); alt = lfsr_tag_mkblack(alt); @@ -2447,7 +2444,7 @@ static int lfsr_rbyd_append(lfs_t *lfs, lfsr_rbyd_t *rbyd, lower_id, upper_id, tag_, id_)) { lfs_swap16(&p_alts[0], &alt); - lfs_swaps32(&p_weights[0], &weight); + lfs_sswap32(&p_weights[0], &weight); lfs_swap32(&p_jumps[0], &jump); p_alts[0] = lfsr_tag_mkred(p_alts[0]); alt = lfsr_tag_mkblack(alt); @@ -2503,10 +2500,10 @@ static int lfsr_rbyd_append(lfs_t *lfs, lfsr_rbyd_t *rbyd, if (diverged >= 4 || !lfsr_tag_isalt(alt)) { diverged ^= 0x1; lfs_swap16(&tag_, &other_tag_); - lfs_swaps32(&id_, &other_id_); + lfs_sswap32(&id_, &other_id_); lfs_swap32(&branch, &other_branch); - lfs_swaps32(&lower_id, &other_lower_id); - lfs_swaps32(&upper_id, &other_upper_id); + lfs_sswap32(&lower_id, &other_lower_id); + lfs_sswap32(&upper_id, &other_upper_id); lfs_swap16(&lower_tag, &other_lower_tag); lfs_swap16(&upper_tag, &other_upper_tag); } @@ -3038,7 +3035,7 @@ static int lfsr_rbyd_commit(lfs_t *lfs, lfsr_rbyd_t *rbyd, /// Rbyd b-tree operations /// // TODO this is a weird null, move weight out of inlined? -#define LFSR_BTREE_NULL ((lfsr_btree_t){.tag=0x2, .u.inlined.weight=0}) +#define LFSR_BTREE_NULL ((lfsr_btree_t){.weight=0}) // B-tree on-disk encoding @@ -3099,16 +3096,16 @@ static lfs_ssize_t lfsr_btree_lookup(lfs_t *lfs, lfsr_rbyd_t *rbyd_, lfs_ssize_t *rid_, lfs_size_t *weight_, void *buffer, lfs_size_t size) { + // in range? + if (id >= btree->weight) { + return LFS_ERR_NOENT; + } + // inlined? if (btree->tag) { - // in range? - if (id >= btree->u.inlined.weight) { - return LFS_ERR_NOENT; - } - // TODO how many of these should be conditional? if (id_) { - *id_ = btree->u.inlined.weight-1; + *id_ = btree->weight-1; } if (tag_) { *tag_ = btree->tag; @@ -3118,7 +3115,7 @@ static lfs_ssize_t lfsr_btree_lookup(lfs_t *lfs, *rid_ = -1; } if (weight_) { - *weight_ = btree->u.inlined.weight; + *weight_ = btree->weight; } memcpy(buffer, btree->u.inlined.buf, @@ -3215,9 +3212,10 @@ static int lfsr_btree_parent(lfs_t *lfs, const lfsr_btree_t *btree, lfs_size_t id, const lfsr_rbyd_t *child, lfsr_rbyd_t *rbyd_, lfs_ssize_t *rid_) { // inlined? root? - if (btree->tag || ( - btree->u.trunk.block == child->block - && btree->u.trunk.limit == child->off)) { + if (id >= btree->weight + || btree->tag + || (btree->u.trunk.block == child->block + && btree->u.trunk.limit == child->off)) { return LFS_ERR_NOENT; } @@ -3919,6 +3917,7 @@ static int lfsr_btree_commit(lfs_t *lfs, } // at this point rbyd should be the trunk of our tree + btree->weight = rbyd->weight; btree->u.trunk.block = rbyd->block; btree->u.trunk.limit = rbyd->off; return 0; @@ -3928,13 +3927,15 @@ static int lfsr_btree_push(lfs_t *lfs, lfsr_btree_t *btree, lfs_size_t id, lfsr_tag_t tag, lfs_size_t weight, const void *buffer, lfs_size_t size) { + LFS_ASSERT(id <= btree->weight); + // printf("- push(%d, %x, w%d) -\n", id, tag, weight); // null btree? - if (btree->tag && btree->u.inlined.weight == 0) { + if (btree->weight == 0) { LFS_ASSERT(id == 0); btree->tag = tag; - btree->u.inlined.weight = weight; + btree->weight = weight; LFS_ASSERT(size <= LFSR_BTREE_INLINE_SIZE); memcpy(btree->u.inlined.buf, buffer, size); @@ -3943,8 +3944,6 @@ static int lfsr_btree_push(lfs_t *lfs, // inlined btree, need to expand into an rbyd } else if (btree->tag) { - LFS_ASSERT(id == btree->u.inlined.weight); - lfsr_rbyd_t rbyd; int err = lfsr_rbyd_alloc(lfs, &rbyd, 1); if (err) { @@ -3953,9 +3952,9 @@ static int lfsr_btree_push(lfs_t *lfs, // commit our entries err = lfsr_rbyd_commit(lfs, &rbyd, - LFSR_ATTR(GROW, 0, NULL, btree->u.inlined.weight, - LFSR_ATTR(MKBRANCH, 0+btree->u.inlined.weight-1, NULL, 0, - LFSR_ATTR_(btree->tag, 0+btree->u.inlined.weight-1, + LFSR_ATTR(GROW, 0, NULL, btree->weight, + LFSR_ATTR(MKBRANCH, 0+btree->weight-1, NULL, 0, + LFSR_ATTR_(btree->tag, 0+btree->weight-1, btree->u.inlined.buf, btree->u.inlined.size, LFSR_ATTR(GROW, id, NULL, weight, LFSR_ATTR(MKBRANCH, id+weight-1, NULL, 0, @@ -3967,6 +3966,7 @@ static int lfsr_btree_push(lfs_t *lfs, } btree->tag = 0; + btree->weight = rbyd.weight; btree->u.trunk.block = rbyd.block; btree->u.trunk.limit = rbyd.off; return 0; @@ -3978,16 +3978,22 @@ static int lfsr_btree_push(lfs_t *lfs, // return ENOENT, is this ok? lfsr_rbyd_t rbyd; lfs_ssize_t rid; - lfs_ssize_t size = lfsr_btree_lookup(lfs, btree, id-weight, + lfs_ssize_t size = lfsr_btree_lookup(lfs, btree, + lfs_min32(id, btree->weight-1), NULL, NULL, &rbyd, &rid, NULL, NULL, 0); if (size < 0) { return size; } - rid += 1; + + // adjust rid if we're appending + if (id >= btree->weight) { + rid += 1; + } // commit our id into the tree, letting lfsr_btree_commit take care // of the rest - return lfsr_btree_commit(lfs, btree, id-weight, &rbyd, + return lfsr_btree_commit(lfs, btree, + lfs_min32(id, btree->weight-1), &rbyd, LFSR_ATTR(GROW, rid, NULL, weight, LFSR_ATTR(MKBRANCH, rid+weight-1, NULL, 0, LFSR_ATTR_(tag, rid+weight-1, buffer, size, diff --git a/lfs.h b/lfs.h index 1470ef23..2e7e26d7 100644 --- a/lfs.h +++ b/lfs.h @@ -367,12 +367,12 @@ typedef struct lfsr_branch { } lfsr_branch_t; typedef struct lfsr_btree { + lfs_size_t weight; // TODO do we need full tag actually? this fits in a byte? lfsr_tag_t tag; // how can we take advantage of byte packing with union alignment? union { struct { - lfs_size_t weight; uint8_t size; uint8_t buf[LFSR_BTREE_INLINE_SIZE]; } inlined; diff --git a/lfs_util.h b/lfs_util.h index 9253d119..8c75edfe 100644 --- a/lfs_util.h +++ b/lfs_util.h @@ -114,6 +114,22 @@ static inline uint32_t lfs_min(uint32_t a, uint32_t b) { return (a < b) ? a : b; } +static inline uint32_t lfs_max32(uint32_t a, uint32_t b) { + return (a > b) ? a : b; +} + +static inline uint32_t lfs_min32(uint32_t a, uint32_t b) { + return (a < b) ? a : b; +} + +static inline int32_t lfs_smax32(int32_t a, int32_t b) { + return (a > b) ? a : b; +} + +static inline int32_t lfs_smin32(int32_t a, int32_t b) { + return (a < b) ? a : b; +} + // TODO how many of these do we actually need // Swap two 16-bit numbers static inline void lfs_swap16(uint16_t *a, uint16_t *b) { @@ -122,7 +138,7 @@ static inline void lfs_swap16(uint16_t *a, uint16_t *b) { *b = t; } -static inline void lfs_swaps16(int16_t *a, int16_t *b) { +static inline void lfs_sswap16(int16_t *a, int16_t *b) { int16_t t = *a; *a = *b; *b = t; @@ -135,7 +151,7 @@ static inline void lfs_swap32(uint32_t *a, uint32_t *b) { *b = t; } -static inline void lfs_swaps32(int32_t *a, int32_t *b) { +static inline void lfs_sswap32(int32_t *a, int32_t *b) { int32_t t = *a; *a = *b; *b = t; diff --git a/tests/test_btree.toml b/tests/test_btree.toml index 618b7224..e6799139 100644 --- a/tests/test_btree.toml +++ b/tests/test_btree.toml @@ -115,6 +115,51 @@ code = ''' buffer, 4) => LFS_ERR_NOENT; ''' +[cases.test_btree_two_backwards] +in = 'lfs.c' +code = ''' + lfs_t lfs; + lfs_init(&lfs, cfg) => 0; + // create free lookahead + memset(lfs.free.buffer, 0, lfs.cfg->lookahead_size); + lfs.free.off = 0; + lfs.free.size = lfs_min(8*lfs.cfg->lookahead_size, + lfs.cfg->block_count); + lfs.free.i = 0; + lfs_alloc_ack(&lfs); + + // create a two-entry tree + lfsr_btree_t btree = LFSR_BTREE_NULL; + lfsr_btree_push(&lfs, &btree, 0, LFSR_TAG_INLINED, 1, "b", 1) => 0; + lfsr_btree_push(&lfs, &btree, 0, LFSR_TAG_INLINED, 1, "a", 1) => 0; + + // try looking up tags + uint8_t buffer[4]; + lfsr_tag_t tag_; + lfs_size_t id_; + lfs_size_t weight_; + + lfsr_btree_get(&lfs, &btree, 0, + &tag_, &id_, &weight_, + buffer, 4) => 1; + assert(tag_ == LFSR_TAG_INLINED); + assert(id_ == 0); + assert(weight_ == 1); + assert(memcmp(buffer, "a", 1) == 0); + + lfsr_btree_get(&lfs, &btree, 1, + &tag_, &id_, &weight_, + buffer, 4) => 1; + assert(tag_ == LFSR_TAG_INLINED); + assert(id_ == 1); + assert(weight_ == 1); + assert(memcmp(buffer, "b", 1) == 0); + + lfsr_btree_get(&lfs, &btree, 2, + &tag_, &id_, &weight_, + buffer, 4) => LFS_ERR_NOENT; +''' + # still a single-rbyd tree, just making sure it works [cases.test_btree_three] in = 'lfs.c' @@ -170,10 +215,64 @@ code = ''' buffer, 4) => LFS_ERR_NOENT; ''' +[cases.test_btree_three_backwards] +in = 'lfs.c' +code = ''' + lfs_t lfs; + lfs_init(&lfs, cfg) => 0; + // create free lookahead + memset(lfs.free.buffer, 0, lfs.cfg->lookahead_size); + lfs.free.off = 0; + lfs.free.size = lfs_min(8*lfs.cfg->lookahead_size, + lfs.cfg->block_count); + lfs.free.i = 0; + lfs_alloc_ack(&lfs); + + // create a two-entry tree + lfsr_btree_t btree = LFSR_BTREE_NULL; + lfsr_btree_push(&lfs, &btree, 0, LFSR_TAG_INLINED, 1, "c", 1) => 0; + lfsr_btree_push(&lfs, &btree, 0, LFSR_TAG_INLINED, 1, "b", 1) => 0; + lfsr_btree_push(&lfs, &btree, 0, LFSR_TAG_INLINED, 1, "a", 1) => 0; + + // try looking up tags + uint8_t buffer[4]; + lfsr_tag_t tag_; + lfs_size_t id_; + lfs_size_t weight_; + + lfsr_btree_get(&lfs, &btree, 0, + &tag_, &id_, &weight_, + buffer, 4) => 1; + assert(tag_ == LFSR_TAG_INLINED); + assert(id_ == 0); + assert(weight_ == 1); + assert(memcmp(buffer, "a", 1) == 0); + + lfsr_btree_get(&lfs, &btree, 1, + &tag_, &id_, &weight_, + buffer, 4) => 1; + assert(tag_ == LFSR_TAG_INLINED); + assert(id_ == 1); + assert(weight_ == 1); + assert(memcmp(buffer, "b", 1) == 0); + + lfsr_btree_get(&lfs, &btree, 2, + &tag_, &id_, &weight_, + buffer, 4) => 1; + assert(tag_ == LFSR_TAG_INLINED); + assert(id_ == 2); + assert(weight_ == 1); + assert(memcmp(buffer, "c", 1) == 0); + + lfsr_btree_get(&lfs, &btree, 3, + &tag_, &id_, &weight_, + buffer, 4) => LFS_ERR_NOENT; +''' + # try larger trees, when exactly a tree splits depends on the disk geometry, so # we don't really have a better way of testing multi-rbyd trees -[cases.test_btree_more] -defines.N = 'range(10, 1001, 50)' +[cases.test_btree_push] +defines.N = [4, 8, 16, 32, 64, 128, 256, 512, 1024] in = 'lfs.c' code = ''' lfs_t lfs; @@ -215,3 +314,122 @@ code = ''' &tag_, &id_, &weight_, buffer, 4) => LFS_ERR_NOENT; ''' + +[cases.test_btree_push_backwards] +defines.N = [4, 8, 16, 32, 64, 128, 256, 512, 1024] +in = 'lfs.c' +code = ''' + lfs_t lfs; + lfs_init(&lfs, cfg) => 0; + // create free lookahead + memset(lfs.free.buffer, 0, lfs.cfg->lookahead_size); + lfs.free.off = 0; + lfs.free.size = lfs_min(8*lfs.cfg->lookahead_size, + lfs.cfg->block_count); + lfs.free.i = 0; + lfs_alloc_ack(&lfs); + + // create a tree with N elements + lfsr_btree_t btree = LFSR_BTREE_NULL; + const char *alphas = "abcdefghijklmnopqrstuvwxyz"; + for (lfs_size_t i = 0; i < N; i++) { + lfsr_btree_push(&lfs, &btree, 0, LFSR_TAG_INLINED, 1, + &alphas[(N-1-i) % 26], 1) => 0; + } + + // check that the elements are in the tree + uint8_t buffer[4]; + lfsr_tag_t tag_; + lfs_size_t id_; + lfs_size_t weight_; + + for (lfs_size_t i = 0; i < N; i++) { + lfsr_btree_get(&lfs, &btree, i, + &tag_, &id_, &weight_, + buffer, 4) => 1; + assert(tag_ == LFSR_TAG_INLINED); + assert(id_ == i); + assert(weight_ == 1); + assert(memcmp(buffer, &alphas[i % 26], 1) == 0); + } + + // and check that we can't lookup elements that aren't in the tree + lfsr_btree_get(&lfs, &btree, N, + &tag_, &id_, &weight_, + buffer, 4) => LFS_ERR_NOENT; +''' + +[cases.test_btree_push_fuzz] +defines.N = [1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024] +defines.ITER = 10 +in = 'lfs.c' +code = ''' + const char *alphas = "abcdefghijklmnopqrstuvwxyz"; + + // iterate through severals seeds that we can reproduce easily + for (uint32_t seed = 1; seed < ITER+1; seed++) { + // create lfs here since we need to reset each iteration, we're + // space constrained and we can't expect gc to work at this point + lfs_t lfs; + lfs_init(&lfs, cfg) => 0; + // create free lookahead + memset(lfs.free.buffer, 0, lfs.cfg->lookahead_size); + lfs.free.off = 0; + lfs.free.size = lfs_min(8*lfs.cfg->lookahead_size, + lfs.cfg->block_count); + lfs.free.i = 0; + lfs_alloc_ack(&lfs); + + // create a btree + lfsr_btree_t btree = LFSR_BTREE_NULL; + + // set up a simulation to compare against + // + // fun fact this is slower than our actual tree! unfun fact this is + // starting to be a problem... + char *sim = malloc(N); + lfs_size_t sim_size = 0; + memset(sim, 0, N); + + uint32_t prng = seed; + for (lfs_size_t i = 0; i < N; i++) { + // choose a pseudo-random id + lfs_size_t id = TEST_PRNG(&prng) % (sim_size+1); + + // add to btree + lfsr_btree_push(&lfs, &btree, id, LFSR_TAG_INLINED, 1, + &alphas[i], 1) => 0; + + // add to sim + memcpy(&sim[id+1], &sim[id], sim_size-id); + sim[id] = alphas[i]; + sim_size += 1; + } + + // check that btree matches sim + assert(btree.weight == N); + + uint8_t buffer[4]; + lfsr_tag_t tag_; + lfs_size_t id_; + lfs_size_t weight_; + for (lfs_size_t i = 0; i < N; i++) { + lfsr_btree_get(&lfs, &btree, i, + &tag_, &id_, &weight_, + buffer, 4) => 1; + assert(tag_ == LFSR_TAG_INLINED); + assert(id_ == i); + assert(weight_ == 1); + assert(memcmp(buffer, &sim[i], 1) == 0); + } + + // and no extra elements + lfsr_btree_get(&lfs, &btree, N, + &tag_, &id_, &weight_, + buffer, 4) => LFS_ERR_NOENT; + + // clean up sim + free(sim); + } +''' +