Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use drop_in_place in Vec and VecDeque #32012

Merged
merged 3 commits into from
Mar 3, 2016
Merged

Conversation

bluss
Copy link
Member

@bluss bluss commented Mar 2, 2016

Use drop_in_place in Vec and VecDeque

We can use drop_in_place's DST capabilities directly in Vec::drop and similarly in VecDeque::drop. I verfied this has the same effect as the previous needs_drop code; drop_in_place it itself an intrinsic.

The VecDeque replacement should be more efficient too, even in release mode (slice iteration makes a more efficient loop than the deque iterator).

Directly use the drop glue for [T]. Still optimizes out in -O1 like with
the `needs_drop` intrinsic.
Just like for Vec. This should benefit both non-optimized and optimized
performance. Non-optimized since the intrinsic drop_in_place is easily
removed, and optimized because iterating the slices is more efficient
than using the VecDeque iterators.
@rust-highfive
Copy link
Collaborator

r? @gankro

(rust_highfive has picked a reviewer for you, use r? to override)

}
}
// use drop for [T]
ptr::drop_in_place(&mut self[..]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa I had no idea this was possible, nifty!

@alexcrichton
Copy link
Member

@bors: r+ 7ceafae

Nice!

bors added a commit that referenced this pull request Mar 3, 2016
Use `drop_in_place` in Vec and VecDeque

We can use drop_in_place's DST capabilities directly in Vec::drop and similarly in VecDeque::drop. I verfied this has the same effect as the previous `needs_drop` code; `drop_in_place` it itself an intrinsic.

The VecDeque replacement should be more efficient too, even in release mode (slice iteration makes a more efficient loop than the deque iterator).
@bors
Copy link
Contributor

bors commented Mar 3, 2016

⌛ Testing commit 7ceafae with merge 4aa913c...

@bors bors merged commit 7ceafae into rust-lang:master Mar 3, 2016
@bluss bluss deleted the more-drop-in-place branch March 7, 2016 20:23
mbrubeck added a commit to mbrubeck/rust-smallvec that referenced this pull request Jun 6, 2018
bors-servo pushed a commit to servo/rust-smallvec that referenced this pull request Jun 6, 2018
Use a single ptr::drop_in_place call instead of a loop

This is what the standard Vec does; see rust-lang/rust#32012.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-smallvec/100)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants