From bebe21294facbbd132b4e8d470eadb276e17f2b8 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 2 Aug 2021 22:22:18 -0700 Subject: [PATCH 1/2] Allow `Relation` to be joined with `Variable<(T, S)>` This case occurred in `Polonius`, where we had to create a separate `Relation<(T, ())>` to make things work. Implementing this required a change to the interface of `JoinInput`, since we are not allowed to assume that a `Relation` and a `Relation<(T, ())>` are layout compatible. Now, `stable` takes a callback instead of returning a slice directly. --- src/join.rs | 50 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/src/join.rs b/src/join.rs index 3dbf6d1..e5917ce 100644 --- a/src/join.rs +++ b/src/join.rs @@ -51,13 +51,13 @@ fn join_delta<'me, Key: Ord, Val1: Ord, Val2: Ord>( let recent1 = input1.recent(); let recent2 = input2.recent(); - for batch2 in input2.stable().iter() { + input2.for_each_stable_set(|batch2| { join_helper(&recent1, &batch2, &mut result); - } + }); - for batch1 in input1.stable().iter() { + input1.for_each_stable_set(|batch1| { join_helper(&batch1, &recent2, &mut result); - } + }); join_helper(&recent1, &recent2, &mut result); } @@ -164,40 +164,56 @@ pub trait JoinInput<'me, Tuple: Ord>: Copy { /// empty slice.) type RecentTuples: Deref; - /// If we are on iteration N of the loop, these are the tuples - /// added on iteration N - 2 or before. (For a `Relation`, this is - /// just `self`.) - type StableTuples: Deref]>; - /// Get the set of recent tuples. fn recent(self) -> Self::RecentTuples; - /// Get the set of stable tuples. - fn stable(self) -> Self::StableTuples; + /// Call a function for each set of stable tuples. + fn for_each_stable_set(self, f: impl FnMut(&[Tuple])); } impl<'me, Tuple: Ord> JoinInput<'me, Tuple> for &'me Variable { type RecentTuples = Ref<'me, [Tuple]>; - type StableTuples = Ref<'me, [Relation]>; fn recent(self) -> Self::RecentTuples { Ref::map(self.recent.borrow(), |r| &r.elements[..]) } - fn stable(self) -> Self::StableTuples { - Ref::map(self.stable.borrow(), |v| &v[..]) + fn for_each_stable_set(self, mut f: impl FnMut(&[Tuple])) { + for stable in self.stable.borrow().iter() { + f(stable) + } } } impl<'me, Tuple: Ord> JoinInput<'me, Tuple> for &'me Relation { type RecentTuples = &'me [Tuple]; - type StableTuples = &'me [Relation]; fn recent(self) -> Self::RecentTuples { &[] } - fn stable(self) -> Self::StableTuples { - std::slice::from_ref(self) + fn for_each_stable_set(self, mut f: impl FnMut(&[Tuple])) { + f(&self.elements) + } +} + +impl<'me, Tuple: Ord> JoinInput<'me, (Tuple, ())> for &'me Relation { + type RecentTuples = &'me [(Tuple, ())]; + + fn recent(self) -> Self::RecentTuples { + &[] + } + + fn for_each_stable_set(self, mut f: impl FnMut(&[(Tuple, ())])) { + use std::mem; + assert_eq!(mem::size_of::<(Tuple, ())>(), mem::size_of::()); + assert_eq!(mem::align_of::<(Tuple, ())>(), mem::align_of::()); + + // SAFETY: https://rust-lang.github.io/unsafe-code-guidelines/layout/structs-and-tuples.html#structs-with-1-zst-fields + // guarantees that `T` is layout compatible with `(T, ())`, since `()` is a 1-ZST. + let elements: &'me [Tuple] = self.elements.as_slice(); + let elements: &'me [(Tuple, ())] = unsafe { std::mem::transmute(elements) }; + + f(elements) } } From 586043f632cdafdb825cc83bdfc4afc2937dfcc3 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 27 Aug 2021 07:28:11 -0700 Subject: [PATCH 2/2] Use `slice::from_raw_parts` instead of `mem::transmute` Layout compatibility guarantees for `T` do not extend to types containing `T` (like `&[T]`), so use `from_raw_parts` instead of `mem::transmute`. --- src/join.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/join.rs b/src/join.rs index e5917ce..dbf1f67 100644 --- a/src/join.rs +++ b/src/join.rs @@ -210,9 +210,14 @@ impl<'me, Tuple: Ord> JoinInput<'me, (Tuple, ())> for &'me Relation { assert_eq!(mem::align_of::<(Tuple, ())>(), mem::align_of::()); // SAFETY: https://rust-lang.github.io/unsafe-code-guidelines/layout/structs-and-tuples.html#structs-with-1-zst-fields - // guarantees that `T` is layout compatible with `(T, ())`, since `()` is a 1-ZST. + // guarantees that `T` is layout compatible with `(T, ())`, since `()` is a 1-ZST. We use + // `slice::from_raw_parts` because the layout compatibility guarantee does not extend to + // containers like `&[T]`. let elements: &'me [Tuple] = self.elements.as_slice(); - let elements: &'me [(Tuple, ())] = unsafe { std::mem::transmute(elements) }; + let len = elements.len(); + + let elements: &'me [(Tuple, ())] = + unsafe { std::slice::from_raw_parts(elements.as_ptr() as *const _, len) }; f(elements) }