Skip to content

Commit

Permalink
fix(hashtable): fix clippy warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
usamoi committed Oct 27, 2022
1 parent e3056c9 commit 9724292
Show file tree
Hide file tree
Showing 16 changed files with 135 additions and 63 deletions.
27 changes: 15 additions & 12 deletions src/common/base/src/mem_allocator/global_allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,22 +104,25 @@ unsafe impl GlobalAlloc for GlobalAllocator {

#[inline]
unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
use std::cmp::Ordering::*;
let ptr = NonNull::new(ptr).unwrap_unchecked();
let new_layout = Layout::from_size_align(new_size, layout.align()).unwrap();
if layout.size() < new_size {
if let Ok(ptr) = GlobalAllocator.grow(ptr, layout, new_layout) {
ptr.as_ptr() as *mut u8
} else {
null_mut()
match layout.size().cmp(&new_size) {
Less => {
if let Ok(ptr) = GlobalAllocator.grow(ptr, layout, new_layout) {
ptr.as_ptr() as *mut u8
} else {
null_mut()
}
}
} else if layout.size() > new_size {
if let Ok(ptr) = GlobalAllocator.shrink(ptr, layout, new_layout) {
ptr.as_ptr() as *mut u8
} else {
null_mut()
Greater => {
if let Ok(ptr) = GlobalAllocator.shrink(ptr, layout, new_layout) {
ptr.as_ptr() as *mut u8
} else {
null_mut()
}
}
} else {
ptr.as_ptr() as *mut u8
Equal => ptr.as_ptr() as *mut u8,
}
}
}
2 changes: 1 addition & 1 deletion src/common/base/src/mem_allocator/mmap_allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ pub mod linux {
while length < uname.release.len() && uname.release[length] != 0 {
length += 1;
}
let slice = unsafe { &*(&uname.release[..length] as *const [i8] as *const [u8]) };
let slice = unsafe { &*(&uname.release[..length] as *const _ as *const [u8]) };
let ver = std::str::from_utf8(slice).unwrap();
let semver = semver::Version::parse(ver).unwrap();
let result = (semver.major.min(65535) as u32) << 16
Expand Down
3 changes: 3 additions & 0 deletions src/common/hashtable/src/container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ use std::ops::DerefMut;
use std::ptr::null_mut;
use std::ptr::NonNull;

/// # Safety
///
/// Any foreign type shouldn't implement this trait.
pub unsafe trait Container
where Self: Deref<Target = [Self::T]> + DerefMut
{
Expand Down
12 changes: 11 additions & 1 deletion src/common/hashtable/src/hashtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ where
}
}

impl<K, V, A> Default for Hashtable<K, V, A>
where
K: Keyable,
A: Allocator + Clone + Default,
{
fn default() -> Self {
Self::new()
}
}

impl<K, V, A> Hashtable<K, V, A>
where
K: Keyable,
Expand Down Expand Up @@ -177,7 +187,7 @@ where
pub fn set_merge(&mut self, other: &Self) {
if let Some(entry) = other.zero.0.as_ref() {
self.zero = ZeroEntry(Some(Entry {
key: entry.key.clone(),
key: entry.key,
val: MaybeUninit::uninit(),
_alignment: [0; 0],
}));
Expand Down
12 changes: 11 additions & 1 deletion src/common/hashtable/src/stack_hashtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,16 @@ where
}
}

impl<K, V, A, const N: usize> Default for StackHashtable<K, V, N, A>
where
K: Keyable,
A: Allocator + Clone + Default,
{
fn default() -> Self {
Self::new()
}
}

impl<K, V, A, const N: usize> StackHashtable<K, V, N, A>
where
K: Keyable,
Expand Down Expand Up @@ -183,7 +193,7 @@ where
pub fn set_merge(&mut self, other: &Self) {
if let Some(entry) = other.zero.0.as_ref() {
self.zero = ZeroEntry(Some(Entry {
key: entry.key.clone(),
key: entry.key,
val: MaybeUninit::uninit(),
_alignment: [0; 0],
}));
Expand Down
10 changes: 9 additions & 1 deletion src/common/hashtable/src/table0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ impl<K: Keyable, V> Entry<K, V> {
unsafe { self.key.assume_init_ref() }
}
// this function can only be used in external crates
/// # Safety
///
/// The new key should be equals the old key.
#[inline(always)]
pub unsafe fn set_key(&mut self, key: K) {
self.key.write(key);
}
// this function can only be used in external crates
#[inline(always)]
pub fn get(&self) -> &V {
unsafe { self.val.assume_init_ref() }
Expand Down Expand Up @@ -267,7 +275,7 @@ where
pub unsafe fn set_merge(&mut self, other: &Self) {
assert!(self.capacity() >= self.len() + other.len());
for entry in other.iter() {
let key = *(*entry).key.assume_init_ref();
let key = entry.key.assume_init();
let _ = self.insert(key);
}
}
Expand Down
11 changes: 5 additions & 6 deletions src/common/hashtable/src/table1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use std::alloc::Allocator;

use super::table0::Entry;

type Ent<V> = Entry<[u8; 2], V>;

pub struct Table1<V, A: Allocator + Clone> {
pub(crate) data: Box<[Entry<[u8; 2], V>; 65536], A>,
pub(crate) len: usize,
Expand All @@ -39,15 +41,15 @@ impl<V, A: Allocator + Clone> Table1<V, A> {
pub fn len(&self) -> usize {
self.len
}
pub fn get(&self, key: [u8; 2]) -> Option<&Entry<[u8; 2], V>> {
pub fn get(&self, key: [u8; 2]) -> Option<&Ent<V>> {
let e = &self.data[key[1] as usize * 256 + key[0] as usize];
if unsafe { e.key.assume_init() } == key {
Some(e)
} else {
None
}
}
pub fn get_mut(&mut self, key: [u8; 2]) -> Option<&mut Entry<[u8; 2], V>> {
pub fn get_mut(&mut self, key: [u8; 2]) -> Option<&mut Ent<V>> {
let e = &mut self.data[key[1] as usize * 256 + key[0] as usize];
if unsafe { e.key.assume_init() } == key {
Some(e)
Expand All @@ -58,10 +60,7 @@ impl<V, A: Allocator + Clone> Table1<V, A> {
/// # Safety
///
/// The resulted `MaybeUninit` should be initialized immedidately.
pub fn insert(
&mut self,
key: [u8; 2],
) -> Result<&mut Entry<[u8; 2], V>, &mut Entry<[u8; 2], V>> {
pub fn insert(&mut self, key: [u8; 2]) -> Result<&mut Ent<V>, &mut Ent<V>> {
let e = &mut self.data[key[1] as usize * 256 + key[0] as usize];
if unsafe { e.key.assume_init() } == key {
Err(e)
Expand Down
13 changes: 10 additions & 3 deletions src/common/hashtable/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,16 @@ pub unsafe trait Keyable: Sized + Copy + Eq {
fn hash(&self) -> u64;
}

pub trait UnsizedKeyable {
/// # Safety
///
/// There shouldn't be any uninitialized bytes or interior mutability in `Self`.
/// The implementation promises `from_bytes(as_bytes(self))` is `self`.
/// `as_bytes` should returns a slice that is valid for reads.
pub unsafe trait UnsizedKeyable {
fn as_bytes(&self) -> &[u8];

/// # Safety
/// The argument should be a return value of `as_bytes` in the same implementation.
unsafe fn from_bytes(bytes: &[u8]) -> &Self;
}

Expand Down Expand Up @@ -153,7 +160,7 @@ unsafe impl<const N: usize> Keyable for [u8; N] {
}
}

impl UnsizedKeyable for [u8] {
unsafe impl UnsizedKeyable for [u8] {
fn as_bytes(&self) -> &[u8] {
self
}
Expand All @@ -163,7 +170,7 @@ impl UnsizedKeyable for [u8] {
}
}

impl UnsizedKeyable for str {
unsafe impl UnsizedKeyable for str {
fn as_bytes(&self) -> &[u8] {
self.as_bytes()
}
Expand Down
72 changes: 49 additions & 23 deletions src/common/hashtable/src/twolevel_hashtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,15 @@ use super::utils::ZeroEntry;
const BUCKETS: usize = 256;
const BUCKETS_LG2: u32 = 8;

type Tables<K, V, A> = [Table0<K, V, HeapContainer<Entry<K, V>, A>, A>; BUCKETS];

pub struct TwolevelHashtable<K, V, A = MmapAllocator<GlobalAllocator>>
where
K: Keyable,
A: Allocator + Clone,
{
zero: ZeroEntry<K, V>,
tables: [Table0<K, V, HeapContainer<Entry<K, V>, A>, A>; BUCKETS],
tables: Tables<K, V, A>,
}

unsafe impl<K: Keyable + Send, V: Send, A: Allocator + Clone + Send> Send
Expand All @@ -62,6 +64,16 @@ where
}
}

impl<K, V, A> Default for TwolevelHashtable<K, V, A>
where
K: Keyable,
A: Allocator + Clone + Default,
{
fn default() -> Self {
Self::new()
}
}

impl<K, V, A> TwolevelHashtable<K, V, A>
where
K: Keyable,
Expand Down Expand Up @@ -197,8 +209,8 @@ where
src.table.dropped = true;
res.zero = ZeroEntry(src.zero.take());
for entry in src.table.iter() {
let key = *(*entry).key.assume_init_ref();
let val = std::ptr::read((*entry).val.assume_init_ref());
let key = entry.key.assume_init();
let val = std::ptr::read(entry.val.assume_init_ref());
let hash = K::hash(&key);
let index = hash as usize >> (64u32 - BUCKETS_LG2);
if unlikely((res.tables[index].len() + 1) * 2 > res.tables[index].capacity()) {
Expand Down Expand Up @@ -232,7 +244,7 @@ where
pub fn set_merge(&mut self, other: &Self) {
if let Some(entry) = other.zero.0.as_ref() {
self.zero = ZeroEntry(Some(Entry {
key: entry.key.clone(),
key: entry.key,
val: MaybeUninit::uninit(),
_alignment: [0; 0],
}));
Expand All @@ -252,19 +264,21 @@ where
}
}

type TwolevelHashtableIterInner<'a, K, V, A> = std::iter::Chain<
std::option::Iter<'a, Entry<K, V>>,
std::iter::FlatMap<
std::slice::Iter<'a, Table0<K, V, HeapContainer<Entry<K, V>, A>, A>>,
Table0Iter<'a, K, V>,
fn(&'a Table0<K, V, HeapContainer<Entry<K, V>, A>, A>) -> Table0Iter<'a, K, V>,
>,
>;

pub struct TwolevelHashtableIter<'a, K, V, A = MmapAllocator<GlobalAllocator>>
where
K: Keyable,
A: Allocator + Clone,
{
inner: std::iter::Chain<
std::option::Iter<'a, Entry<K, V>>,
std::iter::FlatMap<
std::slice::Iter<'a, Table0<K, V, HeapContainer<Entry<K, V>, A>, A>>,
Table0Iter<'a, K, V>,
fn(&'a Table0<K, V, HeapContainer<Entry<K, V>, A>, A>) -> Table0Iter<'a, K, V>,
>,
>,
inner: TwolevelHashtableIterInner<'a, K, V, A>,
}

impl<'a, K, V, A> Iterator for TwolevelHashtableIter<'a, K, V, A>
Expand All @@ -279,19 +293,21 @@ where
}
}

type TwolevelHashtableIterMutInner<'a, K, V, A> = std::iter::Chain<
std::option::IterMut<'a, Entry<K, V>>,
std::iter::FlatMap<
std::slice::IterMut<'a, Table0<K, V, HeapContainer<Entry<K, V>, A>, A>>,
Table0IterMut<'a, K, V>,
fn(&'a mut Table0<K, V, HeapContainer<Entry<K, V>, A>, A>) -> Table0IterMut<'a, K, V>,
>,
>;

pub struct TwolevelHashtableIterMut<'a, K, V, A = MmapAllocator<GlobalAllocator>>
where
K: Keyable,
A: Allocator + Clone,
{
inner: std::iter::Chain<
std::option::IterMut<'a, Entry<K, V>>,
std::iter::FlatMap<
std::slice::IterMut<'a, Table0<K, V, HeapContainer<Entry<K, V>, A>, A>>,
Table0IterMut<'a, K, V>,
fn(&'a mut Table0<K, V, HeapContainer<Entry<K, V>, A>, A>) -> Table0IterMut<'a, K, V>,
>,
>,
inner: TwolevelHashtableIterMutInner<'a, K, V, A>,
}

impl<'a, K, V, A> Iterator for TwolevelHashtableIterMut<'a, K, V, A>
Expand All @@ -312,7 +328,7 @@ where
A: Allocator + Clone,
{
Onelevel(Hashtable<K, V, A>),
Twolevel(TwolevelHashtable<K, V, A>),
Twolevel(Box<TwolevelHashtable<K, V, A>>),
}

impl<K, V, A> HashtableKind<K, V, A>
Expand All @@ -328,6 +344,16 @@ where
}
}

impl<K, V, A> Default for HashtableKind<K, V, A>
where
K: Keyable,
A: Allocator + Clone + Default,
{
fn default() -> Self {
Self::new()
}
}

impl<K, V, A> HashtableKind<K, V, A>
where
K: Keyable,
Expand All @@ -337,7 +363,7 @@ where
Self::Onelevel(Hashtable::new_in(allocator))
}
pub fn new_twolevel_in(allocator: A) -> Self {
Self::Twolevel(TwolevelHashtable::new_in(allocator))
Self::Twolevel(Box::new(TwolevelHashtable::new_in(allocator)))
}
#[inline(always)]
pub fn is_empty(&self) -> bool {
Expand Down Expand Up @@ -437,7 +463,7 @@ where
unsafe {
if let Onelevel(x) = self {
let onelevel = std::ptr::read(x);
let twolevel = TwolevelHashtable::<K, V, A>::from(onelevel);
let twolevel = Box::new(TwolevelHashtable::<K, V, A>::from(onelevel));
std::ptr::write(self, Twolevel(twolevel));
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/common/hashtable/src/unsized_hashtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,16 @@ where
}
}

impl<K, V, A> Default for UnsizedHashtable<K, V, A>
where
K: UnsizedKeyable + ?Sized,
A: Allocator + Clone + Default,
{
fn default() -> Self {
Self::new()
}
}

impl<K, V, A> UnsizedHashtable<K, V, A>
where
K: UnsizedKeyable + ?Sized,
Expand Down
2 changes: 1 addition & 1 deletion src/common/hashtable/tests/it/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ fn test_unsized_hash_map() {
}
let mut hashtable = UnsizedHashMap::<[u8], U64>::new();
for s in sequence.iter() {
match unsafe { hashtable.insert_and_entry(&s) } {
match unsafe { hashtable.insert_and_entry(s) } {
Ok(mut e) => {
e.write(U64::new(1u64));
}
Expand Down
Loading

0 comments on commit 9724292

Please sign in to comment.