From eab526ad9f6cbb6f403f7fe67bb60bd790ee7d22 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 22 Aug 2025 22:08:01 -0500 Subject: [PATCH] Fixed crystal_thresh=0 bugs There was a mismatch between the lfs3_cfg comment and the actual crystal_thresh math where crystal_thresh=0 would break things: - In lfs3_file_flush_, crystal_thresh=0 meant we would never resume crystallization, leading to terrible, _terrible_, linear write performance. - In lfs3_file_sync and lfs3_set, it's unclear if small file commit optimizations were working properly. I went ahead and added a lfs3_max(lfs3->cfg->crystal_thresh, 1) just to be safe. The other references to crystal_thresh all check for >= crystal_thresh conditions, so shouldn't be broken (except for an unrelated bug in lfs3_file_flushset_). The reason for this is because crystal_thresh=1 is technically the lower bound for this math. Allowing crystal_thresh=0 is just a convenience, and honestly allowing it may have a been a bad idea. Maybe we should require crystal_thresh=1 at minimum? I added a TODO. All the new v3 config needs revisiting anyways, for defaults, etc. --- Curiously, this actually saved code? My best guess is maybe some weird code path in lfs3_file_flush_ was eliminated: code stack ctx before: 37036 2352 684 after: 37028 (-0.0%) 2352 (+0.0%) 684 (+0.0%) --- lfs3.c | 10 ++++++---- lfs3.h | 9 +++++++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/lfs3.c b/lfs3.c index 26a92e92..ca0726ce 100644 --- a/lfs3.c +++ b/lfs3.c @@ -12727,7 +12727,7 @@ int lfs3_file_opencfg_(lfs3_t *lfs3, lfs3_file_t *file, if (lfs3_o_iswrset(file->b.h.flags) && file->cache.size <= lfs3->cfg->inline_size && file->cache.size <= lfs3->cfg->fragment_size - && file->cache.size < lfs3->cfg->crystal_thresh) { + && file->cache.size < lfs3_max(lfs3->cfg->crystal_thresh, 1)) { // we need to mark as unsync for sync to do anything file->b.h.flags |= LFS3_o_UNSYNC; @@ -13753,7 +13753,7 @@ static int lfs3_file_flushset_(lfs3_t *lfs3, lfs3_file_t *file, // enough data for a block? #ifndef LFS3_2BONLY - if (size > lfs3->cfg->crystal_thresh) { + if (size >= lfs3->cfg->crystal_thresh) { // align down for prog alignment lfs3_ssize_t d = lfs3_aligndown( lfs3_min(size, lfs3->cfg->block_size), @@ -13867,7 +13867,9 @@ static int lfs3_file_flush_(lfs3_t *lfs3, lfs3_file_t *file, && lfs3_bptr_iserased(&file->leaf.bptr) && pos >= block_end && pos < block_start + lfs3->cfg->block_size - && pos - block_end < lfs3->cfg->crystal_thresh + // if we're more than a crystal away, graft and check crystal + // heuristic before resuming + && pos - block_end < lfs3_max(lfs3->cfg->crystal_thresh, 1) // need to bail if we can't meet prog alignment && (pos + size) - block_end >= lfs3->cfg->prog_size) { // mark as uncrystallized to avoid allocating a new block @@ -14738,7 +14740,7 @@ int lfs3_file_sync(lfs3_t *lfs3, lfs3_file_t *file) { if (file->cache.size == lfs3_file_size_(file) && file->cache.size <= lfs3->cfg->inline_size && file->cache.size <= lfs3->cfg->fragment_size - && file->cache.size < lfs3->cfg->crystal_thresh) { + && file->cache.size < lfs3_max(lfs3->cfg->crystal_thresh, 1)) { // discard any overwritten leaves, this also clears the // LFS3_o_UNCRYST and LFS3_o_UNGRAFT flags lfs3_file_discardleaf(file); diff --git a/lfs3.h b/lfs3.h index 2ba8e7fa..57295055 100644 --- a/lfs3.h +++ b/lfs3.h @@ -562,12 +562,17 @@ struct lfs3_cfg { lfs3_size_t fragment_size; #endif + // TODO crystal_thresh=0 really just means crystal_thresh=1, should we + // allow crystal_thresh=0? crystal_thresh=0 => block_size/16 or + // block_size/8 is probably a better default. need to benchmark. + // Threshold for compacting multiple fragments into a block. Smaller // values will crystallize more eagerly, reducing disk usage, but // increasing the cost of random-writes. // - // 0 only writes blocks, minimizing disk usage, while -1 or any value > - // block_size only writes fragments, minimizing random-write cost. + // 0 or 1 only writes blocks, minimizing disk usage, while -1 or any + // value > block_size only writes fragments, minimizing random-write + // cost. #ifndef LFS3_RDONLY lfs3_size_t crystal_thresh; #endif