Skip to content

Commit

Permalink
always false from owned is_unique
Browse files Browse the repository at this point in the history
  • Loading branch information
amunra committed Oct 28, 2024
1 parent 0173061 commit 12de708
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 30 deletions.
30 changes: 9 additions & 21 deletions src/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ pub(crate) struct Vtable {
pub to_mut: unsafe fn(&AtomicPtr<()>, *const u8, usize) -> BytesMut,
/// fn(data)
pub is_unique: unsafe fn(&AtomicPtr<()>) -> bool,
pub cheap_into_mut: unsafe fn(&AtomicPtr<()>) -> bool,
/// fn(data, ptr, len)
pub drop: unsafe fn(&mut AtomicPtr<()>, *const u8, usize),
}
Expand Down Expand Up @@ -320,14 +319,16 @@ impl Bytes {
self.len == 0
}

/// Returns true if this is the only reference to the data.
/// Returns true if this is the only reference to the data and
/// `Into<BytesMut>` would avoid cloning the underlying buffer.
///
/// Always returns false if the data is backed by a static slice.
/// Always returns false if the data is backed by a [static slice](Bytes::from_static),
/// or an [owner](Bytes::from_owner).
///
/// The result of this method may be invalidated immediately if another
/// thread clones this value while this is being called. Ensure you have
/// unique access to this value (`&mut Bytes`) first if you need to be
/// certain the result is valid (i.e. for safety reasons)
/// certain the result is valid (i.e. for safety reasons).
/// # Examples
///
/// ```
Expand Down Expand Up @@ -626,8 +627,8 @@ impl Bytes {
/// If `self` is not unique for the entire original buffer, this will fail
/// and return self.
///
/// This will also always fail if the buffer was constructed via
/// [from_owner](Bytes::from_owner).
/// This will also always fail if the buffer was constructed via either
/// [from_owner](Bytes::from_owner) or [from_static](Bytes::from_static).
///
/// # Examples
///
Expand All @@ -638,7 +639,7 @@ impl Bytes {
/// assert_eq!(bytes.try_into_mut(), Ok(BytesMut::from(&b"hello"[..])));
/// ```
pub fn try_into_mut(self) -> Result<BytesMut, Bytes> {
if unsafe { (self.vtable.cheap_into_mut)(&self.data) } {
if self.is_unique() {
Ok(self.into())
} else {
Err(self)
Expand Down Expand Up @@ -1079,7 +1080,6 @@ const STATIC_VTABLE: Vtable = Vtable {
to_vec: static_to_vec,
to_mut: static_to_mut,
is_unique: static_is_unique,
cheap_into_mut: static_is_unique,
drop: static_drop,
};

Expand Down Expand Up @@ -1152,15 +1152,7 @@ unsafe fn owned_to_mut(data: &AtomicPtr<()>, ptr: *const u8, len: usize) -> Byte
bytes_mut
}

unsafe fn owned_is_unique(data: &AtomicPtr<()>) -> bool {
let owned = data.load(Ordering::Relaxed);
let ref_cnt = &(*owned.cast::<OwnedLifetime>()).ref_cnt;
ref_cnt.load(Ordering::Acquire) == 1
}

unsafe fn owned_cheap_into_mut(_data: &AtomicPtr<()>) -> bool {
// Since the memory's ownership is tied to an external owner
// it is never zero-copy to create a BytesMut.
unsafe fn owned_is_unique(_data: &AtomicPtr<()>) -> bool {
false
}

Expand Down Expand Up @@ -1188,7 +1180,6 @@ static OWNED_VTABLE: Vtable = Vtable {
to_vec: owned_to_vec,
to_mut: owned_to_mut,
is_unique: owned_is_unique,
cheap_into_mut: owned_cheap_into_mut,
drop: owned_drop,
};

Expand All @@ -1199,7 +1190,6 @@ static PROMOTABLE_EVEN_VTABLE: Vtable = Vtable {
to_vec: promotable_even_to_vec,
to_mut: promotable_even_to_mut,
is_unique: promotable_is_unique,
cheap_into_mut: promotable_is_unique,
drop: promotable_even_drop,
};

Expand All @@ -1208,7 +1198,6 @@ static PROMOTABLE_ODD_VTABLE: Vtable = Vtable {
to_vec: promotable_odd_to_vec,
to_mut: promotable_odd_to_mut,
is_unique: promotable_is_unique,
cheap_into_mut: promotable_is_unique,
drop: promotable_odd_drop,
};

Expand Down Expand Up @@ -1385,7 +1374,6 @@ static SHARED_VTABLE: Vtable = Vtable {
to_vec: shared_to_vec,
to_mut: shared_to_mut,
is_unique: shared_is_unique,
cheap_into_mut: shared_is_unique,
drop: shared_drop,
};

Expand Down
1 change: 0 additions & 1 deletion src/bytes_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1776,7 +1776,6 @@ static SHARED_VTABLE: Vtable = Vtable {
to_vec: shared_v_to_vec,
to_mut: shared_v_to_mut,
is_unique: shared_v_is_unique,
cheap_into_mut: shared_v_is_unique,
drop: shared_v_drop,
};

Expand Down
12 changes: 4 additions & 8 deletions tests/test_bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1533,14 +1533,14 @@ impl<const L: usize> Drop for OwnedTester<L> {
}

#[test]
fn owned_is_unique() {
fn owned_is_unique_always_false() {
let b1 = Bytes::from_owner([1, 2, 3, 4, 5, 6, 7]);
assert!(b1.is_unique());
assert!(!b1.is_unique()); // even if ref_cnt == 1
let b2 = b1.clone();
assert!(!b1.is_unique());
assert!(!b2.is_unique());
drop(b1);
assert!(b2.is_unique());
assert!(!b2.is_unique()); // even if ref_cnt == 1
}

#[test]
Expand Down Expand Up @@ -1578,7 +1578,6 @@ fn owned_dropped_exactly_once() {
let b3 = b2.slice(1..b2.len() - 1);
drop(b2);
assert_eq!(drop_counter.get(), 0);
assert!(b3.is_unique());
drop(b3);
assert_eq!(drop_counter.get(), 1);
}
Expand All @@ -1591,8 +1590,7 @@ fn owned_to_mut() {
let b1 = Bytes::from_owner(owner);

// Holding an owner will fail converting to a BytesMut,
// even when the bytes instance is unique.
assert!(b1.is_unique());
// even when the bytes instance has a ref_cnt == 1.
let b1 = b1.try_into_mut().unwrap_err();

// That said, it's still possible, just not cheap.
Expand All @@ -1610,10 +1608,8 @@ fn owned_to_vec() {
let drop_counter = SharedAtomicCounter::new();
let owner = OwnedTester::new(buf, drop_counter.clone());
let b1 = Bytes::from_owner(owner);
assert!(b1.is_unique());

let v1 = b1.to_vec();
assert!(b1.is_unique());
assert_eq!(&v1[..], &buf[..]);
assert_eq!(&v1[..], &b1[..]);

Expand Down

0 comments on commit 12de708

Please sign in to comment.