From 9ac73ceb868532af003fc62ebe5eae7c4a1121e8 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 27 Apr 2025 01:11:35 -0500 Subject: [PATCH] Reverted non-dag file bshrubs/btrees Ok so, funny story, looks like we won't actually need pure-tree bshrubs/btrees. It _is_ true that the single-parent constraint imposed by pure-trees can enable a wider range of algorithms. But looking forward into the planned design, we just happen to not need this constraint at all. I made a mistake here: 1. Block allocation - On paper block allocation benefits the most from the single-parent constraint. But we have another daggish problem, how do we efficiently account for in-flight/open btrees? Naively, you might think we can just traverse all open btrees during allocation, since we shouldn't have _that_ many. But this scales O(n^2) when writing a large file. The key observation being that open files reference on-disk btrees and are _not_ RAM constrained. The current solution involves tree-diffing in order to figure out bmap updates. Which, humorously, works perfectly fine even if the trees are dags. 2. Error correction - I just completely forgot that the current plans for block redundancy require the ddtree. Each block gets mapped into the dense ddtree, with subranges of the ddtree grouped into parity groups backed by the ptree. Instead of bptrs, file btrees store indirect ddkeys into the ddtree. No bptrs? No dag problem! This is still a problem if we ever support naive data redund (redund blocks in a bptrs), but that's out of scope for other reasons (basically just a lot more code). So reverting. Allowing dags allows for much faster random writes, at least in theory. --- For now I'm still keeping the dag-avoidance in lfsr_file_flush_ around under the LFS_NONDAG ifdef. This will likely be dropped at some point, but I'm curious how it affects benchmarks. Ugh, and of course the unused label makes GCC unhappy. Added -Wno-unused-label to CFLAGS because labels have other uses besides just being goto targets (debug targets, code organization, etc). We probably use labels more that other libraries because to littlefs's no-recursion requirement. Code changes minimal, still not sure where that stack difference comes from: code stack ctx before: 35740 2424 640 after: 35736 (-0.0%) 2440 (+0.7%) 640 (+0.0%) --- Makefile | 2 ++ lfs.c | 37 +++++++++++++++---------------------- lfs_util.h | 6 ++++++ 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/Makefile b/Makefile index fdf64a69..eb4dd1c2 100644 --- a/Makefile +++ b/Makefile @@ -74,6 +74,8 @@ CFLAGS += -fcallgraph-info=su CFLAGS += -g3 CFLAGS += -I. CFLAGS += -std=c99 -Wall -Wextra -pedantic +# labels are useful for debugging, in-function organization, etc +CFLAGS += -Wno-unused-label # compiler bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101854 CFLAGS += -Wno-stringop-overflow CFLAGS += -ftrack-macro-expansion=0 diff --git a/lfs.c b/lfs.c index f94c99f3..43ba53b4 100644 --- a/lfs.c +++ b/lfs.c @@ -11879,27 +11879,16 @@ static int lfsr_file_carve(lfs_t *lfs, lfsr_file_t *file, &l.data); // carve bptr? - // - // make sure we're not creating a dag! we can't allow this - // to happen or it would break our littlefs-is-a-tree - // invariant - } else if (!(pos+weight < bid+1 - && lfsr_data_size(r.data) > lfs->cfg->fragment_size)) { + } else { + #ifdef LFS_NONDAG + // if LFS_NONDAG, we're not creating a dag are we? + LFS_ASSERT(!(pos+weight < bid+1 + && lfsr_data_size(r.data) > lfs->cfg->fragment_size)); + #endif rattrs[rattr_count++] = LFSR_RATTR_BPTR( LFSR_TAG_GROW | LFSR_TAG_MASK8 | LFSR_TAG_BLOCK, -(bid+1 - pos), &l); - - // uh oh, keeping both siblings would create a dag? we have - // no choice but to rewrite one into a new block - // - // our crystallization algorithm currently prevents this from - // happening, but it may be a problem in the future (more - // advanced hole APIs, alternative write strategies, etc) - } else { - // this is where we would split dags if an algorithm - // needed it - LFS_UNREACHABLE(); } // completely overwriting this entry? @@ -12448,15 +12437,19 @@ fragment:; // uh oh, would we end up creating a dag? // - // we would be forced to split the dag in lfsr_file_carve in - // order to keep the littlefs-is-a-tree invariant, so we might - // as well try to recrystallize the left sibling - } else if (lfsr_bptr_isbptr(&bptr) + // if LFS_NONDAG, we would be forced to split the dag in + // lfsr_file_carve, so we might as well try to recrystallize + // the left sibling + } else if (LFS_IFDEF_NONDAG( + lfsr_bptr_isbptr(&bptr) && fragment_end - < bid-(weight-1) + lfsr_data_size(bptr.data)) { + < bid-(weight-1) + lfsr_data_size(bptr.data), + false)) { + #ifdef LFS_NONDAG crystal_start = bid-(weight-1); crystal_end = fragment_end; goto crystallize; + #endif } } diff --git a/lfs_util.h b/lfs_util.h index 76fa55bc..a3b6d671 100644 --- a/lfs_util.h +++ b/lfs_util.h @@ -258,6 +258,12 @@ #define LFS_IFDEF_GC(a, b) (b) #endif +#ifdef LFS_NONDAG +#define LFS_IFDEF_NONDAG(a, b) (a) +#else +#define LFS_IFDEF_NONDAG(a, b) (b) +#endif + // Some function attributes, no way around these