Skip to content

Commit

Permalink
Split Vec::dedup_by into 2 cycles
Browse files Browse the repository at this point in the history
First cycle runs until we found 2 same elements, second runs after if there any found in the first one. This allows to avoid any memory writes until we found an item which we want to remove.

This leads to significant performance gains if all `Vec` items are kept: -40% on my benchmark with unique integers.

Results of benchmarks before implementation (including new benchmark where nothing needs to be removed):
 *   vec::bench_dedup_all_100                 74.00ns/iter  +/- 13.00ns
 *   vec::bench_dedup_all_1000               572.00ns/iter +/- 272.00ns
 *   vec::bench_dedup_all_100000              64.42µs/iter  +/- 19.47µs
 *   __vec::bench_dedup_none_100                67.00ns/iter  +/- 17.00ns__
 *   __vec::bench_dedup_none_1000              662.00ns/iter  +/- 86.00ns__
 *   __vec::bench_dedup_none_10000               9.16µs/iter   +/- 2.71µs__
 *   __vec::bench_dedup_none_100000             91.25µs/iter   +/- 1.82µs__
 *   vec::bench_dedup_random_100             105.00ns/iter  +/- 11.00ns
 *   vec::bench_dedup_random_1000            781.00ns/iter  +/- 10.00ns
 *   vec::bench_dedup_random_10000             9.00µs/iter   +/- 5.62µs
 *   vec::bench_dedup_random_100000          449.81µs/iter  +/- 74.99µs
 *   vec::bench_dedup_slice_truncate_100     105.00ns/iter  +/- 16.00ns
 *   vec::bench_dedup_slice_truncate_1000      2.65µs/iter +/- 481.00ns
 *   vec::bench_dedup_slice_truncate_10000    18.33µs/iter   +/- 5.23µs
 *   vec::bench_dedup_slice_truncate_100000  501.12µs/iter  +/- 46.97µs

Results after implementation:
 *   vec::bench_dedup_all_100                 75.00ns/iter   +/- 9.00ns
 *   vec::bench_dedup_all_1000               494.00ns/iter +/- 117.00ns
 *   vec::bench_dedup_all_100000              58.13µs/iter   +/- 8.78µs
 *   __vec::bench_dedup_none_100                52.00ns/iter  +/- 22.00ns__
 *   __vec::bench_dedup_none_1000              417.00ns/iter +/- 116.00ns__
 *   __vec::bench_dedup_none_10000               4.11µs/iter +/- 546.00ns__
 *   __vec::bench_dedup_none_100000             40.47µs/iter   +/- 5.36µs__
 *   vec::bench_dedup_random_100              77.00ns/iter  +/- 15.00ns
 *   vec::bench_dedup_random_1000            681.00ns/iter  +/- 86.00ns
 *   vec::bench_dedup_random_10000            11.66µs/iter   +/- 2.22µs
 *   vec::bench_dedup_random_100000          469.35µs/iter  +/- 20.53µs
 *   vec::bench_dedup_slice_truncate_100     100.00ns/iter   +/- 5.00ns
 *   vec::bench_dedup_slice_truncate_1000      2.55µs/iter +/- 224.00ns
 *   vec::bench_dedup_slice_truncate_10000    18.95µs/iter   +/- 2.59µs
 *   vec::bench_dedup_slice_truncate_100000  492.85µs/iter  +/- 72.84µs

Resolves #77772
  • Loading branch information
AngelicosPhosphoros committed Nov 25, 2023
1 parent 0c6901a commit f0c9274
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 13 deletions.
2 changes: 1 addition & 1 deletion library/alloc/benches/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ fn bench_vec_dedup_none(b: &mut Bencher, sz: usize) {
black_box(vec.first());
// Unlike other benches of `dedup`
// this doesn't reinitialize vec
// because we measure how effecient dedup is
// because we measure how efficient dedup is
// when no memory written
});
}
Expand Down
56 changes: 44 additions & 12 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1775,7 +1775,31 @@ impl<T, A: Allocator> Vec<T, A> {
return;
}

/* INVARIANT: vec.len() > read >= write > write-1 >= 0 */
// Check if we ever want to remove anything.
// This allows to use copy_non_overlapping in next cycle.
// And avoids any memory writes if we don't need to remove anything.
let mut possible_remove_idx = 1;
let start = self.as_mut_ptr();
while possible_remove_idx < len {
let found_duplicate = unsafe {
// SAFETY: possible_remove_idx always in range [1..len)
// Note that we start iteration from 1 so we never overflow.
let prev = start.add(possible_remove_idx.wrapping_sub(1));
let current = start.add(possible_remove_idx);
// We explicitly say in docs that references are reversed.
same_bucket(&mut *current, &mut *prev)
};
if found_duplicate {
break;
}
possible_remove_idx += 1;
}
// Don't need to remove anything.
if possible_remove_idx >= len {
return;
}

/* INVARIANT: vec.len() > read > write > write-1 >= 0 */
struct FillGapOnDrop<'a, T, A: core::alloc::Allocator> {
/* Offset of the element we want to check if it is duplicate */
read: usize,
Expand Down Expand Up @@ -1821,31 +1845,39 @@ impl<T, A: Allocator> Vec<T, A> {
}
}

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 */

// Construct gap first and then drop item to avoid memory corruption if `T::drop` panics.
let mut gap =
FillGapOnDrop { read: possible_remove_idx + 1, write: possible_remove_idx, vec: self };
unsafe {
// SAFETY: we checked that possible_remove_idx < len before.
// If drop panics, `gap` would remove this item without drop.
ptr::drop_in_place(start.add(possible_remove_idx));
}

/* 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);
let prev_ptr = ptr.add(gap.write.wrapping_sub(1));
let read_ptr = start.add(gap.read);
let prev_ptr = start.add(gap.write.wrapping_sub(1));

if same_bucket(&mut *read_ptr, &mut *prev_ptr) {
// We explicitly say in docs that references are reversed.
let found_duplicate = same_bucket(&mut *read_ptr, &mut *prev_ptr);
if found_duplicate {
// Increase `gap.read` now since the drop may panic.
gap.read += 1;
/* We have found duplicate, drop it in-place */
ptr::drop_in_place(read_ptr);
} else {
let write_ptr = ptr.add(gap.write);
let write_ptr = start.add(gap.write);

/* Because `read_ptr` can be equal to `write_ptr`, we either
* have to use `copy` or conditional `copy_nonoverlapping`.
* Looks like the first option is faster. */
ptr::copy(read_ptr, write_ptr, 1);
/* read_ptr cannot be equal to write_ptr because at this point
* we guaranteed to skip at least one element (before loop starts).
*/
ptr::copy_nonoverlapping(read_ptr, write_ptr, 1);

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

0 comments on commit f0c9274

Please sign in to comment.