Skip to content

Commit

Permalink
Clippy + parking_lot
Browse files Browse the repository at this point in the history
  • Loading branch information
ecton committed Aug 11, 2024
1 parent 07f86e6 commit 66b4d07
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 24 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 6 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@ crossbeam-utils = "0.8.19"
flume = "0.11.0"
intentional = "0.1.1"
kempt = "0.2.4"
parking_lot = { version = "0.12.1" }
parking_lot = { workspace = true }
refuse-macros = { path = "./refuse-macros", version = "=0.0.5" }

[lints]
rust.missing_docs = "warn"

workspace = true

[profile.release]
debug = true
Expand All @@ -35,3 +34,7 @@ members = ["benchmarks", "refuse-macros", "refuse-pool"]

[workspace.lints]
clippy.pedantic = "warn"
rust.missing_docs = "warn"

[workspace.dependencies]
parking_lot = { version = "0.12.1" }
2 changes: 1 addition & 1 deletion refuse-macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ quote = "1.0.35"
proc-macro2 = "1.0.79"

[lints]
clippy.pedantic = "warn"
workspace = true
6 changes: 6 additions & 0 deletions refuse-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ use proc_macro2::{Span, TokenStream};
use quote::quote;
use syn::{GenericParam, Generics, Lifetime, TraitBound};

/// Derives the `refuse::MapAs` trait for a given struct or enum.
///
/// This macro expects the identifier `refuse` to refer to the crate.
#[manyhow]
#[proc_macro_derive(MapAs, attributes(map_as))]
pub fn derive_map_as(input: syn::Item) -> manyhow::Result {
Expand Down Expand Up @@ -53,6 +56,9 @@ fn derive_map_as_inner(
}
}

/// Derives the `refuse::Trace` trait for a given struct or enum.
///
/// This macro expects the identifier `refuse` to refer to the crate.
#[manyhow]
#[proc_macro_derive(Trace, attributes(trace))]
pub fn derive_trace(input: syn::Item) -> manyhow::Result {
Expand Down
3 changes: 2 additions & 1 deletion refuse-pool/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ ahash = { version = "0.8.11", default-features = false, features = [
"runtime-rng",
] }
hashbrown = "0.14.3"
parking_lot = { workspace = true }

[lints]
rust.missing_docs = "warn"
workspace = true
21 changes: 14 additions & 7 deletions refuse-pool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,11 @@ use std::borrow::Cow;
use std::fmt::{Debug, Display};
use std::hash::{Hash, Hasher};
use std::ops::Deref;
use std::sync::{Mutex, OnceLock};
use std::sync::OnceLock;

use ahash::AHasher;
use hashbrown::{hash_table, HashTable};
use parking_lot::Mutex;
use refuse::{AnyRef, AnyRoot, CollectionGuard, LocalPool, Ref, Root, SimpleType, Trace};

enum PoolEntry {
Expand Down Expand Up @@ -92,7 +93,7 @@ impl StringPool {
let hash = hash_str(key.as_ref());
match self
.strings
.entry(hash, |a| a.equals(&key, guard), |e| e.hash())
.entry(hash, |a| a.equals(&key, guard), PoolEntry::hash)
{
hash_table::Entry::Occupied(entry) => {
let entry = entry.into_mut();
Expand Down Expand Up @@ -149,22 +150,25 @@ impl RootString {
///
/// If another [`RootString`] or [`RefString`] exists already with the same
/// contents as `s`, it will be returned and `s` will be dropped.
pub fn new<'a>(s: impl Into<Cow<'a, str>>, guard: impl AsRef<CollectionGuard<'a>>) -> Self {
let mut pool = StringPool::global().lock().expect("poisoned");
pub fn new<'a>(s: impl Into<Cow<'a, str>>, guard: &impl AsRef<CollectionGuard<'a>>) -> Self {
let mut pool = StringPool::global().lock();
pool.intern(s.into(), guard.as_ref()).clone()
}

/// Returns a reference to this root string.
#[must_use]
pub const fn downgrade(&self) -> RefString {
RefString(self.0.downgrade())
}

/// Returns a typeless reference to this string.
#[must_use]
pub const fn downgrade_any(&self) -> AnyRef {
self.0.downgrade_any()
}

/// Returns the number of root references to this string, `self` included.
#[must_use]
pub fn root_count(&self) -> u64 {
// We subtract one because the string pool always contains a reference.
// Including it in the publicly viewable count would just lead to
Expand All @@ -180,7 +184,7 @@ impl RootString {
/// Returns `Err(root)` if `root` does not contain a pooled string.
pub fn try_from_any<'a>(
root: AnyRoot,
guard: impl AsRef<CollectionGuard<'a>>,
guard: &impl AsRef<CollectionGuard<'a>>,
) -> Result<Self, AnyRoot> {
Root::try_from_any(root, guard).map(Self)
}
Expand All @@ -191,7 +195,7 @@ impl Drop for RootString {
if self.0.root_count() == 2 {
// This is the last `RootString` aside from the one stored in the
// pool, so we should remove the pool entry.
let mut pool = StringPool::global().lock().expect("poisoned");
let mut pool = StringPool::global().lock();
let entry = pool
.strings
.find_entry(self.0.hash, |s| s == &self.0)
Expand Down Expand Up @@ -345,7 +349,7 @@ impl RefString {
/// contents as `s`, it will be returned and `s` will be dropped.
pub fn new<'a>(s: impl Into<Cow<'a, str>>) -> Self {
let guard = CollectionGuard::acquire();
let mut pool = StringPool::global().lock().expect("poisoned");
let mut pool = StringPool::global().lock();
pool.intern(s.into(), &guard).downgrade()
}

Expand All @@ -358,16 +362,19 @@ impl RefString {

/// Loads a reference to the underlying string, if the string hasn't been
/// freed.
#[must_use]
pub fn load<'guard>(&self, guard: &'guard CollectionGuard) -> Option<&'guard str> {
self.0.load(guard).map(|pooled| &*pooled.string)
}

/// Loads this string as a root, if the string hasn't been freed.
#[must_use]
pub fn as_root(&self, guard: &CollectionGuard) -> Option<RootString> {
self.0.as_root(guard).map(RootString)
}

/// Returns a typeless reference to this string.
#[must_use]
pub fn as_any(&self) -> AnyRef {
self.0.as_any()
}
Expand Down
39 changes: 27 additions & 12 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ impl ThreadPool {
}

fn release_thread_guard() {
Self::map_current(Self::release_guard)
Self::map_current(Self::release_guard);
}

fn push_guard(&self) -> usize {
Expand Down Expand Up @@ -1030,6 +1030,7 @@ impl CollectionGuard<'_> {
///
/// If any other guards are currently held by this thread, this function
/// does nothing.
#[allow(clippy::redundant_closure_for_method_calls)] // produces compiler error
pub fn yield_to_collector(&mut self) {
self.coordinated_yield(|yielder| yielder.wait());
}
Expand All @@ -1056,10 +1057,16 @@ impl CollectionGuard<'_> {
///
/// If any other guards are currently held by this thread, this function
/// does nothing.
pub fn coordinated_yield(&mut self, yielder: impl FnOnce(Yielder<'_>) -> YieldComplete) {
pub fn coordinated_yield(
&mut self,
yielder: impl FnOnce(Yielder<'_>) -> YieldComplete,
) -> bool {
// We only need to attempt yielding if we are the outermost guard.
if ThreadPool::current_depth() == 1 && self.collector.release_reader_if_collecting() {
let YieldComplete = yielder(Yielder(&mut self.collector));
let _complete: YieldComplete = yielder(Yielder(&mut self.collector));
true
} else {
false
}
}

Expand Down Expand Up @@ -1122,15 +1129,18 @@ pub struct Yielder<'a>(&'a mut CollectorReadGuard);

impl Yielder<'_> {
/// Waits for the garbage collector to finish the current collection.
#[must_use]
pub fn wait(self) -> YieldComplete {
self.0.acquire_reader();
YieldComplete
YieldComplete { _priv: PhantomData }
}
}

/// A marker indicating that a [coordinated
/// yield](CollectionGuard::coordinated_yield) has completed.
pub struct YieldComplete;
pub struct YieldComplete {
_priv: PhantomData<()>,
}

/// A type that can be garbage collected.
///
Expand Down Expand Up @@ -1571,7 +1581,7 @@ where

/// Stores `value` in the garbage collector, returning a root reference to
/// the data.
pub fn new<'a>(value: T, guard: impl AsRef<CollectionGuard<'a>>) -> Self {
pub fn new<'a>(value: T, guard: &impl AsRef<CollectionGuard<'a>>) -> Self {
let guard = guard.as_ref();
let (type_index, gen, bin) = guard.adopt(Rooted::root(value));
Self::from_parts(type_index, gen, bin, guard)
Expand All @@ -1584,10 +1594,10 @@ where
/// Returns `Err(root)` if `root` does not contain a `T`.
pub fn try_from_any<'a>(
root: AnyRoot,
guard: impl AsRef<CollectionGuard<'a>>,
guard: &impl AsRef<CollectionGuard<'a>>,
) -> Result<Self, AnyRoot> {
if TypeIndex::of::<T>() == root.any.type_index {
let slot = root.any.load_slot(guard.as_ref()).expect("root missing");
let slot = root.any.load_slot(guard.as_ref()).assert("root missing");
Ok(Self {
data: slot,
reference: root.any.downcast_ref(),
Expand Down Expand Up @@ -1630,6 +1640,7 @@ where
}

/// Returns this root as an untyped root.
#[must_use]
pub fn into_any_root(self) -> AnyRoot {
// We transfer ownership of this reference to the AnyRoot, so we want to
// avoid calling drop on `self`.
Expand Down Expand Up @@ -1772,7 +1783,7 @@ where
T: Collectable + Hash,
{
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
(**self).hash(state)
(**self).hash(state);
}
}

Expand Down Expand Up @@ -1856,7 +1867,7 @@ where
{
/// Stores `value` in the garbage collector, returning a "weak" reference to
/// it.
pub fn new<'a>(value: T, guard: impl AsRef<CollectionGuard<'a>>) -> Self {
pub fn new<'a>(value: T, guard: &impl AsRef<CollectionGuard<'a>>) -> Self {
let guard = guard.as_ref();
let (type_index, slot_generation, bin_id) = guard.adopt(Rooted::reference(value));

Expand Down Expand Up @@ -2258,7 +2269,7 @@ where
let slot = unsafe { &(*slot.value.get()).allocated };
slot.roots.fetch_add(1, Ordering::Relaxed);
Some(AnyRoot {
rooted: (&**slot) as *const Rooted<T> as *const (),
rooted: std::ptr::addr_of!(**slot).cast::<()>(),
roots: &slot.roots,
any: AnyRef {
bin_id,
Expand Down Expand Up @@ -2681,7 +2692,7 @@ enum SlotSweepStatus {
/// yielding by the current thread when invoked. If a guard is held, consider
/// calling [`CollectionGuard::collect()`] instead.
pub fn collect() {
try_collect().unwrap()
try_collect().unwrap();
}

/// Invokes the garbage collector.
Expand Down Expand Up @@ -2766,6 +2777,7 @@ pub struct AnyRoot {
impl AnyRoot {
/// Loads a reference to the underlying data. Returns `None` if `T` is not
/// the type of the underlying data.
#[must_use]
pub fn load<T>(&self) -> Option<&T>
where
T: Collectable,
Expand All @@ -2783,6 +2795,7 @@ impl AnyRoot {
}

/// Returns a [`Root<T>`] if the underlying reference points to a `T`.
#[must_use]
pub fn downcast_root<T>(&self) -> Option<Root<T>>
where
T: Collectable,
Expand Down Expand Up @@ -2810,6 +2823,7 @@ impl AnyRoot {
///
/// This function does not do any type checking. If `T` is not the correct
/// type, attempting to load the underyling value will fail.
#[must_use]
pub const fn downcast_ref<T>(&self) -> Ref<T>
where
T: Collectable,
Expand All @@ -2827,6 +2841,7 @@ impl AnyRoot {
}

/// Returns an untyped "weak" reference to this root.
#[must_use]
pub const fn as_any(&self) -> AnyRef {
self.any
}
Expand Down

0 comments on commit 66b4d07

Please sign in to comment.