Skip to content

Commit 1aac55d

Browse files
committed
Auto merge of #50575 - alexcrichton:faster-drain-drop, r=<try>
std: Avoid `ptr::copy` if unnecessary in `vec::Drain` This commit is spawned out of a performance regression investigation in #50496. In tracking down this regression it turned out that the `expand_statements` function in the compiler was taking quite a long time. Further investigation showed two key properties: * The function was "fast" on glibc 2.24 and slow on glibc 2.23 * The hottest function was memmove from glibc Combined together it looked like glibc gained an optimization to the memmove function in 2.24. Ideally we don't want to rely on this optimization, so I wanted to dig further to see what was happening. The hottest part of `expand_statements` was `Drop for Drain` in the call to `splice` where we insert new statements into the original vector. This *should* be a cheap operation because we're draining and replacing iterators of the exact same length, but under the hood memmove was being called a lot, causing a slowdown on glibc 2.23. It turns out that at least one of the optimizations in glibc 2.24 was that `memmove` where the src/dst are equal becomes much faster. [This program][prog] executes in ~2.5s against glibc 2.23 and ~0.3s against glibc 2.24, exhibiting how glibc 2.24 is optimizing `memmove` if the src/dst are equal. And all that brings us to what this commit itself is doing. The change here is purely to `Drop for Drain` to avoid the call to `ptr::copy` if the region being copied doesn't actually need to be copied. For normal usage of just `Drain` itself this check isn't really necessary, but because `Splice` internally contains `Drain` this provides a nice speed boost on glibc 2.23. Overall this should fix the regression seen in #50496 on glibc 2.23 and also fix the regression on Windows where `memmove` looks to not have this optimization. Note that the way `splice` was called in `expand_statements` would cause a quadratic number of elements to be copied via `memmove` which is likely why the tuple-stress benchmark showed such a severe regression. Closes #50496 [prog]: https://gist.github.com/alexcrichton/c05bc51c6771bba5ae5b57561a6c1cd3
2 parents ac287ed + 254b601 commit 1aac55d

File tree

1 file changed

+5
-3
lines changed

1 file changed

+5
-3
lines changed

src/liballoc/vec.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -2533,9 +2533,11 @@ impl<'a, T> Drop for Drain<'a, T> {
25332533
// memmove back untouched tail, update to new length
25342534
let start = source_vec.len();
25352535
let tail = self.tail_start;
2536-
let src = source_vec.as_ptr().offset(tail as isize);
2537-
let dst = source_vec.as_mut_ptr().offset(start as isize);
2538-
ptr::copy(src, dst, self.tail_len);
2536+
if tail != start {
2537+
let src = source_vec.as_ptr().offset(tail as isize);
2538+
let dst = source_vec.as_mut_ptr().offset(start as isize);
2539+
ptr::copy(src, dst, self.tail_len);
2540+
}
25392541
source_vec.set_len(start + self.tail_len);
25402542
}
25412543
}

0 commit comments

Comments
 (0)