Skip to content

Commit

Permalink
Auto merge of #126557 - GrigorenkoPV:vec_track_caller, r=<try>
Browse files Browse the repository at this point in the history
Add `#[track_caller]` to allocating methods of `Vec` & `VecDeque`

Part 4 in a lengthy saga.
r? `@joshtriplett` because they were the reviewer the last 3 times.
`@bors` rollup=never "[just in case this has perf effects, Vec is hot](#79323 (comment))"

This was first attempted in #79323 by `@nvzqz.` It got approval from `@joshtriplett,` but rotted with merge conflicts and got closed.

Then it got picked up by `@Dylan-DPC-zz` in #83359. A benchmark was run[^perf], the results (after a bit of thinking[^thinking]) were deemed ok[^ok], but there was a typo[^typo] and the PR was made from a wrong remote in the first place[^remote], so #83909 was opened instead.

By the time #83909 rolled around, the methods in question had received some optimizations[^optimizations], so another perf run was conducted[^perf2]. The results were ok[^ok2]. There was a suggestion to add regression tests for panic behavior [^tests], but before it could be addressed, the PR fell victim to merge conflicts[^conflicts] and died again[^rip].

3 years have passed, and (from what I can tell) this has not been tried again, so here I am now, reviving this old effort.

Given how much time has passed and the fact that I've also touched `VecDeque` this time, it probably makes sense to
`@bors` try `@rust-timer`

[^perf]: #83359 (comment)
[^thinking]: #83359 (comment)
[^ok]: #83359 (comment)
[^typo]: #83359 (comment)
[^remote]: #83359 (comment)
[^optimizations]: #83909 (comment)
[^perf2]: #83909 (comment)
[^ok2]: #83909 (comment)
[^tests]: #83909 (comment)
[^conflicts]: #83909 (comment)
[^rip]: #83909 (comment)
  • Loading branch information
bors committed Jun 16, 2024
2 parents 55cac26 + 5451c6d commit 579ad25
Show file tree
Hide file tree
Showing 12 changed files with 102 additions and 4 deletions.
25 changes: 25 additions & 0 deletions library/alloc/src/collections/vec_deque/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ pub struct VecDeque<

#[stable(feature = "rust1", since = "1.0.0")]
impl<T: Clone, A: Allocator + Clone> Clone for VecDeque<T, A> {
#[track_caller]
fn clone(&self) -> Self {
let mut deq = Self::with_capacity_in(self.len(), self.allocator().clone());
deq.extend(self.iter().cloned());
Expand All @@ -117,6 +118,7 @@ impl<T: Clone, A: Allocator + Clone> Clone for VecDeque<T, A> {
///
/// This method is preferred over simply assigning `source.clone()` to `self`,
/// as it avoids reallocation if possible.
#[track_caller]
fn clone_from(&mut self, source: &Self) {
self.clear();
self.extend(source.iter().cloned());
Expand Down Expand Up @@ -480,6 +482,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
/// Frobs the head and tail sections around to handle the fact that we
/// just reallocated. Unsafe because it trusts old_capacity.
#[inline]
#[track_caller]
unsafe fn handle_capacity_increase(&mut self, old_capacity: usize) {
let new_capacity = self.capacity();
debug_assert!(new_capacity >= old_capacity);
Expand Down Expand Up @@ -560,6 +563,7 @@ impl<T> VecDeque<T> {
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[must_use]
#[track_caller]
pub fn with_capacity(capacity: usize) -> VecDeque<T> {
Self::with_capacity_in(capacity, Global)
}
Expand Down Expand Up @@ -615,6 +619,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
/// let deque: VecDeque<u32> = VecDeque::with_capacity(10);
/// ```
#[unstable(feature = "allocator_api", issue = "32838")]
#[track_caller]
pub fn with_capacity_in(capacity: usize, alloc: A) -> VecDeque<T, A> {
VecDeque { head: 0, len: 0, buf: RawVec::with_capacity_in(capacity, alloc) }
}
Expand Down Expand Up @@ -779,6 +784,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
///
/// [`reserve`]: VecDeque::reserve
#[stable(feature = "rust1", since = "1.0.0")]
#[track_caller]
pub fn reserve_exact(&mut self, additional: usize) {
let new_cap = self.len.checked_add(additional).expect("capacity overflow");
let old_cap = self.capacity();
Expand Down Expand Up @@ -808,6 +814,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
/// assert!(buf.capacity() >= 11);
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
#[track_caller]
pub fn reserve(&mut self, additional: usize) {
let new_cap = self.len.checked_add(additional).expect("capacity overflow");
let old_cap = self.capacity();
Expand Down Expand Up @@ -939,6 +946,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
/// assert!(buf.capacity() >= 4);
/// ```
#[stable(feature = "deque_extras_15", since = "1.5.0")]
#[track_caller]
pub fn shrink_to_fit(&mut self) {
self.shrink_to(0);
}
Expand All @@ -964,6 +972,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
/// assert!(buf.capacity() >= 4);
/// ```
#[stable(feature = "shrink_to", since = "1.56.0")]
#[track_caller]
pub fn shrink_to(&mut self, min_capacity: usize) {
let target_cap = min_capacity.max(self.len);

Expand Down Expand Up @@ -1729,6 +1738,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
/// assert_eq!(d.front(), Some(&2));
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
#[track_caller]
pub fn push_front(&mut self, value: T) {
if self.is_full() {
self.grow();
Expand Down Expand Up @@ -1756,6 +1766,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_confusables("push", "put", "append")]
#[track_caller]
pub fn push_back(&mut self, value: T) {
if self.is_full() {
self.grow();
Expand Down Expand Up @@ -1865,6 +1876,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
/// assert_eq!(vec_deque, &['a', 'd', 'b', 'c']);
/// ```
#[stable(feature = "deque_extras_15", since = "1.5.0")]
#[track_caller]
pub fn insert(&mut self, index: usize, value: T) {
assert!(index <= self.len(), "index out of bounds");
if self.is_full() {
Expand Down Expand Up @@ -1968,6 +1980,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
#[inline]
#[must_use = "use `.truncate()` if you don't need the other half"]
#[stable(feature = "split_off", since = "1.4.0")]
#[track_caller]
pub fn split_off(&mut self, at: usize) -> Self
where
A: Clone,
Expand Down Expand Up @@ -2034,6 +2047,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
/// ```
#[inline]
#[stable(feature = "append", since = "1.4.0")]
#[track_caller]
pub fn append(&mut self, other: &mut Self) {
if T::IS_ZST {
self.len = self.len.checked_add(other.len).expect("capacity overflow");
Expand Down Expand Up @@ -2156,6 +2170,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
// be called in cold paths.
// This may panic or abort
#[inline(never)]
#[track_caller]
fn grow(&mut self) {
// Extend or possibly remove this assertion when valid use-cases for growing the
// buffer without it being full emerge
Expand Down Expand Up @@ -2194,6 +2209,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
/// assert_eq!(buf, [5, 10, 101, 102, 103]);
/// ```
#[stable(feature = "vec_resize_with", since = "1.33.0")]
#[track_caller]
pub fn resize_with(&mut self, new_len: usize, generator: impl FnMut() -> T) {
let len = self.len;

Expand Down Expand Up @@ -2740,6 +2756,7 @@ impl<T: Clone, A: Allocator> VecDeque<T, A> {
/// assert_eq!(buf, [5, 10, 20, 20, 20]);
/// ```
#[stable(feature = "deque_extras", since = "1.16.0")]
#[track_caller]
pub fn resize(&mut self, new_len: usize, value: T) {
if new_len > self.len() {
let extra = new_len - self.len();
Expand Down Expand Up @@ -2859,6 +2876,7 @@ impl<T, A: Allocator> IndexMut<usize> for VecDeque<T, A> {

#[stable(feature = "rust1", since = "1.0.0")]
impl<T> FromIterator<T> for VecDeque<T> {
#[track_caller]
fn from_iter<I: IntoIterator<Item = T>>(iter: I) -> VecDeque<T> {
SpecFromIter::spec_from_iter(iter.into_iter())
}
Expand Down Expand Up @@ -2898,33 +2916,39 @@ impl<'a, T, A: Allocator> IntoIterator for &'a mut VecDeque<T, A> {

#[stable(feature = "rust1", since = "1.0.0")]
impl<T, A: Allocator> Extend<T> for VecDeque<T, A> {
#[track_caller]
fn extend<I: IntoIterator<Item = T>>(&mut self, iter: I) {
<Self as SpecExtend<T, I::IntoIter>>::spec_extend(self, iter.into_iter());
}

#[inline]
#[track_caller]
fn extend_one(&mut self, elem: T) {
self.push_back(elem);
}

#[inline]
#[track_caller]
fn extend_reserve(&mut self, additional: usize) {
self.reserve(additional);
}
}

#[stable(feature = "extend_ref", since = "1.2.0")]
impl<'a, T: 'a + Copy, A: Allocator> Extend<&'a T> for VecDeque<T, A> {
#[track_caller]
fn extend<I: IntoIterator<Item = &'a T>>(&mut self, iter: I) {
self.spec_extend(iter.into_iter());
}

#[inline]
#[track_caller]
fn extend_one(&mut self, &elem: &'a T) {
self.push_back(elem);
}

#[inline]
#[track_caller]
fn extend_reserve(&mut self, additional: usize) {
self.reserve(additional);
}
Expand Down Expand Up @@ -3014,6 +3038,7 @@ impl<T, const N: usize> From<[T; N]> for VecDeque<T> {
/// let deq2: VecDeque<_> = [1, 2, 3, 4].into();
/// assert_eq!(deq1, deq2);
/// ```
#[track_caller]
fn from(arr: [T; N]) -> Self {
let mut deq = VecDeque::with_capacity(N);
let arr = ManuallyDrop::new(arr);
Expand Down
6 changes: 6 additions & 0 deletions library/alloc/src/collections/vec_deque/spec_extend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ use super::VecDeque;

// Specialization trait used for VecDeque::extend
pub(super) trait SpecExtend<T, I> {
#[track_caller]
fn spec_extend(&mut self, iter: I);
}

impl<T, I, A: Allocator> SpecExtend<T, I> for VecDeque<T, A>
where
I: Iterator<Item = T>,
{
#[track_caller]
default fn spec_extend(&mut self, mut iter: I) {
// This function should be the moral equivalent of:
//
Expand Down Expand Up @@ -53,6 +55,7 @@ impl<T, I, A: Allocator> SpecExtend<T, I> for VecDeque<T, A>
where
I: TrustedLen<Item = T>,
{
#[track_caller]
default fn spec_extend(&mut self, iter: I) {
// This is the case for a TrustedLen iterator.
let (low, high) = iter.size_hint();
Expand Down Expand Up @@ -85,6 +88,7 @@ where
}

impl<T, A: Allocator> SpecExtend<T, vec::IntoIter<T>> for VecDeque<T, A> {
#[track_caller]
fn spec_extend(&mut self, mut iterator: vec::IntoIter<T>) {
let slice = iterator.as_slice();
self.reserve(slice.len());
Expand All @@ -102,6 +106,7 @@ where
I: Iterator<Item = &'a T>,
T: Copy,
{
#[track_caller]
default fn spec_extend(&mut self, iterator: I) {
self.spec_extend(iterator.copied())
}
Expand All @@ -111,6 +116,7 @@ impl<'a, T: 'a, A: Allocator> SpecExtend<&'a T, slice::Iter<'a, T>> for VecDeque
where
T: Copy,
{
#[track_caller]
fn spec_extend(&mut self, iterator: slice::Iter<'a, T>) {
let slice = iterator.as_slice();
self.reserve(slice.len());
Expand Down
1 change: 1 addition & 0 deletions library/alloc/src/collections/vec_deque/spec_from_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ impl<T, I> SpecFromIter<T, I> for VecDeque<T>
where
I: Iterator<Item = T>,
{
#[track_caller]
default fn spec_from_iter(iterator: I) -> Self {
// Since converting is O(1) now, just re-use the `Vec` logic for
// anything where we can't do something extra-special for `VecDeque`,
Expand Down
13 changes: 11 additions & 2 deletions library/alloc/src/raw_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ mod tests;
// only one location which panics rather than a bunch throughout the module.
#[cfg(not(no_global_oom_handling))]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
fn capacity_overflow() -> ! {
#[track_caller]
pub(crate) fn capacity_overflow() -> ! {
panic!("capacity overflow");
}

Expand Down Expand Up @@ -113,6 +114,7 @@ impl<T> RawVec<T, Global> {
#[cfg(not(any(no_global_oom_handling, test)))]
#[must_use]
#[inline]
#[track_caller]
pub fn with_capacity(capacity: usize) -> Self {
match Self::try_allocate_in(capacity, AllocInit::Uninitialized, Global) {
Ok(res) => res,
Expand All @@ -124,6 +126,7 @@ impl<T> RawVec<T, Global> {
#[cfg(not(any(no_global_oom_handling, test)))]
#[must_use]
#[inline]
#[track_caller]
pub fn with_capacity_zeroed(capacity: usize) -> Self {
Self::with_capacity_zeroed_in(capacity, Global)
}
Expand Down Expand Up @@ -154,6 +157,7 @@ impl<T, A: Allocator> RawVec<T, A> {
/// allocator for the returned `RawVec`.
#[cfg(not(no_global_oom_handling))]
#[inline]
#[track_caller]
pub fn with_capacity_in(capacity: usize, alloc: A) -> Self {
match Self::try_allocate_in(capacity, AllocInit::Uninitialized, alloc) {
Ok(res) => res,
Expand All @@ -172,6 +176,7 @@ impl<T, A: Allocator> RawVec<T, A> {
/// of allocator for the returned `RawVec`.
#[cfg(not(no_global_oom_handling))]
#[inline]
#[track_caller]
pub fn with_capacity_zeroed_in(capacity: usize, alloc: A) -> Self {
match Self::try_allocate_in(capacity, AllocInit::Zeroed, alloc) {
Ok(res) => res,
Expand Down Expand Up @@ -335,6 +340,7 @@ impl<T, A: Allocator> RawVec<T, A> {
/// Aborts on OOM.
#[cfg(not(no_global_oom_handling))]
#[inline]
#[track_caller]
pub fn reserve(&mut self, len: usize, additional: usize) {
// Callers expect this function to be very cheap when there is already sufficient capacity.
// Therefore, we move all the resizing and error-handling logic from grow_amortized and
Expand All @@ -360,6 +366,7 @@ impl<T, A: Allocator> RawVec<T, A> {
/// caller to ensure `len == self.capacity()`.
#[cfg(not(no_global_oom_handling))]
#[inline(never)]
#[track_caller]
pub fn grow_one(&mut self) {
if let Err(err) = self.grow_amortized(self.cap.0, 1) {
handle_error(err);
Expand Down Expand Up @@ -396,6 +403,7 @@ impl<T, A: Allocator> RawVec<T, A> {
///
/// Aborts on OOM.
#[cfg(not(no_global_oom_handling))]
#[track_caller]
pub fn reserve_exact(&mut self, len: usize, additional: usize) {
if let Err(err) = self.try_reserve_exact(len, additional) {
handle_error(err);
Expand Down Expand Up @@ -429,6 +437,7 @@ impl<T, A: Allocator> RawVec<T, A> {
///
/// Aborts on OOM.
#[cfg(not(no_global_oom_handling))]
#[track_caller]
pub fn shrink_to_fit(&mut self, cap: usize) {
if let Err(err) = self.shrink(cap) {
handle_error(err);
Expand Down Expand Up @@ -510,7 +519,6 @@ impl<T, A: Allocator> RawVec<T, A> {
Ok(())
}

#[cfg(not(no_global_oom_handling))]
fn shrink(&mut self, cap: usize) -> Result<(), TryReserveError> {
assert!(cap <= self.capacity(), "Tried to shrink to a larger capacity");

Expand Down Expand Up @@ -588,6 +596,7 @@ unsafe impl<#[may_dangle] T, A: Allocator> Drop for RawVec<T, A> {
// Central function for reserve error handling.
#[cfg(not(no_global_oom_handling))]
#[cold]
#[track_caller]
fn handle_error(e: TryReserveError) -> ! {
match e.kind() {
CapacityOverflow => capacity_overflow(),
Expand Down
1 change: 1 addition & 0 deletions library/alloc/src/vec/cow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ impl<'a, T> FromIterator<T> for Cow<'a, [T]>
where
T: Clone,
{
#[track_caller]
fn from_iter<I: IntoIterator<Item = T>>(it: I) -> Cow<'a, [T]> {
Cow::Owned(FromIterator::from_iter(it))
}
Expand Down
2 changes: 2 additions & 0 deletions library/alloc/src/vec/in_place_collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ where
I: Iterator<Item = T> + InPlaceCollect,
<I as SourceIter>::Source: AsVecIntoIter,
{
#[track_caller]
default fn from_iter(iterator: I) -> Self {
// Select the implementation in const eval to avoid codegen of the dead branch to improve compile times.
let fun: fn(I) -> Vec<T> = const {
Expand All @@ -246,6 +247,7 @@ where
}
}

#[track_caller]
fn from_iter_in_place<I, T>(mut iterator: I) -> Vec<T>
where
I: Iterator<Item = T> + InPlaceCollect,
Expand Down
Loading

0 comments on commit 579ad25

Please sign in to comment.