From b700c8c8193e2eb095148dd18cc660f0c7e38835 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Thu, 3 Jul 2025 19:15:10 -0500 Subject: [PATCH] Dropped fragmenting blocks > 1 fragment So we now keep blocks around until they can be replaced with a single fragment. This is simpler, cheaper, and reduces the number of commits needed to graft (though note arbitrary range removals still keep this unbounded). --- So, this is a delicate tradeoff. On one hand, not fully fragmenting blocks risks keeping around bptrs containing very little data, depending on fragment_size. On the other hand: - It's expensive, and disk utilization during random _deletes_ is not the biggest of concerns. Note our crystallization algorithm should still clean up partial blocks _eventually_, so this doesn't really impact random writes. The main concerns are lfs3_file_truncate/fruncate, and in the future collapserange/punchhole. - Fragmenting bptrs introduces more commits, which have their own prog/erase cost, and it's unclear how this impacts logging operations. There's no point in fragmenting blocks at the head of a log if we're going to fruncate them eventually. I figure lets err on minimizing complexity/code size for now, and if this turns out to be a mistake, we can always revert or introduce fragmenting >1 fragment blocks as an optional feature in the future. --- Saves a big chunk of code, stack, and even some ctx (no more fragment_thresh): code stack ctx before: 37504 2448 656 after: 37024 (-1.3%) 2416 (-1.3%) 652 (-0.6%) --- lfs3.c | 78 +++--------------------------------------- lfs3.h | 15 -------- runners/bench_runner.h | 4 +-- runners/test_runner.h | 4 +-- tests/test_fwrite.toml | 10 +++--- 5 files changed, 11 insertions(+), 100 deletions(-) diff --git a/lfs3.c b/lfs3.c index c0a5d45f..6bd688ae 100644 --- a/lfs3.c +++ b/lfs3.c @@ -12296,69 +12296,6 @@ static int lfs3_file_graft_(lfs3_t *lfs3, lfs3_file_t *file, pos+weight - (bid-(weight_-1)), -1); - // left sibling needs carving but falls underneath our - // fragment threshold? break into fragments - while (lfs3_bptr_isbptr(&bptr_) - && lfs3_bptr_size(&l) > lfs3->cfg->fragment_size - && lfs3_bptr_size(&l) < lfs3_min( - lfs3->cfg->fragment_thresh, - lfs3->cfg->crystal_thresh)) { - bptr_.data = LFS3_DATA_SLICE(bptr_.data, - lfs3->cfg->fragment_size, - -1); - - err = lfs3_file_commit(lfs3, file, bid, LFS3_RATTRS( - LFS3_RATTR_DATA( - LFS3_TAG_GROW | LFS3_TAG_MASK8 | LFS3_TAG_DATA, - -(weight_ - lfs3->cfg->fragment_size), - &LFS3_DATA_TRUNCATE(l.data, - lfs3->cfg->fragment_size)), - LFS3_RATTR_BPTR( - LFS3_TAG_BLOCK, - +(weight_ - lfs3->cfg->fragment_size), - &bptr_))); - if (err) { - goto failed; - } - - weight_ -= lfs3->cfg->fragment_size; - l.data = LFS3_DATA_SLICE(bptr_.data, - -1, - pos - (bid-(weight_-1))); - } - - // right sibling needs carving but falls underneath our - // fragment threshold? break into fragments - while (lfs3_bptr_isbptr(&bptr_) - && lfs3_bptr_size(&r) > lfs3->cfg->fragment_size - && lfs3_bptr_size(&r) < lfs3_min( - lfs3->cfg->fragment_thresh, - lfs3->cfg->crystal_thresh)) { - bptr_.data = LFS3_DATA_SLICE(bptr_.data, - -1, - lfs3_bptr_size(&bptr_) - lfs3->cfg->fragment_size); - - err = lfs3_file_commit(lfs3, file, bid, LFS3_RATTRS( - LFS3_RATTR_BPTR( - LFS3_TAG_GROW | LFS3_TAG_MASK8 | LFS3_TAG_BLOCK, - -(weight_ - lfs3_bptr_size(&bptr_)), - &bptr_), - LFS3_RATTR_DATA( - LFS3_TAG_DATA, - +(weight_ - lfs3_bptr_size(&bptr_)), - &LFS3_DATA_FRUNCATE(r.data, - lfs3->cfg->fragment_size)))); - if (err) { - goto failed; - } - - bid -= (weight_-lfs3_bptr_size(&bptr_)); - weight_ -= (weight_-lfs3_bptr_size(&bptr_)); - r.data = LFS3_DATA_SLICE(bptr_.data, - pos+weight - (bid-(weight_-1)), - -1); - } - // found left sibling? if (bid-(weight_-1) < pos) { // can we get away with a grow attribute? @@ -12368,6 +12305,7 @@ static int lfs3_file_graft_(lfs3_t *lfs3, lfs3_file_t *file, // carve fragment? } else if (!lfs3_bptr_isbptr(&bptr_) + // carve bptr into fragment? || lfs3_bptr_size(&l) <= lfs3->cfg->fragment_size) { rattrs[rattr_count++] = LFS3_RATTR_DATA( LFS3_TAG_GROW | LFS3_TAG_MASK8 | LFS3_TAG_DATA, @@ -12415,6 +12353,7 @@ static int lfs3_file_graft_(lfs3_t *lfs3, lfs3_file_t *file, // carve fragment? } else if (!lfs3_bptr_isbptr(&bptr_) + // carve bptr into fragment? || lfs3_bptr_size(&r) <= lfs3->cfg->fragment_size) { r_rattr_ = LFS3_RATTR_DATA( LFS3_TAG_DATA, bid+1 - (pos+weight), @@ -14001,10 +13940,7 @@ int lfs3_file_truncate(lfs3_t *lfs3, lfs3_file_t *file, lfs3_off_t size_) { // truncated, we can't rely on any in-bshrub/btree state if (!lfs3_bptr_isbptr(&file->leaf.bptr) || lfs3_bptr_size(&file->leaf.bptr) - < lfs3_min( - lfs3->cfg->fragment_thresh, - lfs3->cfg->crystal_thresh) - || lfs3_bptr_size(&file->leaf.bptr) == 0) { + <= lfs3->cfg->fragment_size) { lfs3_file_discardleaf(file); } @@ -14086,10 +14022,7 @@ int lfs3_file_fruncate(lfs3_t *lfs3, lfs3_file_t *file, lfs3_off_t size_) { // truncated, we can't rely on any in-bshrub/btree state if (!lfs3_bptr_isbptr(&file->leaf.bptr) || lfs3_bptr_size(&file->leaf.bptr) - < lfs3_min( - lfs3->cfg->fragment_thresh, - lfs3->cfg->crystal_thresh) - || lfs3_bptr_size(&file->leaf.bptr) == 0) { + <= lfs3->cfg->fragment_size) { lfs3_file_discardleaf(file); } @@ -14405,9 +14338,6 @@ static int lfs3_init(lfs3_t *lfs3, uint32_t flags, LFS3_ASSERT(lfs3->cfg->inline_size <= lfs3->cfg->block_size/4); // fragment_size must be <= block_size/4 LFS3_ASSERT(lfs3->cfg->fragment_size <= lfs3->cfg->block_size/4); - // fragment_thresh > crystal_thresh is probably a mistake - LFS3_ASSERT(lfs3->cfg->fragment_thresh == (lfs3_size_t)-1 - || lfs3->cfg->fragment_thresh <= lfs3->cfg->crystal_thresh); #endif // setup flags diff --git a/lfs3.h b/lfs3.h index cac2c93d..95725bde 100644 --- a/lfs3.h +++ b/lfs3.h @@ -525,21 +525,6 @@ struct lfs3_config { #ifndef LFS3_RDONLY lfs3_size_t crystal_thresh; #endif - - // Threshold for breaking a block into fragments. Smaller values will - // fragment more lazily, reducing random-write cost, but risk higher - // disk usage. - // - // This can be set lower than crystal_thresh to prevent repeated - // compact/break operations in files with heavy random writes, at a - // storage cost. Setting this higher than crystal_thresh is probably not - // a good idea. - // - // 0 will never fragment a block once compacted, while -1 will fragment - // as soon as a block drops below crystal_thresh. - #ifndef LFS3_RDONLY - lfs3_size_t fragment_thresh; - #endif }; // File info structure diff --git a/runners/bench_runner.h b/runners/bench_runner.h index eaa9d287..a6ea00ab 100644 --- a/runners/bench_runner.h +++ b/runners/bench_runner.h @@ -120,7 +120,6 @@ void bench_permutation(size_t i, uint32_t *buffer, size_t size); BENCH_DEFINE(INLINE_SIZE, BLOCK_SIZE/4 ) \ BENCH_DEFINE(FRAGMENT_SIZE, LFS3_MIN(BLOCK_SIZE/8, 512) ) \ BENCH_DEFINE(CRYSTAL_THRESH, BLOCK_SIZE/8 ) \ - BENCH_DEFINE(FRAGMENT_THRESH, -1 ) \ BENCH_DEFINE(ERASE_VALUE, 0xff ) \ BENCH_DEFINE(ERASE_CYCLES, 0 ) \ BENCH_DEFINE(BADBLOCK_BEHAVIOR, LFS3_EMUBD_BADBLOCK_PROGERROR ) \ @@ -149,8 +148,7 @@ void bench_permutation(size_t i, uint32_t *buffer, size_t size); .gc_compact_thresh = GC_COMPACT_THRESH, \ .inline_size = INLINE_SIZE, \ .fragment_size = FRAGMENT_SIZE, \ - .crystal_thresh = CRYSTAL_THRESH, \ - .fragment_thresh = FRAGMENT_THRESH, + .crystal_thresh = CRYSTAL_THRESH, #ifdef LFS3_GC #define BENCH_GC_CFG \ diff --git a/runners/test_runner.h b/runners/test_runner.h index 3bb9e0bb..79f2bdb5 100644 --- a/runners/test_runner.h +++ b/runners/test_runner.h @@ -111,7 +111,6 @@ void test_permutation(size_t i, uint32_t *buffer, size_t size); TEST_DEFINE(INLINE_SIZE, BLOCK_SIZE/4 ) \ TEST_DEFINE(FRAGMENT_SIZE, LFS3_MIN(BLOCK_SIZE/8, 512) ) \ TEST_DEFINE(CRYSTAL_THRESH, BLOCK_SIZE/8 ) \ - TEST_DEFINE(FRAGMENT_THRESH, -1 ) \ TEST_DEFINE(ERASE_VALUE, 0xff ) \ TEST_DEFINE(ERASE_CYCLES, 0 ) \ TEST_DEFINE(BADBLOCK_BEHAVIOR, LFS3_EMUBD_BADBLOCK_PROGERROR ) \ @@ -140,8 +139,7 @@ void test_permutation(size_t i, uint32_t *buffer, size_t size); .gc_compact_thresh = GC_COMPACT_THRESH, \ .inline_size = INLINE_SIZE, \ .fragment_size = FRAGMENT_SIZE, \ - .crystal_thresh = CRYSTAL_THRESH, \ - .fragment_thresh = FRAGMENT_THRESH, + .crystal_thresh = CRYSTAL_THRESH, #ifdef LFS3_GC #define TEST_GC_CFG \ diff --git a/tests/test_fwrite.toml b/tests/test_fwrite.toml index 9c49aa77..cca7c685 100644 --- a/tests/test_fwrite.toml +++ b/tests/test_fwrite.toml @@ -2003,12 +2003,12 @@ code = ''' lfs3_unmount(&lfs3) => 0; ''' -# test that carving below fragment_thresh breaks blocks into fragments +# test that carving to fragment_size breaks blocks into fragments [cases.test_fwrite_truncate_litmus_fragment] defines.N = [1, 2, 8] defines.SIZE = 'N*BLOCK_SIZE' -# 1 vs multiple fragments are implemented slightly differently -defines.FRAGMENTS = [1, 2, 3] +# currently we only support fragmenting blocks <= 1 fragment +defines.FRAGMENTS = [1] defines.TSIZE = 'FRAGMENTS*FRAGMENT_SIZE' defines.SYNC = [false, true] in = 'lfs3.c' @@ -2159,8 +2159,8 @@ code = ''' [cases.test_fwrite_fruncate_litmus_fragment] defines.N = [1, 2, 8] defines.SIZE = 'N*BLOCK_SIZE' -# 1 vs multiple fragments are implemented slightly differently -defines.FRAGMENTS = [1, 2, 3] +# currently we only support fragmenting blocks <= 1 fragment +defines.FRAGMENTS = [1] defines.TSIZE = 'FRAGMENTS*FRAGMENT_SIZE' defines.SYNC = [false, true] in = 'lfs3.c'