From 0c81995ea9f38caf1ba8b63e314634c1881558ac Mon Sep 17 00:00:00 2001 From: Yegor Bugayenko Date: Tue, 25 Apr 2023 07:05:19 +0300 Subject: [PATCH] #69 MaybeUninit for each array item --- .github/workflows/cargo.yml | 1 + .rultor.yml | 1 + README.md | 5 +---- src/clone.rs | 43 +++++++++++++++++++++++++++++++++++++ src/ctors.rs | 10 +++++++-- src/iterators.rs | 9 ++++---- src/lib.rs | 13 +++++------ src/map.rs | 37 +++++++++++++++++++------------ 8 files changed, 89 insertions(+), 30 deletions(-) create mode 100644 src/clone.rs diff --git a/.github/workflows/cargo.yml b/.github/workflows/cargo.yml index 9e88793..a6ccd97 100644 --- a/.github/workflows/cargo.yml +++ b/.github/workflows/cargo.yml @@ -18,6 +18,7 @@ jobs: with: toolchain: stable - run: cargo --color=never test --all-features -vv -- --nocapture + - run: cargo --color=never test --release --all-features -vv -- --nocapture - run: cargo --color=never fmt --check - run: cargo --color=never doc --no-deps - run: cargo --color=never clippy -- --no-deps diff --git a/.rultor.yml b/.rultor.yml index a298324..5130b57 100644 --- a/.rultor.yml +++ b/.rultor.yml @@ -16,6 +16,7 @@ release: sed -i -e "s/^version = \"0.0.0\"/version = \"${tag}\"/" Cargo.toml sed -i -e "s/0.0.0/${tag}/" src/lib.rs cargo --color=never test --all-features -vv -- --nocapture + cargo --color=never test --release --all-features -vv -- --nocapture cargo --color=never fmt --check cargo --color=never clippy -- --no-deps git commit -am "${tag}" diff --git a/README.md b/README.md index 906d2af..3215f9f 100644 --- a/README.md +++ b/README.md @@ -65,8 +65,6 @@ the numbers over 1.0 indicate performance gain, while the numbers below 1.0 demonstrate performance loss. -| | 2 | 4 | 8 | 16 | 32 | 64 | 128 | -| --- | --: | --: | --: | --: | --: | --: | --: | | `hashbrown::HashMap` | 18.04 | 3.79 | 2.34 | 1.44 | 0.67 | 0.30 | 0.17 | | `indexmap::IndexMap` | 14.77 | 6.44 | 4.30 | 2.42 | 1.44 | 0.63 | 0.35 | | `linear_map::LinearMap` | 2.50 | 0.76 | 0.65 | 0.60 | 0.63 | 0.65 | 0.63 | @@ -79,10 +77,9 @@ while the numbers below 1.0 demonstrate performance loss. | `std::collections::HashMap` | 46.50 | 6.26 | 3.98 | 2.38 | 1.35 | 0.62 | 0.36 | | `tinymap::array_map::ArrayMap` | 1.21 | 2.26 | 2.23 | 2.26 | 2.64 | 2.37 | 2.52 | -The experiment was performed on 24-04-2023. +The experiment was performed on 25-04-2023. There were 1000000 repetition cycles. The entire benchmark took 385s. - As you see, the highest performance gain was achieved for the maps that were smaller than ten keys. diff --git a/src/clone.rs b/src/clone.rs new file mode 100644 index 0000000..8a1b858 --- /dev/null +++ b/src/clone.rs @@ -0,0 +1,43 @@ +// Copyright (c) 2023 Yegor Bugayenko +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +use crate::Map; + +impl Clone for Map { + fn clone(&self) -> Self { + let mut m: Self = Self::new(); + for (k, v) in self.iter() { + m.insert(k.clone(), v.clone()); + } + m + } +} + +#[cfg(test)] +use anyhow::Result; + +#[test] +#[ignore] +fn map_can_be_cloned() -> Result<()> { + let mut m: Map = Map::new(); + m.insert(0, 42); + assert_eq!(42, *m.clone().get(&0).unwrap()); + Ok(()) +} diff --git a/src/ctors.rs b/src/ctors.rs index bbeef10..3a67461 100644 --- a/src/ctors.rs +++ b/src/ctors.rs @@ -21,7 +21,7 @@ use crate::{Map, Pair}; use std::mem::MaybeUninit; -impl Map { +impl Map { /// Make it. #[inline] #[must_use] @@ -30,7 +30,7 @@ impl Map { unsafe { Self { next: 0, - pairs: MaybeUninit::<[Pair; N]>::uninit().assume_init(), + pairs: MaybeUninit::<[MaybeUninit>; N]>::uninit().assume_init(), } } } @@ -45,3 +45,9 @@ fn makes_new_map() -> Result<()> { assert_eq!(0, m.len()); Ok(()) } + +#[test] +fn drops_correctly() -> Result<()> { + let _m: Map, u8, 8> = Map::new(); + Ok(()) +} diff --git a/src/iterators.rs b/src/iterators.rs index a9c133c..e23b452 100644 --- a/src/iterators.rs +++ b/src/iterators.rs @@ -27,7 +27,8 @@ impl<'a, K: Clone, V: Clone, const N: usize> Iterator for Iter<'a, K, V, N> { #[must_use] fn next(&mut self) -> Option { while self.pos < self.next { - if let Present(p) = &self.pairs[self.pos] { + let p = unsafe { self.pairs[self.pos].assume_init_ref() }; + if let Present(p) = p { self.pos += 1; return Some((&p.0, &p.1)); } @@ -44,10 +45,10 @@ impl<'a, K: Clone, V: Clone, const N: usize> Iterator for IntoIter<'a, K, V, N> #[must_use] fn next(&mut self) -> Option { while self.pos < self.next { - if self.pairs[self.pos].is_some() { - let pair = self.pairs[self.pos].clone().unwrap(); + let p = unsafe { self.pairs[self.pos].assume_init_ref() }; + if p.is_some() { self.pos += 1; - return Some(pair); + return Some(p.clone().unwrap()); } self.pos += 1; } diff --git a/src/lib.rs b/src/lib.rs index c778b7b..2c076e9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -45,6 +45,7 @@ #![allow(clippy::multiple_inherent_impl)] #![allow(clippy::multiple_crate_versions)] +mod clone; mod ctors; mod debug; mod eq; @@ -57,7 +58,7 @@ mod pair; mod serialization; /// A pair in the Map. -#[derive(Clone, Default, Copy, Eq, PartialEq)] +#[derive(Clone, Default, Eq, PartialEq)] enum Pair { Present((K, V)), #[default] @@ -72,28 +73,28 @@ enum Pair { /// because it doesn't use heap. When a [`Map`] is being created, it allocates the necessary /// space on stack. That's why the maximum size of the map must be provided in /// compile time. -#[derive(Clone, Copy)] -pub struct Map { +pub struct Map { next: usize, - pairs: [Pair; N], + pairs: [MaybeUninit>; N], } /// Iterator over the [`Map`]. pub struct Iter<'a, K, V, const N: usize> { next: usize, pos: usize, - pairs: &'a [Pair; N], + pairs: &'a [MaybeUninit>; N], } /// Into-iterator over the [`Map`]. pub struct IntoIter<'a, K, V, const N: usize> { next: usize, pos: usize, - pairs: &'a [Pair; N], + pairs: &'a [MaybeUninit>; N], } #[cfg(test)] use simple_logger::SimpleLogger; +use std::mem::MaybeUninit; #[cfg(test)] use log::LevelFilter; diff --git a/src/map.rs b/src/map.rs index 068729c..b6d4975 100644 --- a/src/map.rs +++ b/src/map.rs @@ -21,8 +21,9 @@ use crate::Pair::{Absent, Present}; use crate::{IntoIter, Iter, Map}; use std::borrow::Borrow; +use std::mem::MaybeUninit; -impl Default for Map { +impl Default for Map { fn default() -> Self { Self::new() } @@ -64,7 +65,8 @@ impl Map { pub fn len(&self) -> usize { let mut busy = 0; for i in 0..self.next { - if self.pairs[i].is_some() { + let p = unsafe { self.pairs[i].assume_init_ref() }; + if p.is_some() { busy += 1; } } @@ -75,7 +77,8 @@ impl Map { #[inline] pub fn contains_key(&self, k: &K) -> bool { for i in 0..self.next { - if let Present((bk, _bv)) = &self.pairs[i] { + let p = unsafe { self.pairs[i].assume_init_ref() }; + if let Present((bk, _bv)) = &p { if bk == k { return true; } @@ -88,9 +91,10 @@ impl Map { #[inline] pub fn remove(&mut self, k: &K) { for i in 0..self.next { - if let Present((bk, _bv)) = &self.pairs[i] { + let p = unsafe { self.pairs[i].assume_init_ref() }; + if let Present((bk, _bv)) = &p { if bk == k { - self.pairs[i] = Absent; + self.pairs[i] = MaybeUninit::new(Absent); break; } } @@ -106,13 +110,14 @@ impl Map { pub fn insert(&mut self, k: K, v: V) { self.remove(&k); for i in 0..self.next { - if !self.pairs[i].is_some() { - self.pairs[i] = Present((k, v)); + let p = unsafe { self.pairs[i].assume_init_ref() }; + if !p.is_some() { + self.pairs[i] = MaybeUninit::new(Present((k, v))); return; } } if self.next < N { - self.pairs[self.next] = Present((k, v)); + self.pairs[self.next] = MaybeUninit::new(Present((k, v))); self.next += 1; return; } @@ -127,7 +132,8 @@ impl Map { K: Borrow, { for i in 0..self.next { - if let Present(p) = &self.pairs[i] { + let p = unsafe { self.pairs[i].assume_init_ref() }; + if let Present(p) = &p { if p.0.borrow() == k { return Some(&p.1); } @@ -148,9 +154,11 @@ impl Map { K: Borrow, { for i in 0..self.next { - if let Present(p) = &mut self.pairs[i] { - if p.0.borrow() == k { - return Some(&mut self.pairs[i].as_mut().unwrap().1); + let p = unsafe { self.pairs[i].assume_init_ref() }; + if let Present(p1) = &p { + if p1.0.borrow() == k { + let p2 = unsafe { self.pairs[i].assume_init_mut() }; + return Some(&mut p2.as_mut().unwrap().1); } } } @@ -167,9 +175,10 @@ impl Map { #[inline] pub fn retain bool>(&mut self, f: F) { for i in 0..self.next { - if let Present((k, v)) = &self.pairs[i] { + let p = unsafe { self.pairs[i].assume_init_ref() }; + if let Present((k, v)) = &p { if !f(k, v) { - self.pairs[i] = Absent; + self.pairs[i] = MaybeUninit::new(Absent); } } }