From 27dd339a6a26470a386c679e9d12817d834eb93b Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 28 Apr 2025 17:30:05 -0500 Subject: [PATCH] Added big vestigial-name-split comment This is the _nth_ time I've tried to force arbitrary btree name inserts to work, so _clearly_ I need a bigger comment. Hopefully this will prevent me from trying to delete the LFSR_RATTR_NOOP in test_btree_find_general_fuzz _again_. --- The gist is that insert-before-bid+1 is fundamentally different from insert-after-bid when named btrees are involved: .-----f-----. insert-after-d .-------f-----. .-b--. .--j-. => .-b---. .--j-. | .-. .-. | | .---. .-. | a c d h i k a c d e h i k ^ insert-before-h => .-----f-------. .-b--. .---j-. | .-. .---. | a c d g h i k ^ The problem is that lfsr_btree_commit_ needs to find the same leaf rbyd as lfsr_btree_namelookup, and potentially insert-before the first rid or insert-after the last rid. Instead of separate insert-before/after flags, we make the first tag in a commit insert-before, and all following non-grow tags insert-after (splits). This info is now captured in the above mentioned comment. --- lfs.c | 30 ++++++++++++++++++++++++ tests/test_btree.toml | 53 ++++++++++++++++++++++++++++++++++++++----- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/lfs.c b/lfs.c index 88895b9c..01d4c7f9 100644 --- a/lfs.c +++ b/lfs.c @@ -5436,6 +5436,36 @@ static inline uint32_t lfsr_rev_btree(lfs_t *lfs); // 1. we need a new root // 2. we have a shrub root // +// --- +// +// note! all non-bid-0 name updates must be via splits! +// +// This is because our btrees contain vestigial names, i.e. our inner +// nodes may contain names no longer in the tree. This simplifies +// lfsr_btree_commit_, but means insert-before-bid+1 is _not_ the same +// as insert-after-bid when named btrees are involved. If you try this +// it _will not_ work and if try to make it work you _will_ cry: +// +// .-----f-----. insert-after-d .-------f-----. +// .-b--. .--j-. => .-b---. .--j-. +// | .-. .-. | | .---. .-. | +// a c d h i k a c d e h i k +// ^ +// insert-before-h +// => .-----f-------. +// .-b--. .---j-. +// | .-. .---. | +// a c d g h i k +// ^ +// +// The problem is that lfsr_btree_commit_ needs to find the same leaf +// rbyd as lfsr_btree_namelookup, and potentially insert-before the +// first rid or insert-after the last rid. +// +// Instead of separate insert-before/after flags, we make the first tag +// in a commit insert-before, and all following non-grow tags +// insert-after (splits). +// static int lfsr_btree_commit_(lfs_t *lfs, lfsr_btree_t *btree, lfsr_bctx_t *bctx, lfsr_bid_t *bid, diff --git a/tests/test_btree.toml b/tests/test_btree.toml index ee20966f..336d4c6f 100644 --- a/tests/test_btree.toml +++ b/tests/test_btree.toml @@ -3866,6 +3866,28 @@ code = ''' } // split btree + // + // note! all name updates _must_ be via splits (except for + // the first one) + // + // This is because our btrees contain vestigial names, i.e. + // our inner nodes may contain names no longer in the tree. + // This simplifies lfsr_btree_commit_, but means + // insert-before-bid+1 is _not_ the same as insert-after-bid + // when named btrees are involved. If you try this it _will + // not_ work and if try to make it work you _will_ cry: + // + // .-----f-----. insert-after-d .-------f-----. + // .-b--. .--j-. => .-b---. .--j-. + // | .-. .-. | | .---. .-. | + // a c d h i k a c d e h i k + // ^ + // insert-before-h + // => .-----f-------. + // .-b--. .---j-. + // | .-. .---. | + // a c d g h i k + // ^ lfsr_bid_t split_bid; lfs_scmp_t cmp = lfsr_btree_namelookupleaf(&lfs, &btree, 0, name, 3, @@ -3882,9 +3904,7 @@ code = ''' &(uint8_t){'0'+(i % 10)}, 1))) => 0; } else { lfsr_btree_commit(&lfs, &btree, split_bid, LFSR_RATTRS( - // TODO can we avoid this noop? the problem is we need - // some way to differentiate inserting before vs after - // the requested bid + // yes, we need this noop, see above LFSR_RATTR_NOOP(), LFSR_RATTR_NAME( LFSR_TAG_REG, +1, @@ -4038,6 +4058,29 @@ code = ''' } // split btree + // + // note! all name updates _must_ be via splits (except for + // the first one) + // + // This is because our btrees contain vestigial names, i.e. + // our inner nodes may contain names no longer in the tree. + // This simplifies lfsr_btree_commit_, but means + // insert-before-bid+1 is _not_ the same as insert-after-bid + // when named btrees are involved. If you try this it _will + // not_ work and if try to make it work you _will_ cry: + // + // + // .-----f-----. insert-after-d .-------f-----. + // .-b--. .--j-. => .-b---. .--j-. + // | .-. .-. | | .---. .-. | + // a c d h i k a c d e h i k + // ^ + // insert-before-h + // => .-----f-------. + // .-b--. .---j-. + // | .-. .---. | + // a c d g h i k + // ^ lfsr_bid_t split_bid; lfsr_bid_t split_weight; lfs_scmp_t cmp = lfsr_btree_namelookupleaf(&lfs, &btree, @@ -4056,9 +4099,7 @@ code = ''' &(uint8_t){'0'+(i % 10)}, 1))) => 0; } else { lfsr_btree_commit(&lfs, &btree, split_bid, LFSR_RATTRS( - // TODO can we avoid this noop? the problem is we need - // some way to differentiate inserting before vs after - // the requested bid + // yes, we need this noop, see above LFSR_RATTR_NOOP(), LFSR_RATTR_NAME( LFSR_TAG_REG, +weight,