Skip to content

Commit

Permalink
Fix BinaryHeap place by only constructing vec::PlaceBack once
Browse files Browse the repository at this point in the history
  • Loading branch information
Martin Hafskjold Thoresen committed Jan 17, 2017
1 parent fb3483c commit 90fbe15
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 50 deletions.
69 changes: 24 additions & 45 deletions src/libcollections/binary_heap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ impl<T: Ord> BinaryHeap<T> {
// the hole is filled back at the end of its scope, even on panic.
// Using a hole reduces the constant factor compared to using swaps,
// which involves twice as many moves.
fn sift_up(&mut self, start: usize, pos: usize) {
fn sift_up(&mut self, start: usize, pos: usize) -> usize {
unsafe {
// Take out the value at `pos` and create a hole.
let mut hole = Hole::new(&mut self.data, pos);
Expand All @@ -685,21 +685,6 @@ impl<T: Ord> BinaryHeap<T> {
}
hole.move_to(parent);
}
}
}

fn sift_up_ind(&mut self, start: usize, pos: usize) -> usize {
unsafe {
// Take out the value at `pos` and create a hole.
let mut hole = Hole::new(&mut self.data, pos);

while hole.pos() > start {
let parent = (hole.pos() - 1) / 2;
if hole.element() <= hole.get(parent) {
return hole.pos();
}
hole.move_to(parent);
}
hole.pos()
}
}
Expand Down Expand Up @@ -905,19 +890,6 @@ impl<T: Ord> BinaryHeap<T> {
}
}

impl<T> BinaryHeap<T>
where T: Clone + Ord {
/// kek
#[unstable(feature = "collection_placement",
reason = "placement protocol is subject to change",
issue = "30172")]
pub fn place(&mut self) -> PlaceIn<T> {
PlaceIn {
heap: self,
}
}
}

/// Hole represents a hole in a slice i.e. an index without valid value
/// (because it was moved from or duplicated).
/// In drop, `Hole` will restore the slice by filling the hole
Expand Down Expand Up @@ -1222,45 +1194,52 @@ impl<'a, T: 'a + Ord + Copy> Extend<&'a T> for BinaryHeap<T> {
#[unstable(feature = "collection_placement",
reason = "placement protocol is subject to change",
issue = "30172")]
pub struct PlaceIn<'a, T: 'a>
pub struct BinaryHeapPlace<'a, T: 'a>
where T: Clone + Ord {
heap: &'a mut BinaryHeap<T>,
heap: *mut BinaryHeap<T>,
place: vec::PlaceBack<'a, T>,
}

#[unstable(feature = "collection_placement",
reason = "placement protocol is subject to change",
issue = "30172")]
impl<'a, T> Place<T> for PlaceIn<'a, T>
impl<'a, T: 'a> Placer<T> for &'a mut BinaryHeap<T>
where T: Clone + Ord {
fn pointer(&mut self) -> *mut T {
self.heap.data.place_back().pointer()
type Place = BinaryHeapPlace<'a, T>;

fn make_place(self) -> Self::Place {
let ptr = self as *mut BinaryHeap<T>;
let place = Placer::make_place(self.data.place_back());
BinaryHeapPlace {
heap: ptr,
place: place,
}
}
}

#[unstable(feature = "collection_placement",
reason = "placement protocol is subject to change",
issue = "30172")]
impl<'a, T> Placer<T> for PlaceIn<'a, T>
impl<'a, T> Place<T> for BinaryHeapPlace<'a, T>
where T: Clone + Ord {
type Place = PlaceIn<'a, T>;

fn make_place(self) -> Self {
let _ = self.heap.data.place_back().make_place();
self
fn pointer(&mut self) -> *mut T {
self.place.pointer()
}
}

#[unstable(feature = "collection_placement",
reason = "placement protocol is subject to change",
issue = "30172")]
impl<'a, T> InPlace<T> for PlaceIn<'a, T>
impl<'a, T> InPlace<T> for BinaryHeapPlace<'a, T>
where T: Clone + Ord {
type Owner = &'a T;

unsafe fn finalize(self) -> &'a T {
let len = self.heap.len();
let _ = self.heap.data.place_back().finalize();
let i = self.heap.sift_up_ind(0, len);
&mut self.heap.data[i]
self.place.finalize();

let heap: &mut BinaryHeap<T> = &mut *self.heap;
let len = heap.len();
let i = heap.sift_up(0, len - 1);
heap.data.get_unchecked(i)
}
}
10 changes: 5 additions & 5 deletions src/libcollectionstest/binary_heap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,20 +314,20 @@ fn test_extend_specialization() {
#[test]
fn test_placement() {
let mut a = BinaryHeap::new();
a.place() <- 2;
a.place() <- 4;
a.place() <- 3;
&mut a <- 2;
&mut a <- 4;
&mut a <- 3;
assert_eq!(a.peek(), Some(&4));
assert_eq!(a.len(), 3);
a.place() <- 1;
&mut a <- 1;
assert_eq!(a.into_sorted_vec(), vec![1, 2, 3, 4]);
}

#[test]
fn test_placement_panic() {
let mut heap = BinaryHeap::from(vec![1, 2, 3]);
fn mkpanic() -> usize { panic!() }
let _ = panic::catch_unwind(panic::AssertUnwindSafe(|| { heap.place() <- mkpanic(); }));
let _ = panic::catch_unwind(panic::AssertUnwindSafe(|| { &mut heap <- mkpanic(); }));
assert_eq!(heap.len(), 3);
}

Expand Down

0 comments on commit 90fbe15

Please sign in to comment.