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.

Sign up for GitHub

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

Jump to bottom

Implement Vec::from_elem specialization for all Copy types #41335

Closed
wants to merge 2 commits into from
Closed

Implement Vec::from_elem specialization for all Copy types #41335

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
55e52f3
Implement `Vec::from_elem` specialization for all `Copy` types
ranma42 Apr 17, 2017
a53dfbe
Add explanation for `is_zero` and `chunked_or`
ranma42 Apr 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/libcollections/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
#![feature(box_patterns)]
#![feature(box_syntax)]
#![cfg_attr(not(test), feature(char_escape_debug))]
#![cfg_attr(not(test), feature(core_float))]
#![feature(core_intrinsics)]
#![feature(dropck_eyepatch)]
#![feature(exact_size_is_empty)]
Expand Down
77 changes: 32 additions & 45 deletions src/libcollections/vec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,6 @@ use core::hash::{self, Hash};
use core::intrinsics::{arith_offset, assume};
use core::iter::{FromIterator, FusedIterator, TrustedLen};
use core::mem;
#[cfg(not(test))]
use core::num::Float;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is why I think attributes should be on the same line with use/mod/extern crate items.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Argh, sorry! It should now be fixed.

use core::ops::{InPlace, Index, IndexMut, Place, Placer};
use core::ops;
use core::ptr;
Expand Down Expand Up @@ -1388,59 +1386,48 @@ impl<T: Clone> SpecFromElem for T {
}
}

impl SpecFromElem for u8 {
#[inline]
fn from_elem(elem: u8, n: usize) -> Vec<u8> {
if elem == 0 {
unsafe fn chunked_or<T, U: ops::BitOr<Output = U> + Copy>(x: T) -> U {
let p = &x as *const T as *const U;
let len = mem::size_of::<T>() / mem::size_of::<U>();
slice::from_raw_parts(p, len).iter().fold(mem::zeroed(), |state, &x| state | x)
}

fn is_zero<T: Copy>(x: T) -> bool {
unsafe {
match mem::align_of::<T>() {
n if n % 16 == 0 => 0u128 == chunked_or(x),
n if n % 8 == 0 => 0u64 == chunked_or(x),
n if n % 4 == 0 => 0u32 == chunked_or(x),
n if n % 2 == 0 => 0u16 == chunked_or(x),
_ => 0u8 == chunked_or(x),
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you write some comments explaining how chunked_or and is_zero work? Are there any pitfalls? What happens when the alignment of a type is bigger than 16?

Copy link
Contributor Author

@ranma42 ranma42 Apr 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I added some comments that should answer your questions, with the exception of "are there any pitfalls?".
There should be no correctness issues, but there is certainly a performance tradeoff: we are now checking the contents of the input and we pay for that.
We are making vec![None; 1024*1024] much faster, but vec![Some(3); 1] is going to be a little slower (if the compiler can see the literal value, the checks are going to be optimized away, but in general that might not be possible).


impl<T: Copy> SpecFromElem for T {
default fn from_elem(elem: Self, n: usize) -> Vec<Self> {
if is_zero(elem) {
return Vec {
buf: RawVec::with_capacity_zeroed(n),
len: n,
}
}
unsafe {
let mut v = Vec::with_capacity(n);
ptr::write_bytes(v.as_mut_ptr(), elem, n);
v.set_len(n);
v
}
}
}

macro_rules! impl_spec_from_elem {
($t: ty, $is_zero: expr) => {
impl SpecFromElem for $t {
#[inline]
fn from_elem(elem: $t, n: usize) -> Vec<$t> {
if $is_zero(elem) {
return Vec {
buf: RawVec::with_capacity_zeroed(n),
len: n,
}
}
let mut v = Vec::with_capacity(n);
v.extend_with_element(n, elem);
v
let mut v = Vec::with_capacity(n);
if mem::size_of::<T>() == 1 {
unsafe {
// let elem: u8 = mem::transmute(elem);
let elem: u8 = *(&elem as *const T as *const u8);
ptr::write_bytes(v.as_mut_ptr(), elem, n);
v.set_len(n);
}
} else {
v.extend_with_element(n, elem);
}
};
v
}
}

impl_spec_from_elem!(i8, |x| x == 0);
impl_spec_from_elem!(i16, |x| x == 0);
impl_spec_from_elem!(i32, |x| x == 0);
impl_spec_from_elem!(i64, |x| x == 0);
impl_spec_from_elem!(i128, |x| x == 0);
impl_spec_from_elem!(isize, |x| x == 0);

impl_spec_from_elem!(u16, |x| x == 0);
impl_spec_from_elem!(u32, |x| x == 0);
impl_spec_from_elem!(u64, |x| x == 0);
impl_spec_from_elem!(u128, |x| x == 0);
impl_spec_from_elem!(usize, |x| x == 0);

impl_spec_from_elem!(f32, |x: f32| x == 0. && x.is_sign_positive());
impl_spec_from_elem!(f64, |x: f64| x == 0. && x.is_sign_positive());

////////////////////////////////////////////////////////////////////////////////
// Common trait implementations for Vec
////////////////////////////////////////////////////////////////////////////////
Expand Down