Skip to content

Commit

Permalink
refactor: simplify pointer trait and remove DefaultPointerOps (#100)
Browse files Browse the repository at this point in the history 
* refactor: simplify some generic type definition

* refactor: simplify pointer trait and remove DefaultPointerOps

* run clippy

* fix test compile

* rename PointerOps to Pointer

* use adapter in eviction policy associated type
  • Loading branch information
@wenym1
wenym1 committed Aug 3, 2023
1 parent 0e5a595 commit 049f538
Show file tree
Hide file tree
Showing 10 changed files with 292 additions and 417 deletions.
56 changes: 26 additions & 30 deletions foyer-intrusive/src/collections/dlist.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::ptr::NonNull;

use crate::core::{
adapter::{Adapter, Link},
pointer::PointerOps,
pointer::Pointer,
};

#[derive(Debug, Default)]
Expand Down Expand Up @@ -90,23 +90,23 @@ where
}
}

pub fn front(&self) -> Option<&<A::PointerOps as PointerOps>::Item> {
pub fn front(&self) -> Option<&<A::Pointer as Pointer>::Item> {
unsafe {
self.head
.map(|link| self.adapter.link2item(link.as_ptr()))
.map(|link| &*link)
}
}

pub fn back(&self) -> Option<&<A::PointerOps as PointerOps>::Item> {
pub fn back(&self) -> Option<&<A::Pointer as Pointer>::Item> {
unsafe {
self.tail
.map(|link| self.adapter.link2item(link.as_ptr()))
.map(|link| &*link)
}
}

pub fn front_mut(&mut self) -> Option<&mut <A::PointerOps as PointerOps>::Item> {
pub fn front_mut(&mut self) -> Option<&mut <A::Pointer as Pointer>::Item> {
unsafe {
self.head
.map(|link| self.adapter.link2item(link.as_ptr()))
Expand All @@ -115,7 +115,7 @@ where
}
}

pub fn back_mut(&mut self) -> Option<&mut <A::PointerOps as PointerOps>::Item> {
pub fn back_mut(&mut self) -> Option<&mut <A::Pointer as Pointer>::Item> {
unsafe {
self.tail
.map(|link| self.adapter.link2item(link.as_ptr()))
Expand All @@ -124,21 +124,21 @@ where
}
}

pub fn push_front(&mut self, ptr: <A::PointerOps as PointerOps>::Pointer) {
pub fn push_front(&mut self, ptr: A::Pointer) {
self.iter_mut().insert_after(ptr);
}

pub fn push_back(&mut self, ptr: <A::PointerOps as PointerOps>::Pointer) {
pub fn push_back(&mut self, ptr: A::Pointer) {
self.iter_mut().insert_before(ptr);
}

pub fn pop_front(&mut self) -> Option<<A::PointerOps as PointerOps>::Pointer> {
pub fn pop_front(&mut self) -> Option<A::Pointer> {
let mut iter = self.iter_mut();
iter.next();
iter.remove()
}

pub fn pop_back(&mut self) -> Option<<A::PointerOps as PointerOps>::Pointer> {
pub fn pop_back(&mut self) -> Option<A::Pointer> {
let mut iter = self.iter_mut();
iter.prev();
iter.remove()
Expand Down Expand Up @@ -224,7 +224,7 @@ where
self.link.is_some()
}

pub fn get(&self) -> Option<&<A::PointerOps as PointerOps>::Item> {
pub fn get(&self) -> Option<&<A::Pointer as Pointer>::Item> {
self.link
.map(|link| unsafe { &*self.dlist.adapter.link2item(link.as_ptr()) })
}
Expand Down Expand Up @@ -290,12 +290,12 @@ where
self.link.is_some()
}

pub fn get(&self) -> Option<&<A::PointerOps as PointerOps>::Item> {
pub fn get(&self) -> Option<&<A::Pointer as Pointer>::Item> {
self.link
.map(|link| unsafe { &*self.dlist.adapter.link2item(link.as_ptr()) })
}

pub fn get_mut(&mut self) -> Option<&mut <A::PointerOps as PointerOps>::Item> {
pub fn get_mut(&mut self) -> Option<&mut <A::Pointer as Pointer>::Item> {
self.link
.map(|link| unsafe { &mut *(self.dlist.adapter.link2item(link.as_ptr()) as *mut _) })
}
Expand Down Expand Up @@ -337,7 +337,7 @@ where
}

/// Removes the current item from [`DList`] and move next.
pub fn remove(&mut self) -> Option<<A::PointerOps as PointerOps>::Pointer> {
pub fn remove(&mut self) -> Option<A::Pointer> {
unsafe {
if !self.is_valid() {
return None;
Expand All @@ -346,7 +346,7 @@ where
let mut link = self.link.unwrap();

let item = self.dlist.adapter.link2item(link.as_ptr());
let ptr = self.dlist.adapter.pointer_ops().from_raw(item);
let ptr = A::Pointer::from_raw(item);

// fix head and tail if node is either of that
let mut prev = link.as_ref().prev;
Expand Down Expand Up @@ -381,9 +381,9 @@ where
/// Link a new ptr before the current one.
///
/// If iter is on null, link to tail.
pub fn insert_before(&mut self, ptr: <A::PointerOps as PointerOps>::Pointer) {
pub fn insert_before(&mut self, ptr: A::Pointer) {
unsafe {
let item_new = self.dlist.adapter.pointer_ops().into_raw(ptr);
let item_new = A::Pointer::into_raw(ptr);
let mut link_new =
NonNull::new_unchecked(self.dlist.adapter.item2link(item_new) as *mut A::Link);
assert!(!link_new.as_ref().is_linked());
Expand All @@ -409,9 +409,9 @@ where
/// Link a new ptr after the current one.
///
/// If iter is on null, link to head.
pub fn insert_after(&mut self, ptr: <A::PointerOps as PointerOps>::Pointer) {
pub fn insert_after(&mut self, ptr: A::Pointer) {
unsafe {
let item_new = self.dlist.adapter.pointer_ops().into_raw(ptr);
let item_new = A::Pointer::into_raw(ptr);
let mut link_new =
NonNull::new_unchecked(self.dlist.adapter.item2link(item_new) as *mut A::Link);
assert!(!link_new.as_ref().is_linked());
Expand Down Expand Up @@ -471,7 +471,7 @@ impl<'a, A> Iterator for DListIter<'a, A>
where
A: Adapter<Link = DListLink>,
{
type Item = &'a <A::PointerOps as PointerOps>::Item;
type Item = &'a <A::Pointer as Pointer>::Item;

fn next(&mut self) -> Option<Self::Item> {
self.next();
Expand All @@ -486,7 +486,7 @@ impl<'a, A> Iterator for DListIterMut<'a, A>
where
A: Adapter<Link = DListLink>,
{
type Item = &'a mut <A::PointerOps as PointerOps>::Item;
type Item = &'a mut <A::Pointer as Pointer>::Item;

fn next(&mut self) -> Option<Self::Item> {
self.next();
Expand All @@ -507,7 +507,7 @@ mod tests {

use itertools::Itertools;

use crate::{core::pointer::DefaultPointerOps, intrusive_adapter};
use crate::intrusive_adapter;

use super::*;

Expand All @@ -527,31 +527,27 @@ mod tests {
}

#[derive(Debug, Default)]
struct DListAdapter(DefaultPointerOps<Box<DListItem>>);
struct DListAdapter;

unsafe impl Adapter for DListAdapter {
type PointerOps = DefaultPointerOps<Box<DListItem>>;
type Pointer = Box<DListItem>;

type Link = DListLink;

fn new() -> Self {
Self::default()
}

fn pointer_ops(&self) -> &Self::PointerOps {
&self.0
Self
}

unsafe fn link2item(
&self,
link: *const Self::Link,
) -> *const <Self::PointerOps as PointerOps>::Item {
) -> *const <Self::Pointer as Pointer>::Item {
crate::container_of!(link, DListItem, link)
}

unsafe fn item2link(
&self,
item: *const <Self::PointerOps as PointerOps>::Item,
item: *const <Self::Pointer as Pointer>::Item,
) -> *const Self::Link {
(item as *const u8).add(crate::offset_of!(DListItem, link)) as *const _
}
Expand Down
18 changes: 9 additions & 9 deletions foyer-intrusive/src/collections/duplicated_hashmap.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use twox_hash::XxHash64;
use crate::{
core::{
adapter::{Adapter, KeyAdapter, Link},
pointer::PointerOps,
pointer::Pointer,
},
intrusive_adapter,
};
Expand Down Expand Up @@ -104,7 +104,7 @@ where
while iter_group.is_valid() {
let link_group = iter_group.remove().unwrap();
let item = self.adapter.link2item(link_group.as_ptr());
let _ = self.adapter.pointer_ops().from_raw(item);
let _ = A::Pointer::from_raw(item);
}
}
}
Expand All @@ -131,9 +131,9 @@ where
}
}

pub fn insert(&mut self, ptr: <A::PointerOps as PointerOps>::Pointer) {
pub fn insert(&mut self, ptr: A::Pointer) {
unsafe {
let item_new = self.adapter.pointer_ops().into_raw(ptr);
let item_new = A::Pointer::into_raw(ptr);
let mut link_new =
NonNull::new_unchecked(self.adapter.item2link(item_new) as *mut A::Link);

Expand All @@ -158,7 +158,7 @@ where
}
}

pub fn remove(&mut self, key: &K) -> Vec<<A::PointerOps as PointerOps>::Pointer> {
pub fn remove(&mut self, key: &K) -> Vec<A::Pointer> {
unsafe {
let hash = self.hash_key(key);
let slot = (self.slots.len() - 1) & hash as usize;
Expand All @@ -173,7 +173,7 @@ where
let link = iter.remove().unwrap();
debug_assert!(!link.as_ref().is_linked());
let item = self.adapter.link2item(link.as_ptr());
let ptr = self.adapter.pointer_ops().from_raw(item);
let ptr = A::Pointer::from_raw(item);
res.push(ptr);
}
debug_assert!(link.as_ref().group.is_empty());
Expand All @@ -185,7 +185,7 @@ where
}
}

pub fn lookup(&self, key: &K) -> Vec<&<A::PointerOps as PointerOps>::Item> {
pub fn lookup(&self, key: &K) -> Vec<&<A::Pointer as Pointer>::Item> {
unsafe {
let hash = self.hash_key(key);
let slot = (self.slots.len() - 1) & hash as usize;
Expand Down Expand Up @@ -223,7 +223,7 @@ where
pub unsafe fn remove_in_place(
&mut self,
mut link: NonNull<DuplicatedHashMapLink>,
) -> <A::PointerOps as PointerOps>::Pointer {
) -> A::Pointer {
assert!(link.as_ref().is_linked());
let item = self.adapter.link2item(link.as_ptr());
let key = &*self.adapter.item2key(item);
Expand Down Expand Up @@ -276,7 +276,7 @@ where
debug_assert!(!link.as_ref().group_link.is_linked());
debug_assert!(link.as_ref().group.is_empty());

self.adapter.pointer_ops().from_raw(item)
A::Pointer::from_raw(item)
}

/// # Safety
Expand Down
34 changes: 12 additions & 22 deletions foyer-intrusive/src/collections/hashmap.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use twox_hash::XxHash64;
use crate::{
core::{
adapter::{KeyAdapter, Link},
pointer::PointerOps,
pointer::Pointer,
},
intrusive_adapter,
};
Expand Down Expand Up @@ -79,7 +79,7 @@ where
while iter.is_valid() {
let link = iter.remove().unwrap();
let item = self.adapter.link2item(link.as_ptr());
let _ = self.adapter.pointer_ops().from_raw(item);
let _ = A::Pointer::from_raw(item);
}
}
}
Expand All @@ -105,12 +105,9 @@ where
}
}

pub fn insert(
&mut self,
ptr: <A::PointerOps as PointerOps>::Pointer,
) -> Option<<A::PointerOps as PointerOps>::Pointer> {
pub fn insert(&mut self, ptr: A::Pointer) -> Option<A::Pointer> {
unsafe {
let item_new = self.adapter.pointer_ops().into_raw(ptr);
let item_new = A::Pointer::into_raw(ptr);
let link_new = NonNull::new_unchecked(self.adapter.item2link(item_new) as *mut A::Link);

let key_new = &*self.adapter.item2key(item_new);
Expand All @@ -130,7 +127,7 @@ where
}
}

pub fn remove(&mut self, key: &K) -> Option<<A::PointerOps as PointerOps>::Pointer> {
pub fn remove(&mut self, key: &K) -> Option<A::Pointer> {
unsafe {
let hash = self.hash_key(key);
let slot = (self.slots.len() - 1) & hash as usize;
Expand All @@ -143,7 +140,7 @@ where
}
}

pub fn lookup(&self, key: &K) -> Option<&<A::PointerOps as PointerOps>::Item> {
pub fn lookup(&self, key: &K) -> Option<&<A::Pointer as Pointer>::Item> {
unsafe {
let hash = self.hash_key(key);
let slot = (self.slots.len() - 1) & hash as usize;
Expand Down Expand Up @@ -174,10 +171,7 @@ where
/// # Safety
///
/// `link` MUST be in this [`HashMap`].
pub unsafe fn remove_in_place(
&mut self,
link: NonNull<HashMapLink>,
) -> <A::PointerOps as PointerOps>::Pointer {
pub unsafe fn remove_in_place(&mut self, link: NonNull<HashMapLink>) -> A::Pointer {
assert!(link.as_ref().is_linked());
let item = self.adapter.link2item(link.as_ptr());
let key = &*self.adapter.item2key(item);
Expand All @@ -187,7 +181,7 @@ where
.iter_mut_from_raw(link.as_ref().dlist_link.raw())
.remove();
self.len -= 1;
self.adapter.pointer_ops().from_raw(item)
A::Pointer::from_raw(item)
}

/// # Safety
Expand Down Expand Up @@ -235,16 +229,12 @@ where
/// # Safety
///
/// there must be at most one matches in the slot
unsafe fn remove_inner(
&mut self,
key: &K,
slot: usize,
) -> Option<<A::PointerOps as PointerOps>::Pointer> {
unsafe fn remove_inner(&mut self, key: &K, slot: usize) -> Option<A::Pointer> {
match self.lookup_inner_mut(key, slot) {
Some(mut iter) => {
let link = iter.remove().unwrap();
let item = self.adapter.link2item(link.as_ptr());
let ptr = self.adapter.pointer_ops().from_raw(item);
let ptr = A::Pointer::from_raw(item);
Some(ptr)
}
None => None,
Expand Down Expand Up @@ -296,13 +286,13 @@ where
self.iters[self.slot].is_valid()
}

pub fn get(&self) -> Option<&<A::PointerOps as PointerOps>::Item> {
pub fn get(&self) -> Option<&<A::Pointer as Pointer>::Item> {
self.iters[self.slot]
.get()
.map(|link| unsafe { &*(self.adapter.link2item(link.raw().as_ptr()) as *const _) })
}

pub fn get_mut(&mut self) -> Option<&mut <A::PointerOps as PointerOps>::Item> {
pub fn get_mut(&mut self) -> Option<&mut <A::Pointer as Pointer>::Item> {
self.iters[self.slot]
.get()
.map(|link| unsafe { &mut *(self.adapter.link2item(link.raw().as_ptr()) as *mut _) })
Expand Down
Loading

0 comments on commit 049f538

Please sign in to comment.