From fb3483c827e1ef8ad73b200fb73f1dd03994e510 Mon Sep 17 00:00:00 2001 From: Martin Hafskjold Thoresen Date: Sat, 14 Jan 2017 13:58:41 +0100 Subject: [PATCH 1/2] Add initial impl of placement-in for `BinaryHeap` --- src/libcollections/binary_heap.rs | 77 ++++++++++++++++++++++++++- src/libcollectionstest/binary_heap.rs | 21 ++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/src/libcollections/binary_heap.rs b/src/libcollections/binary_heap.rs index 7d245f79f699..8fcc7e0d4da4 100644 --- a/src/libcollections/binary_heap.rs +++ b/src/libcollections/binary_heap.rs @@ -151,7 +151,7 @@ #![allow(missing_docs)] #![stable(feature = "rust1", since = "1.0.0")] -use core::ops::{Deref, DerefMut}; +use core::ops::{Deref, DerefMut, Place, Placer, InPlace}; use core::iter::{FromIterator, FusedIterator}; use core::mem::{swap, size_of}; use core::ptr; @@ -688,6 +688,22 @@ impl BinaryHeap { } } + 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() + } + } + /// Take an element at `pos` and move it down the heap, /// while its children are larger. fn sift_down_range(&mut self, pos: usize, end: usize) { @@ -889,6 +905,19 @@ impl BinaryHeap { } } +impl BinaryHeap +where T: Clone + Ord { + /// kek + #[unstable(feature = "collection_placement", + reason = "placement protocol is subject to change", + issue = "30172")] + pub fn place(&mut self) -> PlaceIn { + 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 @@ -1189,3 +1218,49 @@ impl<'a, T: 'a + Ord + Copy> Extend<&'a T> for BinaryHeap { self.extend(iter.into_iter().cloned()); } } + +#[unstable(feature = "collection_placement", + reason = "placement protocol is subject to change", + issue = "30172")] +pub struct PlaceIn<'a, T: 'a> +where T: Clone + Ord { + heap: &'a mut BinaryHeap, +} + +#[unstable(feature = "collection_placement", + reason = "placement protocol is subject to change", + issue = "30172")] +impl<'a, T> Place for PlaceIn<'a, T> +where T: Clone + Ord { + fn pointer(&mut self) -> *mut T { + self.heap.data.place_back().pointer() + } +} + +#[unstable(feature = "collection_placement", + reason = "placement protocol is subject to change", + issue = "30172")] +impl<'a, T> Placer for PlaceIn<'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 + } +} + +#[unstable(feature = "collection_placement", + reason = "placement protocol is subject to change", + issue = "30172")] +impl<'a, T> InPlace for PlaceIn<'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] + } +} diff --git a/src/libcollectionstest/binary_heap.rs b/src/libcollectionstest/binary_heap.rs index 1df341d1fc28..a0846f570820 100644 --- a/src/libcollectionstest/binary_heap.rs +++ b/src/libcollectionstest/binary_heap.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use std::panic; use std::collections::BinaryHeap; use std::collections::binary_heap::{Drain, PeekMut}; @@ -310,6 +311,26 @@ fn test_extend_specialization() { assert_eq!(a.into_sorted_vec(), [-20, -10, 1, 2, 3, 3, 5, 43]); } +#[test] +fn test_placement() { + let mut a = BinaryHeap::new(); + a.place() <- 2; + a.place() <- 4; + a.place() <- 3; + assert_eq!(a.peek(), Some(&4)); + assert_eq!(a.len(), 3); + a.place() <- 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(); })); + assert_eq!(heap.len(), 3); +} + #[allow(dead_code)] fn assert_covariance() { fn drain<'new>(d: Drain<'static, &'static str>) -> Drain<'new, &'new str> { From 90fbe155f2104c9cbbec5ff1e79ee513e112fa9c Mon Sep 17 00:00:00 2001 From: Martin Hafskjold Thoresen Date: Tue, 17 Jan 2017 18:58:49 +0100 Subject: [PATCH 2/2] Fix BinaryHeap place by only constructing vec::PlaceBack once --- src/libcollections/binary_heap.rs | 69 ++++++++++----------------- src/libcollectionstest/binary_heap.rs | 10 ++-- 2 files changed, 29 insertions(+), 50 deletions(-) diff --git a/src/libcollections/binary_heap.rs b/src/libcollections/binary_heap.rs index 8fcc7e0d4da4..b7c2a708baf4 100644 --- a/src/libcollections/binary_heap.rs +++ b/src/libcollections/binary_heap.rs @@ -673,7 +673,7 @@ impl BinaryHeap { // 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); @@ -685,21 +685,6 @@ impl BinaryHeap { } 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() } } @@ -905,19 +890,6 @@ impl BinaryHeap { } } -impl BinaryHeap -where T: Clone + Ord { - /// kek - #[unstable(feature = "collection_placement", - reason = "placement protocol is subject to change", - issue = "30172")] - pub fn place(&mut self) -> PlaceIn { - 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 @@ -1222,45 +1194,52 @@ impl<'a, T: 'a + Ord + Copy> Extend<&'a T> for BinaryHeap { #[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, + heap: *mut BinaryHeap, + place: vec::PlaceBack<'a, T>, } #[unstable(feature = "collection_placement", reason = "placement protocol is subject to change", issue = "30172")] -impl<'a, T> Place for PlaceIn<'a, T> +impl<'a, T: 'a> Placer for &'a mut BinaryHeap 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; + 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 for PlaceIn<'a, T> +impl<'a, T> Place 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 for PlaceIn<'a, T> +impl<'a, T> InPlace 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 = &mut *self.heap; + let len = heap.len(); + let i = heap.sift_up(0, len - 1); + heap.data.get_unchecked(i) } } diff --git a/src/libcollectionstest/binary_heap.rs b/src/libcollectionstest/binary_heap.rs index a0846f570820..d284937a9e67 100644 --- a/src/libcollectionstest/binary_heap.rs +++ b/src/libcollectionstest/binary_heap.rs @@ -314,12 +314,12 @@ 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]); } @@ -327,7 +327,7 @@ fn test_placement() { 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); }