small fixups in bytes.rs (#145)

* Inner: make uninitialized construction explicit
* Remove Inner2
* Remove unnecessary transmutes
* Use AtomicPtr::get_mut where possible
* Some minor tweaks
This commit is contained in:
Dan Burkert
2017-08-18 08:33:28 -07:00
committed by Carl Lerche
parent 34540be54c
commit 03d501b18d
+23 -87
View File
@@ -102,7 +102,7 @@ use std::iter::{FromIterator, Iterator};
/// [1] Small enough: 31 bytes on 64 bit systems, 15 on 32 bit systems. /// [1] Small enough: 31 bytes on 64 bit systems, 15 on 32 bit systems.
/// ///
pub struct Bytes { pub struct Bytes {
inner: Inner2, inner: Inner,
} }
/// A unique reference to a contiguous slice of memory. /// A unique reference to a contiguous slice of memory.
@@ -148,7 +148,7 @@ pub struct Bytes {
/// assert_eq!(&b[..], b"hello"); /// assert_eq!(&b[..], b"hello");
/// ``` /// ```
pub struct BytesMut { pub struct BytesMut {
inner: Inner2, inner: Inner,
} }
// Both `Bytes` and `BytesMut` are backed by `Inner` and functions are delegated // Both `Bytes` and `BytesMut` are backed by `Inner` and functions are delegated
@@ -310,16 +310,6 @@ struct Inner {
arc: AtomicPtr<Shared>, arc: AtomicPtr<Shared>,
} }
// This struct is only here to make older versions of Rust happy. In older
// versions of `Rust`, `repr(C)` structs could not have drop functions. While
// this is no longer the case for newer rust versions, a number of major Rust
// libraries still support older versions of Rust for which it is the case. To
// get around this, `Inner` (the actual struct) is wrapped by `Inner2` which has
// the drop fn implementation.
struct Inner2 {
inner: Inner,
}
// Thread-safe reference-counted container for the shared storage. This mostly // Thread-safe reference-counted container for the shared storage. This mostly
// the same as `std::sync::Arc` but without the weak counter. The ref counting // the same as `std::sync::Arc` but without the weak counter. The ref counting
// fns are based on the ones found in `std`. // fns are based on the ones found in `std`.
@@ -397,9 +387,7 @@ impl Bytes {
#[inline] #[inline]
pub fn with_capacity(capacity: usize) -> Bytes { pub fn with_capacity(capacity: usize) -> Bytes {
Bytes { Bytes {
inner: Inner2 { inner: Inner::with_capacity(capacity),
inner: Inner::with_capacity(capacity),
},
} }
} }
@@ -436,9 +424,7 @@ impl Bytes {
#[inline] #[inline]
pub fn from_static(bytes: &'static [u8]) -> Bytes { pub fn from_static(bytes: &'static [u8]) -> Bytes {
Bytes { Bytes {
inner: Inner2 { inner: Inner::from_static(bytes),
inner: Inner::from_static(bytes),
}
} }
} }
@@ -596,9 +582,7 @@ impl Bytes {
} }
Bytes { Bytes {
inner: Inner2 { inner: self.inner.split_off(at),
inner: self.inner.split_off(at),
}
} }
} }
@@ -637,9 +621,7 @@ impl Bytes {
} }
Bytes { Bytes {
inner: Inner2 { inner: self.inner.split_to(at),
inner: self.inner.split_to(at),
}
} }
} }
@@ -786,9 +768,7 @@ impl<'a> IntoBuf for &'a Bytes {
impl Clone for Bytes { impl Clone for Bytes {
fn clone(&self) -> Bytes { fn clone(&self) -> Bytes {
Bytes { Bytes {
inner: Inner2 { inner: self.inner.shallow_clone(),
inner: self.inner.shallow_clone(),
}
} }
} }
} }
@@ -990,9 +970,7 @@ impl BytesMut {
#[inline] #[inline]
pub fn with_capacity(capacity: usize) -> BytesMut { pub fn with_capacity(capacity: usize) -> BytesMut {
BytesMut { BytesMut {
inner: Inner2 { inner: Inner::with_capacity(capacity),
inner: Inner::with_capacity(capacity),
},
} }
} }
@@ -1122,9 +1100,7 @@ impl BytesMut {
/// Panics if `at > capacity`. /// Panics if `at > capacity`.
pub fn split_off(&mut self, at: usize) -> BytesMut { pub fn split_off(&mut self, at: usize) -> BytesMut {
BytesMut { BytesMut {
inner: Inner2 { inner: self.inner.split_off(at),
inner: self.inner.split_off(at),
}
} }
} }
@@ -1192,9 +1168,7 @@ impl BytesMut {
/// Panics if `at > len`. /// Panics if `at > len`.
pub fn split_to(&mut self, at: usize) -> BytesMut { pub fn split_to(&mut self, at: usize) -> BytesMut {
BytesMut { BytesMut {
inner: Inner2 { inner: self.inner.split_to(at),
inner: self.inner.split_to(at),
}
} }
} }
@@ -1445,9 +1419,7 @@ impl ops::DerefMut for BytesMut {
impl From<Vec<u8>> for BytesMut { impl From<Vec<u8>> for BytesMut {
fn from(src: Vec<u8>) -> BytesMut { fn from(src: Vec<u8>) -> BytesMut {
BytesMut { BytesMut {
inner: Inner2 { inner: Inner::from_vec(src),
inner: Inner::from_vec(src),
},
} }
} }
} }
@@ -1474,9 +1446,7 @@ impl<'a> From<&'a [u8]> for BytesMut {
inner.as_raw()[0..len].copy_from_slice(src); inner.as_raw()[0..len].copy_from_slice(src);
BytesMut { BytesMut {
inner: Inner2 { inner: inner,
inner: inner,
}
} }
} }
} else { } else {
@@ -1654,10 +1624,9 @@ impl Inner {
if capacity <= INLINE_CAP { if capacity <= INLINE_CAP {
unsafe { unsafe {
// Using uninitialized memory is ~30% faster // Using uninitialized memory is ~30% faster
Inner { let mut inner: Inner = mem::uninitialized();
arc: AtomicPtr::new(KIND_INLINE as *mut Shared), inner.arc = AtomicPtr::new(KIND_INLINE as *mut Shared);
.. mem::uninitialized() inner
}
} }
} else { } else {
Inner::from_vec(Vec::with_capacity(capacity)) Inner::from_vec(Vec::with_capacity(capacity))
@@ -1749,8 +1718,8 @@ impl Inner {
#[inline] #[inline]
fn set_inline_len(&mut self, len: usize) { fn set_inline_len(&mut self, len: usize) {
debug_assert!(len <= INLINE_CAP); debug_assert!(len <= INLINE_CAP);
let p: &mut usize = unsafe { mem::transmute(&mut self.arc) }; let p = self.arc.get_mut();
*p = (*p & !INLINE_LEN_MASK) | (len << INLINE_LEN_OFFSET); *p = ((*p as usize & !INLINE_LEN_MASK) | (len << INLINE_LEN_OFFSET)) as _;
} }
/// slice. /// slice.
@@ -1891,15 +1860,9 @@ impl Inner {
} else if kind == KIND_STATIC { } else if kind == KIND_STATIC {
false false
} else { } else {
// The function requires `&mut self`, which guarantees a unique
// reference to the current handle. This means that the `arc` field
// *cannot* be concurrently mutated. As such, `Relaxed` ordering is
// fine (since we aren't synchronizing with anything).
let arc = self.arc.load(Relaxed);
// Otherwise, the underlying buffer is potentially shared with other // Otherwise, the underlying buffer is potentially shared with other
// handles, so the ref_count needs to be checked. // handles, so the ref_count needs to be checked.
unsafe { (*arc).is_unique() } unsafe { (**self.arc.get_mut()).is_unique() }
} }
} }
@@ -1982,7 +1945,7 @@ impl Inner {
// The upgrade failed, a concurrent clone happened. Release // The upgrade failed, a concurrent clone happened. Release
// the allocation that was made in this thread, it will not // the allocation that was made in this thread, it will not
// be needed. // be needed.
let shared: Box<Shared> = mem::transmute(shared); let shared = Box::from_raw(shared);
mem::forget(*shared); mem::forget(*shared);
// Update the `arc` local variable and fall through to a ref // Update the `arc` local variable and fall through to a ref
@@ -2068,10 +2031,7 @@ impl Inner {
} }
} }
// `Relaxed` is Ok here (and really, no synchronization is necessary) let arc = *self.arc.get_mut();
// due to having a `&mut self` pointer. The `&mut self` pointer ensures
// that there is no concurrent access on `self`.
let arc = self.arc.load(Relaxed);
debug_assert!(kind == KIND_ARC); debug_assert!(kind == KIND_ARC);
@@ -2211,7 +2171,7 @@ impl Inner {
} }
} }
impl Drop for Inner2 { impl Drop for Inner {
fn drop(&mut self) { fn drop(&mut self) {
let kind = self.kind(); let kind = self.kind();
@@ -2221,9 +2181,7 @@ impl Drop for Inner2 {
let _ = Vec::from_raw_parts(self.ptr, self.len, self.cap); let _ = Vec::from_raw_parts(self.ptr, self.len, self.cap);
} }
} else if kind == KIND_ARC { } else if kind == KIND_ARC {
// &mut self guarantees correct ordering release_shared(*self.arc.get_mut());
let arc = self.arc.load(Relaxed);
release_shared(arc);
} }
} }
} }
@@ -2255,7 +2213,7 @@ fn release_shared(ptr: *mut Shared) {
atomic::fence(Acquire); atomic::fence(Acquire);
// Drop the data // Drop the data
let _: Box<Shared> = mem::transmute(ptr); Box::from_raw(ptr);
} }
} }
@@ -2278,28 +2236,6 @@ impl Shared {
unsafe impl Send for Inner {} unsafe impl Send for Inner {}
unsafe impl Sync for Inner {} unsafe impl Sync for Inner {}
/*
*
* ===== impl Inner2 =====
*
*/
impl ops::Deref for Inner2 {
type Target = Inner;
#[inline]
fn deref(&self) -> &Inner {
&self.inner
}
}
impl ops::DerefMut for Inner2 {
#[inline]
fn deref_mut(&mut self) -> &mut Inner {
&mut self.inner
}
}
/* /*
* *
* ===== PartialEq / PartialOrd ===== * ===== PartialEq / PartialOrd =====