Skip to content

Commit

Permalink
First draft of a sound ShallowCopy
Browse files Browse the repository at this point in the history
This is trying to address the unsoundness that arises from the current
version of `ShallowCopy` (see #74). In the process, it also deals with
the fact that casting between `Inner<.., T, ..>` and `Inner<..,
ManuallyDrop<T>, ..>` is likely not sound
(rust-lang/unsafe-code-guidelines#35 (comment)).
It does not yet work.
  • Loading branch information
jonhoo committed Nov 29, 2020
1 parent 04e7b24 commit e758133
Show file tree
Hide file tree
Showing 12 changed files with 793 additions and 423 deletions.
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
[workspace]
members = ["evmap", "evmap-derive", "left-right"]
members = ["evmap", "left-right"]
exclude = ["evmap-derive"]
7 changes: 6 additions & 1 deletion evmap/src/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ pub(crate) use indexmap::IndexMap as MapImpl;
pub(crate) use std::collections::HashMap as MapImpl;

use crate::values::Values;
use crate::ShallowCopy;

pub(crate) struct Inner<K, V, M, S>
where
K: Eq + Hash,
V: ShallowCopy,
S: BuildHasher,
{
pub(crate) data: MapImpl<K, Values<V, S>, S>,
Expand All @@ -22,7 +24,8 @@ impl<K, V, M, S> fmt::Debug for Inner<K, V, M, S>
where
K: Eq + Hash + fmt::Debug,
S: BuildHasher,
V: fmt::Debug,
V: ShallowCopy,
V::Target: fmt::Debug,
M: fmt::Debug,
{
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand All @@ -38,6 +41,7 @@ impl<K, V, M, S> Clone for Inner<K, V, M, S>
where
K: Eq + Hash + Clone,
S: BuildHasher + Clone,
V: ShallowCopy,
M: Clone,
{
fn clone(&self) -> Self {
Expand All @@ -56,6 +60,7 @@ where
impl<K, V, M, S> Inner<K, V, M, S>
where
K: Eq + Hash,
V: ShallowCopy,
S: BuildHasher,
{
pub fn with_hasher(m: M, hash_builder: S) -> Self {
Expand Down
71 changes: 51 additions & 20 deletions evmap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ pub use crate::read::{MapReadRef, ReadGuardIter, ReadHandle, ReadHandleFactory};

pub mod shallow_copy;
pub use crate::shallow_copy::ShallowCopy;
use shallow_copy::MaybeShallowCopied;

// Expose `ReadGuard` since it has useful methods the user will likely care about.
#[doc(inline)]
Expand All @@ -225,27 +226,27 @@ pub use left_right::ReadGuard;
///
/// The predicate function is called once for each distinct value, and `true` if this is the
/// _first_ call to the predicate on the _second_ application of the operation.
pub struct Predicate<V>(pub(crate) Box<dyn FnMut(&V, bool) -> bool + Send>);
pub struct Predicate<V: ?Sized>(pub(crate) Box<dyn FnMut(&V, bool) -> bool + Send>);

impl<V> Predicate<V> {
impl<V: ?Sized> Predicate<V> {
/// Evaluate the predicate for the given element
#[inline]
pub fn eval(&mut self, value: &V, reset: bool) -> bool {
(*self.0)(value, reset)
}
}

impl<V> PartialEq for Predicate<V> {
impl<V: ?Sized> PartialEq for Predicate<V> {
#[inline]
fn eq(&self, other: &Self) -> bool {
// only compare data, not vtable: https://stackoverflow.com/q/47489449/472927
&*self.0 as *const _ as *const () == &*other.0 as *const _ as *const ()
}
}

impl<V> Eq for Predicate<V> {}
impl<V: ?Sized> Eq for Predicate<V> {}

impl<V> fmt::Debug for Predicate<V> {
impl<V: ?Sized> fmt::Debug for Predicate<V> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_tuple("Predicate")
.field(&format_args!("{:p}", &*self.0 as *const _))
Expand All @@ -255,12 +256,14 @@ impl<V> fmt::Debug for Predicate<V> {

/// A pending map operation.
#[non_exhaustive]
#[derive(PartialEq, Eq, Debug)]
pub(crate) enum Operation<K, V, M> {
pub(crate) enum Operation<K, V, M>
where
V: ShallowCopy,
{
/// Replace the set of entries for this key with this value.
Replace(K, V),
Replace(K, MaybeShallowCopied<V>),
/// Add this value to the set of entries for this key.
Add(K, V),
Add(K, MaybeShallowCopied<V>),
/// Remove this value from the set of entries for this key.
RemoveValue(K, V),
/// Remove the value set for this key.
Expand All @@ -277,24 +280,48 @@ pub(crate) enum Operation<K, V, M> {
/// Note that this will iterate once over all the keys internally.
Purge,
/// Retains all values matching the given predicate.
Retain(K, Predicate<V>),
/// Shrinks [`Values`] to their minimum necessary size, freeing memory
Retain(K, Predicate<V::Target>),
/// Shrinks [`MaybeShallowCopied<V>alues`] to their minimum necessary size, freeing memory
/// and potentially improving cache locality.
///
/// If no key is given, all `Values` will shrink to fit.
/// If no key is given, all `MaybeShallowCopied<V>alues` will shrink to fit.
Fit(Option<K>),
/// Reserves capacity for some number of additional elements in [`Values`]
/// Reserves capacity for some number of additional elements in [`MaybeShallowCopied<V>alues`]
/// for the given key. If the given key does not exist, allocate an empty
/// `Values` with the given capacity.
/// `MaybeShallowCopied<V>alues` with the given capacity.
///
/// This can improve performance by pre-allocating space for large bags of values.
Reserve(K, usize),
/// Mark the map as ready to be consumed for readers.
MarkReady,
/// Set the value of the map meta.
SetMeta(M),
/// Copy over the contents of the read map wholesale as the write map is empty.
JustCloneRHandle,
}

impl<K, V, M> fmt::Debug for Operation<K, V, M>
where
K: fmt::Debug,
V: ShallowCopy + fmt::Debug,
V::Target: fmt::Debug,
M: fmt::Debug,
{
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match *self {
Operation::Replace(ref a, ref b) => f.debug_tuple("Replace").field(a).field(b).finish(),
Operation::Add(ref a, ref b) => f.debug_tuple("Add").field(a).field(b).finish(),
Operation::RemoveValue(ref a, ref b) => {
f.debug_tuple("RemoveValue").field(a).field(b).finish()
}
Operation::RemoveEntry(ref a) => f.debug_tuple("RemoveEntry").field(a).finish(),
Operation::Clear(ref a) => f.debug_tuple("Clear").field(a).finish(),
Operation::Purge => f.debug_tuple("Purge").finish(),
Operation::Retain(ref a, ref b) => f.debug_tuple("Retain").field(a).field(b).finish(),
Operation::Fit(ref a) => f.debug_tuple("Fit").field(a).finish(),
Operation::Reserve(ref a, ref b) => f.debug_tuple("Reserve").field(a).field(b).finish(),
Operation::MarkReady => f.debug_tuple("MarkReady").finish(),
Operation::SetMeta(ref a) => f.debug_tuple("SetMeta").field(a).finish(),
}
}
}

/// Options for how to initialize the map.
Expand Down Expand Up @@ -373,7 +400,8 @@ where
where
K: Eq + Hash + Clone,
S: BuildHasher + Clone,
V: Eq + Hash + ShallowCopy,
V: ShallowCopy,
V::Target: Eq + Hash,
M: 'static + Clone,
{
let inner = if let Some(cap) = self.capacity {
Expand All @@ -399,7 +427,8 @@ pub fn new<K, V>() -> (
)
where
K: Eq + Hash + Clone,
V: Eq + Hash + ShallowCopy,
V: ShallowCopy,
V::Target: Eq + Hash,
{
Options::default().construct()
}
Expand All @@ -416,7 +445,8 @@ pub fn with_meta<K, V, M>(
)
where
K: Eq + Hash + Clone,
V: Eq + Hash + ShallowCopy,
V: ShallowCopy,
V::Target: Eq + Hash,
M: 'static + Clone,
{
Options::default().with_meta(meta).construct()
Expand All @@ -432,7 +462,8 @@ pub fn with_hasher<K, V, M, S>(
) -> (WriteHandle<K, V, M, S>, ReadHandle<K, V, M, S>)
where
K: Eq + Hash + Clone,
V: Eq + Hash + ShallowCopy,
V: ShallowCopy,
V::Target: Eq + Hash,
M: 'static + Clone,
S: BuildHasher + Clone,
{
Expand Down
16 changes: 10 additions & 6 deletions evmap/src/read.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::inner::Inner;
use crate::values::Values;
use crate::ShallowCopy;
use left_right::ReadGuard;

use std::borrow::Borrow;
Expand All @@ -22,6 +23,7 @@ pub use factory::ReadHandleFactory;
pub struct ReadHandle<K, V, M = (), S = RandomState>
where
K: Eq + Hash,
V: ShallowCopy,
S: BuildHasher,
{
pub(crate) handle: left_right::ReadHandle<Inner<K, V, M, S>>,
Expand All @@ -30,6 +32,7 @@ where
impl<K, V, M, S> fmt::Debug for ReadHandle<K, V, M, S>
where
K: Eq + Hash + fmt::Debug,
V: ShallowCopy,
S: BuildHasher,
M: fmt::Debug,
{
Expand All @@ -43,6 +46,7 @@ where
impl<K, V, M, S> Clone for ReadHandle<K, V, M, S>
where
K: Eq + Hash,
V: ShallowCopy,
S: BuildHasher,
{
fn clone(&self) -> Self {
Expand All @@ -55,6 +59,7 @@ where
impl<K, V, M, S> ReadHandle<K, V, M, S>
where
K: Eq + Hash,
V: ShallowCopy,
S: BuildHasher,
{
pub(crate) fn new(handle: left_right::ReadHandle<Inner<K, V, M, S>>) -> Self {
Expand All @@ -65,7 +70,8 @@ where
impl<K, V, M, S> ReadHandle<K, V, M, S>
where
K: Eq + Hash,
V: Eq + Hash,
V: ShallowCopy,
V::Target: Eq + Hash,
S: BuildHasher,
M: Clone,
{
Expand Down Expand Up @@ -150,7 +156,7 @@ where
/// refreshed by the writer. If no refresh has happened, or the map has been destroyed, this
/// function returns `None`.
#[inline]
pub fn get_one<'rh, Q: ?Sized>(&'rh self, key: &'_ Q) -> Option<ReadGuard<'rh, V>>
pub fn get_one<'rh, Q: ?Sized>(&'rh self, key: &'_ Q) -> Option<ReadGuard<'rh, V::Target>>
where
K: Borrow<Q>,
Q: Hash + Eq,
Expand Down Expand Up @@ -204,14 +210,12 @@ where

/// Returns true if the map contains the specified value for the specified key.
///
/// The key and value may be any borrowed form of the map's respective types, but `Hash` and
/// The key may be any borrowed form of the map's key type, but `Hash` and
/// `Eq` on the borrowed form *must* match.
pub fn contains_value<Q: ?Sized, W: ?Sized>(&self, key: &Q, value: &W) -> bool
pub fn contains_value<Q: ?Sized>(&self, key: &Q, value: &V::Target) -> bool
where
K: Borrow<Q>,
V: Borrow<W>,
Q: Hash + Eq,
W: Hash + Eq,
{
self.get_raw(key.borrow())
.map(|x| x.contains(value))
Expand Down
5 changes: 5 additions & 0 deletions evmap/src/read/factory.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use super::ReadHandle;
use crate::inner::Inner;
use crate::ShallowCopy;
use std::collections::hash_map::RandomState;
use std::fmt;
use std::hash::{BuildHasher, Hash};
Expand All @@ -13,6 +14,7 @@ use std::hash::{BuildHasher, Hash};
pub struct ReadHandleFactory<K, V, M, S = RandomState>
where
K: Eq + Hash,
V: ShallowCopy,
S: BuildHasher,
{
pub(super) factory: left_right::ReadHandleFactory<Inner<K, V, M, S>>,
Expand All @@ -21,6 +23,7 @@ where
impl<K, V, M, S> fmt::Debug for ReadHandleFactory<K, V, M, S>
where
K: Eq + Hash,
V: ShallowCopy,
S: BuildHasher,
{
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand All @@ -33,6 +36,7 @@ where
impl<K, V, M, S> Clone for ReadHandleFactory<K, V, M, S>
where
K: Eq + Hash,
V: ShallowCopy,
S: BuildHasher,
{
fn clone(&self) -> Self {
Expand All @@ -45,6 +49,7 @@ where
impl<K, V, M, S> ReadHandleFactory<K, V, M, S>
where
K: Eq + Hash,
V: ShallowCopy,
S: BuildHasher,
{
/// Produce a new [`ReadHandle`] to the same left-right data structure as this factory was
Expand Down
Loading

0 comments on commit e758133

Please sign in to comment.