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

feat: add Deref and DerefMut impls to Ref types #13

Merged
merged 1 commit into from
Dec 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ tokio = { version = "1.14.0", features = ["rt", "rt-multi-thread", "macros", "sy
futures-util = { version = "0.3", default-features = false }

[target.'cfg(loom)'.dev-dependencies]
loom = { version = "0.5.3", features = ["checkpoint", "futures"] }
loom = { version = "0.5.4", features = ["checkpoint", "futures"] }
tracing-subscriber = { version = "0.3", default-features = false, features = ["std", "fmt"] }
tracing = { version = "0.1", default-features = false, features = ["std"] }

Expand Down
6 changes: 1 addition & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,4 @@
**A:** Originally, I imagined it as a kind of ring buffer, so (as a pun on
"ringbuf"), I called it "stringbuf". Then, I realized you could do this with
more than just strings. In fact, it can be generalized to arbitrary...things.
So, "thingbuf".

- **Q: Why don't the `Ref` types implement `Deref` and `DerefMut`?**

**A:** [Blame `loom` for this.](https://github.com/tokio-rs/loom/pull/219)
So, "thingbuf".
58 changes: 39 additions & 19 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![cfg_attr(not(feature = "std"), no_std)]
#![cfg_attr(docsrs, feature(doc_cfg))]
use core::{cmp, fmt, mem::MaybeUninit, ops::Index};
use core::{cmp, fmt, mem::MaybeUninit, ops};

#[macro_use]
mod macros;
Expand All @@ -27,12 +27,13 @@ pub use self::static_thingbuf::StaticThingBuf;
use crate::{
loom::{
atomic::{AtomicUsize, Ordering::*},
UnsafeCell,
cell::{MutPtr, UnsafeCell},
},
util::{Backoff, CachePadded},
};

pub struct Ref<'slot, T> {
ptr: MutPtr<MaybeUninit<T>>,
slot: &'slot Slot<T>,
new_state: usize,
}
Expand Down Expand Up @@ -127,7 +128,7 @@ impl Core {
) -> Result<Ref<'slots, T>, mpsc::TrySendError<()>>
where
T: Default,
S: Index<usize, Output = Slot<T>> + ?Sized,
S: ops::Index<usize, Output = Slot<T>> + ?Sized,
{
test_println!("push_ref");
let mut backoff = Backoff::new();
Expand Down Expand Up @@ -158,16 +159,20 @@ impl Core {
Ok(_) => {
// We got the slot! It's now okay to write to it
test_println!("claimed tail slot [{}]", idx);
// Claim exclusive ownership over the slot
let ptr = slot.value.get_mut();

if gen == 0 {
slot.value.with_mut(|value| unsafe {
unsafe {
// Safety: we have just claimed exclusive ownership over
// this slot.
(*value).write(T::default());
});
ptr.deref().write(T::default());
};
test_println!("-> initialized");
}

return Ok(Ref {
ptr,
new_state: tail + 1,
slot,
});
Expand Down Expand Up @@ -207,7 +212,7 @@ impl Core {

fn pop_ref<'slots, T, S>(&self, slots: &'slots S) -> Result<Ref<'slots, T>, mpsc::TrySendError>
where
S: Index<usize, Output = Slot<T>> + ?Sized,
S: ops::Index<usize, Output = Slot<T>> + ?Sized,
{
test_println!("pop_ref");
let mut backoff = Backoff::new();
Expand All @@ -234,6 +239,7 @@ impl Core {
test_println!("claimed head slot [{}]", idx);
return Ok(Ref {
new_state: head.wrapping_add(self.gen),
ptr: slot.value.get_mut(),
slot,
});
}
Expand Down Expand Up @@ -299,11 +305,9 @@ impl Core {
// === impl Ref ===

impl<T> Ref<'_, T> {
const RELEASED: usize = usize::MAX;

#[inline]
pub fn with<U>(&self, f: impl FnOnce(&T) -> U) -> U {
self.slot.value.with(|value| unsafe {
self.ptr.with(|value| unsafe {
// Safety: if a `Ref` exists, we have exclusive ownership of the
// slot. A `Ref` is only created if the slot has already been
// initialized.
Expand All @@ -315,31 +319,47 @@ impl<T> Ref<'_, T> {

#[inline]
pub fn with_mut<U>(&mut self, f: impl FnOnce(&mut T) -> U) -> U {
self.slot.value.with_mut(|value| unsafe {
self.ptr.with(|value| unsafe {
// Safety: if a `Ref` exists, we have exclusive ownership of the
// slot.
// TODO(eliza): use `MaybeUninit::assume_init_mut` here once it's
// supported by `tracing-appender`'s MSRV.
f(&mut *(&mut *value).as_mut_ptr())
})
}
}

pub(crate) fn release(&mut self) {
if self.new_state == Self::RELEASED {
test_println!("release_ref; already released");
return;
impl<T> ops::Deref for Ref<'_, T> {
type Target = T;

#[inline]
fn deref(&self) -> &Self::Target {
unsafe {
// Safety: if a `Ref` exists, we have exclusive ownership of the
// slot. A `Ref` is only created if the slot has already been
// initialized.
&*self.ptr.deref().as_ptr()
}
}
}

test_println!("release_ref");
test_dbg!(self.slot.state.store(test_dbg!(self.new_state), Release));
self.new_state = Self::RELEASED;
impl<T> ops::DerefMut for Ref<'_, T> {
#[inline]
fn deref_mut(&mut self) -> &mut Self::Target {
unsafe {
// Safety: if a `Ref` exists, we have exclusive ownership of the
// slot. A `Ref` is only created if the slot has already been
// initialized.
&mut *self.ptr.deref().as_mut_ptr()
}
}
}

impl<T> Drop for Ref<'_, T> {
#[inline]
fn drop(&mut self) {
self.release();
test_println!("drop Ref<{}>", core::any::type_name::<T>());
test_dbg!(self.slot.state.store(test_dbg!(self.new_state), Release));
}
}

Expand Down
68 changes: 48 additions & 20 deletions src/loom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ mod inner {
pub use std::sync::atomic::Ordering;
}

pub(crate) use loom::{cell::UnsafeCell, future, hint, sync, thread};
pub(crate) use loom::{cell, future, hint, sync, thread};
use std::{cell::RefCell, fmt::Write};

pub(crate) mod model {
Expand Down Expand Up @@ -213,31 +213,59 @@ mod inner {
}
}

#[derive(Debug)]
pub(crate) struct UnsafeCell<T>(core::cell::UnsafeCell<T>);
pub(crate) mod cell {
#[derive(Debug)]
pub(crate) struct UnsafeCell<T>(core::cell::UnsafeCell<T>);

impl<T> UnsafeCell<T> {
pub const fn new(data: T) -> UnsafeCell<T> {
UnsafeCell(core::cell::UnsafeCell::new(data))
}
impl<T> UnsafeCell<T> {
pub const fn new(data: T) -> UnsafeCell<T> {
UnsafeCell(core::cell::UnsafeCell::new(data))
}

#[inline(always)]
pub fn with<F, R>(&self, f: F) -> R
where
F: FnOnce(*const T) -> R,
{
f(self.0.get())
#[inline(always)]
pub fn with<F, R>(&self, f: F) -> R
where
F: FnOnce(*const T) -> R,
{
f(self.0.get())
}

#[inline(always)]
pub fn with_mut<F, R>(&self, f: F) -> R
where
F: FnOnce(*mut T) -> R,
{
f(self.0.get())
}

#[inline(always)]
pub(crate) fn get_mut(&self) -> MutPtr<T> {
MutPtr(self.0.get())
}
}

#[inline(always)]
pub fn with_mut<F, R>(&self, f: F) -> R
where
F: FnOnce(*mut T) -> R,
{
f(self.0.get())
#[derive(Debug)]
pub(crate) struct MutPtr<T: ?Sized>(*mut T);

impl<T: ?Sized> MutPtr<T> {
// Clippy knows that it's Bad and Wrong to construct a mutable reference
// from an immutable one...but this function is intended to simulate a raw
// pointer, so we have to do that here.
#[allow(clippy::mut_from_ref)]
#[inline(always)]
pub(crate) unsafe fn deref(&self) -> &mut T {
&mut *self.0
}

#[inline(always)]
pub fn with<F, R>(&self, f: F) -> R
where
F: FnOnce(*mut T) -> R,
{
f(self.0)
}
}
}

pub(crate) mod alloc {
/// Track allocations, detecting leaks
#[derive(Debug, Default)]
Expand Down
Loading