Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vec::dedup_by optimization #82191

Merged
merged 7 commits into from
Mar 18, 2021
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 93 additions & 6 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1514,15 +1514,102 @@ impl<T, A: Allocator> Vec<T, A> {
/// assert_eq!(vec, ["foo", "bar", "baz", "bar"]);
/// ```
#[stable(feature = "dedup_by", since = "1.16.0")]
pub fn dedup_by<F>(&mut self, same_bucket: F)
pub fn dedup_by<F>(&mut self, mut same_bucket: F)
where
F: FnMut(&mut T, &mut T) -> bool,
{
let len = {
let (dedup, _) = self.as_mut_slice().partition_dedup_by(same_bucket);
dedup.len()
};
self.truncate(len);
let len = self.len();
if len <= 1 {
return;
}

/* INVARIANT: vec.len() > read >= write > write-1 >= 0 */
nagisa marked this conversation as resolved.
Show resolved Hide resolved
struct FillGapOnDrop<'a, T, A: core::alloc::Allocator> {
/* Offset of the element we want to check if it is duplicate */
read: usize,

/* Offset of the place where we want to place the non-duplicate
* when we find it. */
write: usize,

/* The Vec that would need correction if `same_bucket` panicked */
vec: &'a mut Vec<T, A>,
}

impl<'a, T, A: core::alloc::Allocator> Drop for FillGapOnDrop<'a, T, A> {
fn drop(&mut self) {
/* This code gets executed either at the end of `dedup_by` or
* when `same_bucket` panics */

/* SAFETY (if finishing successfully): self.read == len, so
nagisa marked this conversation as resolved.
Show resolved Hide resolved
* no data is copied and length is set correctly */

/* SAFETY (if panicing): invariant guarantees that `read - write`
* and `len - read` never overflow and that the copy is always
* in-bounds. */
unsafe {
let ptr = self.vec.as_mut_ptr();
let len = self.vec.len();

/* How many items were left when `same_bucket` paniced.
* Basically vec[read..].len() */
let items_left = len - self.read;

/* Pointer to first item in vec[write..write+items_left] slice */
let dropped_ptr = ptr.add(self.write);
/* Pointer to first item in vec[read..] slice */
let valid_ptr = ptr.add(self.read);

/* Copy `vec[read..]` to `vec[write..write+items_left]`.
* The slices can overlap, so `copy_nonoverlapping` cannot be used */
ptr::copy(valid_ptr, dropped_ptr, items_left);

/* How many items have been already dropped
* Basically vec[read..write].len() */
let dropped = self.read - self.write;

self.vec.set_len(len - dropped);
}
}
}

let mut gap = FillGapOnDrop { read: 1, write: 1, vec: self };

let ptr = gap.vec.as_mut_ptr();

/* Drop items while going through Vec, it should be more efficient than
* doing slice partition_dedup + truncate */

/* SAFETY: Because of the invariant, read_ptr, prev_ptr and write_ptr
* are always in-bounds and read_ptr never aliases prev_ptr */
unsafe {
while gap.read < len {
let read_ptr = ptr.add(gap.read);
nagisa marked this conversation as resolved.
Show resolved Hide resolved
let prev_ptr = ptr.add(gap.write.wrapping_sub(1));
nagisa marked this conversation as resolved.
Show resolved Hide resolved

if same_bucket(&mut *read_ptr, &mut *prev_ptr) {
/* We have found duplicate, drop it in-place */
ptr::drop_in_place(read_ptr);
} else {
let write_ptr = ptr.add(gap.write);

/* Looks like doing just `copy` can be faster than
* conditional `copy_nonoverlapping` */
ptr::copy(read_ptr, write_ptr, 1);
nagisa marked this conversation as resolved.
Show resolved Hide resolved

/* We have filled that place, so go further */
gap.write += 1;
}

gap.read += 1;
}

/* Technically we could let `gap` clean up with its Drop, but
* when `same_bucket` is guaranteed to not panic, this bloats a little
* the codegen, so we just do it manually */
gap.vec.set_len(gap.write);
mem::forget(gap);
}
}

/// Appends an element to the back of a collection.
Expand Down