Describe the bug
The optimized helper function for finding the common prefix length is incorrect.
To Reproduce
- Add this test to
sort_exec.rs:
#[test]
fn has_common_prefix_len_been_optimized_into_bogusness() {
let cpl = common_prefix_len(&[0; 8], &[0; 8]);
assert_eq!(cpl, 8);
let cpl = common_prefix_len(&[0; 4], &[0; 4]);
assert_eq!(cpl, 4);
let cpl = common_prefix_len(&[0; 2], &[0; 2]);
assert_eq!(cpl, 2);
let cpl = common_prefix_len(&[0; 1], &[0; 1]);
assert_eq!(cpl, 1);
}
- Run the test.
Proposed fix
Change the < min_len comparisons to <= min_len.
Or just replace with a simple implementation and benchmark if the tweaky one really improves perf, e.g.:
fn common_prefix_len(a: &[u8], b: &[u8]) -> usize {
std::iter::zip(a, b).take_while(|(&x, &y)| x == y).count()
}
Describe the bug
The optimized helper function for finding the common prefix length is incorrect.
To Reproduce
sort_exec.rs:Proposed fix
Change the
< min_lencomparisons to<= min_len.Or just replace with a simple implementation and benchmark if the tweaky one really improves perf, e.g.: