Skip to content

Commit 01b35d8

Browse files
authored
Auto merge of #36072 - arthurprs:binary_heap_opt, r=Aatch
Optimize BinaryHeap bounds checking I was experimenting with d-ary binary heaps during the weekend (dead end) and I found that we could get some good improvements by removing bounds checking. Specially due to the panic-safe additional code, llvm can't really optimize them out. ``` name d_ary_heap:: ns/iter std___heap:: ns/iter diff ns/iter diff % bench_build_insert 148,610 236,960 88,350 59.45% bench_from_vec 243,846 299,719 55,873 22.91% bench_insert_2000_empty 4,512 7,517 3,005 66.60% bench_insert_2000_prefilled 28,665 32,605 3,940 13.74% bench_pop_2000 111,515 128,005 16,490 14.79% bench_pop_all 2,759,945 3,317,626 557,681 20.21% peek_mut 23,186 23,635 449 1.94% pop_modify_push 41,573 43,822 2,249 5.41% test d_ary_heap::bench_build_insert ... bench: 148,610 ns/iter (+/- 10,687) test d_ary_heap::bench_from_vec ... bench: 243,846 ns/iter (+/- 16,500) test d_ary_heap::bench_insert_2000_empty ... bench: 4,512 ns/iter (+/- 136) test d_ary_heap::bench_insert_2000_prefilled ... bench: 28,665 ns/iter (+/- 1,347) test d_ary_heap::bench_pop_2000 ... bench: 111,515 ns/iter (+/- 104,677) test d_ary_heap::bench_pop_all ... bench: 2,759,945 ns/iter (+/- 173,838) test d_ary_heap::peek_mut ... bench: 23,186 ns/iter (+/- 106,254) test d_ary_heap::pop_modify_push ... bench: 41,573 ns/iter (+/- 3,313) test std___heap::bench_build_insert ... bench: 236,960 ns/iter (+/- 16,955) test std___heap::bench_from_vec ... bench: 299,719 ns/iter (+/- 6,354) test std___heap::bench_insert_2000_empty ... bench: 7,517 ns/iter (+/- 372) test std___heap::bench_insert_2000_prefilled ... bench: 32,605 ns/iter (+/- 2,433) test std___heap::bench_pop_2000 ... bench: 128,005 ns/iter (+/- 11,787) test std___heap::bench_pop_all ... bench: 3,317,626 ns/iter (+/- 238,968) test std___heap::peek_mut ... bench: 23,635 ns/iter (+/- 1,420) test std___heap::pop_modify_push ... bench: 43,822 ns/iter (+/- 3,788) ``` Test code: https://github.com/arthurprs/heap-experiments
2 parents a029ea3 + 175d434 commit 01b35d8

File tree

1 file changed

+23
-20
lines changed

1 file changed

+23
-20
lines changed

src/libcollections/binary_heap.rs

+23-20
Original file line numberDiff line numberDiff line change
@@ -884,58 +884,61 @@ struct Hole<'a, T: 'a> {
884884

885885
impl<'a, T> Hole<'a, T> {
886886
/// Create a new Hole at index `pos`.
887-
fn new(data: &'a mut [T], pos: usize) -> Self {
888-
unsafe {
889-
let elt = ptr::read(&data[pos]);
890-
Hole {
891-
data: data,
892-
elt: Some(elt),
893-
pos: pos,
894-
}
887+
///
888+
/// Unsafe because pos must be within the data slice.
889+
#[inline]
890+
unsafe fn new(data: &'a mut [T], pos: usize) -> Self {
891+
debug_assert!(pos < data.len());
892+
let elt = ptr::read(&data[pos]);
893+
Hole {
894+
data: data,
895+
elt: Some(elt),
896+
pos: pos,
895897
}
896898
}
897899

898-
#[inline(always)]
900+
#[inline]
899901
fn pos(&self) -> usize {
900902
self.pos
901903
}
902904

903905
/// Return a reference to the element removed
904-
#[inline(always)]
906+
#[inline]
905907
fn element(&self) -> &T {
906908
self.elt.as_ref().unwrap()
907909
}
908910

909911
/// Return a reference to the element at `index`.
910912
///
911-
/// Panics if the index is out of bounds.
912-
///
913-
/// Unsafe because index must not equal pos.
914-
#[inline(always)]
913+
/// Unsafe because index must be within the data slice and not equal to pos.
914+
#[inline]
915915
unsafe fn get(&self, index: usize) -> &T {
916916
debug_assert!(index != self.pos);
917-
&self.data[index]
917+
debug_assert!(index < self.data.len());
918+
self.data.get_unchecked(index)
918919
}
919920

920921
/// Move hole to new location
921922
///
922-
/// Unsafe because index must not equal pos.
923-
#[inline(always)]
923+
/// Unsafe because index must be within the data slice and not equal to pos.
924+
#[inline]
924925
unsafe fn move_to(&mut self, index: usize) {
925926
debug_assert!(index != self.pos);
926-
let index_ptr: *const _ = &self.data[index];
927-
let hole_ptr = &mut self.data[self.pos];
927+
debug_assert!(index < self.data.len());
928+
let index_ptr: *const _ = self.data.get_unchecked(index);
929+
let hole_ptr = self.data.get_unchecked_mut(self.pos);
928930
ptr::copy_nonoverlapping(index_ptr, hole_ptr, 1);
929931
self.pos = index;
930932
}
931933
}
932934

933935
impl<'a, T> Drop for Hole<'a, T> {
936+
#[inline]
934937
fn drop(&mut self) {
935938
// fill the hole again
936939
unsafe {
937940
let pos = self.pos;
938-
ptr::write(&mut self.data[pos], self.elt.take().unwrap());
941+
ptr::write(self.data.get_unchecked_mut(pos), self.elt.take().unwrap());
939942
}
940943
}
941944
}

0 commit comments

Comments
 (0)