From 42cf8f3a69f0ecd81284aa1477207ae647255d41 Mon Sep 17 00:00:00 2001 From: Ellen Emilia Anna Zscheile Date: Fri, 30 May 2025 15:06:49 +0200 Subject: [PATCH] fix(math/cyclic_search): Use classic binary search instead It is unnecessary to use exponential search here, and the computation complexity of binary search is easier to understand and obvious O(log n). This degrades performance minimally in biased edge cases, and improves performance in the hopefully common "roughly half of values are false, and other half is true" case. --- src/math/cyclic_search.rs | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/src/math/cyclic_search.rs b/src/math/cyclic_search.rs index d444121..02a4689 100644 --- a/src/math/cyclic_search.rs +++ b/src/math/cyclic_search.rs @@ -129,7 +129,7 @@ where } /// Search for the largest index inside the bounds which still fulfills the condition -fn exponential_search( +fn binary_search( eval: &EF, expected_value: T, mut bounds: core::ops::Range, @@ -143,28 +143,21 @@ where return None; } - let mut largest_checked = bounds.start; - while (bounds.start + 1) < bounds.end { - let len = bounds.end - bounds.start; - for level in 0..64u8 { - let mut index = 1 << level; - if index >= len { - break; - } - index += bounds.start; - if eval(index) != expected_value { - bounds.end = index; - break; - } - largest_checked = index; - } + while bounds.start + 1 < bounds.end { + let middle = bounds.start + (bounds.end - bounds.start) / 2; + debug_assert_ne!(middle, bounds.start); + debug_assert_ne!(middle, bounds.end); - bounds.start = largest_checked; - // this implies that `bounds.start` doesn't have to get checked again + // binary search for partition bounds + if eval(middle) == expected_value { + bounds.start = middle; + } else { + bounds.end = middle; + } } - debug_assert_eq!(eval(largest_checked), expected_value); - Some(largest_checked) + debug_assert_eq!(eval(bounds.start), expected_value); + Some(bounds.start) } /// Perform a breadth-first search on an induced binary tree on the list, @@ -221,8 +214,8 @@ where true => (&mut pos_true, &mut pos_false), }; - *pos_start = exponential_search(&eval, val_start, bounds.start..*pos_next).unwrap(); - *pos_next = exponential_search(&eval, !val_start, *pos_start + 1..bounds.end).unwrap(); + *pos_start = binary_search(&eval, val_start, bounds.start..*pos_next).unwrap(); + *pos_next = binary_search(&eval, !val_start, *pos_start + 1..bounds.end).unwrap(); } (Some(pos_false), Some(pos_true))