From f62ae0e8fdb66e29163e30959973e74636ba382d Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 6 Mar 2024 15:09:40 -0600 Subject: [PATCH] Reroute range removal pruning through diverged path swaps I think this was just an oversight when merging/unmerging pruning operations. Lazily finding alts (eagerly swapping) seems to result in better trees based on some napkin sketches. For example, consider this remove, with lazy alts (eager swaps): .-------o-------. .---o---. .---o---. .-----------------o .-o-. .-o-. .-o-. .-o-. .-o-. .-----------o .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .--------o--------. a b c d e f g h i j k l m n o p => a b c d e f g p '-------+-------' remove h=3 And with eager alts (lazy swaps): .-------o-------. .-----------------o .---o---. .---o---. | o--------. .-o-. .-o-. .-o-. .-o-. .-o-. .--.--------o | .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. | | a b c d e f g h i j k l m n o p => a b c d e f g p '-------+-------' remove h=4 This isn't really rigorous, but without more evidence lazy alts (eager swaps) seem the best option for now. Note that we do _not_ eagerly swap when pruning yellow alts. The two other continue statements in the appendattr loop, one for pruning yellow alts and one for splitting yellow alts, are bookkeeping operations that don't map to real alt visits. We should pretend these alts don't exist when looking at the tree layout. With diverged recoloring, we can't actually hit the yellow-split case, but we can hit the yellow pruning case since it only relies on unreachability. Code cost, uh, I don't really know why this saved code, it's probably just compiler noise: code stack before: 33992 2880 after: 33976 (+0.0%) 2880 (+0.0%) --- lfs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lfs.c b/lfs.c index fefc7b74..29e2cb4c 100644 --- a/lfs.c +++ b/lfs.c @@ -2858,7 +2858,7 @@ static int lfsr_rbyd_appendattr(lfs_t *lfs, lfsr_rbyd_t *rbyd, graft = branch; branch = branch_; - continue; + goto next; } } @@ -3036,6 +3036,7 @@ static int lfsr_rbyd_appendattr(lfs_t *lfs, lfsr_rbyd_t *rbyd, } } + next:; // switch to the other path if we have diverged if (lfsr_tag_hasdiverged(tag_)) { lfs_swap16(&tag_, &other_tag_);