From 74435b4c3eb6c561074174dca943ead5f2fb4ae7 Mon Sep 17 00:00:00 2001 From: Frank McSherry Date: Tue, 30 Jan 2024 16:34:27 -0500 Subject: [PATCH 1/2] Canonicalize PointStamp representation --- src/dynamic/mod.rs | 2 ++ src/dynamic/pointstamp.rs | 24 +++++++++++++++++------- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/dynamic/mod.rs b/src/dynamic/mod.rs index a1c116405..3ab2c735b 100644 --- a/src/dynamic/mod.rs +++ b/src/dynamic/mod.rs @@ -52,9 +52,11 @@ where data.swap(&mut vector); let mut new_time = cap.time().clone(); new_time.inner.vector.truncate(level - 1); + new_time.inner.enforce(); let new_cap = cap.delayed(&new_time); for (_data, time, _diff) in vector.iter_mut() { time.inner.vector.truncate(level - 1); + time.inner.enforce(); } output.session(&new_cap).give_vec(&mut vector); }); diff --git a/src/dynamic/pointstamp.rs b/src/dynamic/pointstamp.rs index 199e7e479..c888aedf4 100644 --- a/src/dynamic/pointstamp.rs +++ b/src/dynamic/pointstamp.rs @@ -17,7 +17,7 @@ use serde::{Deserialize, Serialize}; /// A sequence of timestamps, partially ordered by the product order. /// /// Sequences of different lengths are compared as if extended indefinitely by `T::minimum()`. -/// Sequences are not guaranteed to be "minimal", and may end with `T::minimum()` entries. +/// Sequences are guaranteed to be "minimal", and may not end with `T::minimum()` entries. #[derive( Hash, Default, Clone, Eq, PartialEq, Ord, PartialOrd, Debug, Serialize, Deserialize, Abomonation, )] @@ -26,10 +26,20 @@ pub struct PointStamp { pub vector: Vec, } -impl PointStamp { +impl PointStamp { /// Create a new sequence. + /// + /// This method will modify `vector` to ensure it does not end with `T::minimum()`. pub fn new(vector: Vec) -> Self { - PointStamp { vector } + let mut result = PointStamp { vector }; + result.enforce(); + result + } + /// Enforces that `self` not end with `T::minimum()` by popping elements until it is true. + pub fn enforce(&mut self) { + while self.vector.last() == Some(&T::minimum()) { + self.vector.pop(); + } } } @@ -109,7 +119,7 @@ impl PathSummary> for PointStampSummary vector.push(action.results_in(&T::minimum())?); } - Some(PointStamp { vector }) + Some(PointStamp::new(vector)) } fn followed_by(&self, other: &Self) -> Option { // The output `retain` will be the minimum of the two inputs. @@ -166,7 +176,7 @@ impl PartialOrder for PointStampSummary { use timely::progress::Timestamp; impl Timestamp for PointStamp { fn minimum() -> Self { - Self { vector: Vec::new() } + Self::new(Vec::new()) } type Summary = PointStampSummary; } @@ -190,7 +200,7 @@ impl Lattice for PointStamp { for time in &other.vector[min_len..] { vector.push(time.clone()); } - Self { vector } + Self::new(vector) } fn meet(&self, other: &Self) -> Self { let min_len = ::std::cmp::min(self.vector.len(), other.vector.len()); @@ -200,7 +210,7 @@ impl Lattice for PointStamp { vector.push(self.vector[index].meet(&other.vector[index])); } // Remaining coordinates are `T::minimum()` in one input, and so in the output. - Self { vector } + Self::new(vector) } } From 24244a9b41b5a1beb841fd12bd03538324bcd733 Mon Sep 17 00:00:00 2001 From: Frank McSherry Date: Tue, 30 Jan 2024 16:45:56 -0500 Subject: [PATCH 2/2] Make PointStamp::vector private --- src/dynamic/mod.rs | 10 ++++++---- src/dynamic/pointstamp.rs | 23 +++++++++++++---------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/dynamic/mod.rs b/src/dynamic/mod.rs index 3ab2c735b..4d1eeff43 100644 --- a/src/dynamic/mod.rs +++ b/src/dynamic/mod.rs @@ -51,12 +51,14 @@ where input.for_each(|cap, data| { data.swap(&mut vector); let mut new_time = cap.time().clone(); - new_time.inner.vector.truncate(level - 1); - new_time.inner.enforce(); + let mut vec = std::mem::take(&mut new_time.inner).into_vec(); + vec.truncate(level - 1); + new_time.inner = PointStamp::new(vec); let new_cap = cap.delayed(&new_time); for (_data, time, _diff) in vector.iter_mut() { - time.inner.vector.truncate(level - 1); - time.inner.enforce(); + let mut vec = std::mem::take(&mut time.inner).into_vec(); + vec.truncate(level - 1); + time.inner = PointStamp::new(vec); } output.session(&new_cap).give_vec(&mut vector); }); diff --git a/src/dynamic/pointstamp.rs b/src/dynamic/pointstamp.rs index c888aedf4..60f4e239f 100644 --- a/src/dynamic/pointstamp.rs +++ b/src/dynamic/pointstamp.rs @@ -23,23 +23,26 @@ use serde::{Deserialize, Serialize}; )] pub struct PointStamp { /// A sequence of timestamps corresponding to timestamps in a sequence of nested scopes. - pub vector: Vec, + vector: Vec, } impl PointStamp { /// Create a new sequence. /// /// This method will modify `vector` to ensure it does not end with `T::minimum()`. - pub fn new(vector: Vec) -> Self { - let mut result = PointStamp { vector }; - result.enforce(); - result - } - /// Enforces that `self` not end with `T::minimum()` by popping elements until it is true. - pub fn enforce(&mut self) { - while self.vector.last() == Some(&T::minimum()) { - self.vector.pop(); + pub fn new(mut vector: Vec) -> Self { + while vector.last() == Some(&T::minimum()) { + vector.pop(); } + PointStamp { vector } + } + /// Returns the wrapped vector. + /// + /// This method is the support way to mutate the contents of `self`, by extracting + /// the vector and then re-introducting it with `PointStamp::new` to re-establish + /// the invariant that the vector not end with `T::minimum`. + pub fn into_vec(self) -> Vec { + self.vector } }