Skip to content

Commit 922902c

Browse files
committed
Remove Container::clear
I validated all call sites to handle non-empty containers gracefully. Container builders already need to handle it, and inputs should do so, too. Signed-off-by: Moritz Hoffmann <[email protected]>
1 parent 45bda3b commit 922902c

File tree

4 files changed

+7
-37
lines changed

4 files changed

+7
-37
lines changed

container/src/lib.rs

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,18 @@ use std::collections::VecDeque;
77
/// A container transferring data through dataflow edges
88
///
99
/// A container stores a number of elements and thus is able to describe it length (`len()`) and
10-
/// whether it is empty (`is_empty()`). It supports removing all elements (`clear`).
10+
/// whether it is empty (`is_empty()`).
1111
///
1212
/// A container must implement default. The default implementation is not required to allocate
1313
/// memory for variable-length components.
1414
///
1515
/// We require the container to be cloneable to enable efficient copies when providing references
1616
/// of containers to operators. Care must be taken that the type's `clone_from` implementation
1717
/// is efficient (which is not necessarily the case when deriving `Clone`.)
18+
// The container is `Default` because `CapacityContainerBuilder` only implements `ContainerBuilder`
19+
// for containers that implement `Default`, and we use the associated `::Container` all over Timely.
20+
// We can only access the type if all requirements for the `ContainerBuilder` implementation are
21+
// satisfied.
1822
pub trait Container: Default {
1923
/// The type of elements when reading non-destructively from the container.
2024
type ItemRef<'a> where Self: 'a;
@@ -35,10 +39,6 @@ pub trait Container: Default {
3539
self.len() == 0
3640
}
3741

38-
/// Remove all contents from `self` while retaining allocated memory.
39-
/// After calling `clear`, `is_empty` must return `true` and `len` 0.
40-
fn clear(&mut self);
41-
4242
/// Iterator type when reading from the container.
4343
type Iter<'a>: Iterator<Item=Self::ItemRef<'a>> where Self: 'a;
4444

@@ -96,6 +96,8 @@ pub trait PushInto<T> {
9696
/// decide to represent a push order for `extract` and `finish`, or not.
9797
pub trait ContainerBuilder: Default + 'static {
9898
/// The container type we're building.
99+
// The container is `Clone` because `Tee` requires it, otherwise we need to repeat it
100+
// all over Timely. `'static` because we don't want lifetimes everywhere.
99101
type Container: Container + Clone + 'static;
100102
/// Extract assembled containers, potentially leaving unfinished data behind. Can
101103
/// be called repeatedly, for example while the caller can send data.
@@ -117,7 +119,6 @@ pub trait ContainerBuilder: Default + 'static {
117119
let index = index(&datum);
118120
builders[index].push_into(datum);
119121
}
120-
container.clear();
121122
}
122123

123124
/// Indicates a good moment to release resources.
@@ -205,8 +206,6 @@ impl<T> Container for Vec<T> {
205206
Vec::is_empty(self)
206207
}
207208

208-
fn clear(&mut self) { Vec::clear(self) }
209-
210209
type Iter<'a> = std::slice::Iter<'a, T> where Self: 'a;
211210

212211
fn iter(&self) -> Self::Iter<'_> {
@@ -276,15 +275,6 @@ mod rc {
276275
std::ops::Deref::deref(self).is_empty()
277276
}
278277

279-
fn clear(&mut self) {
280-
// Try to reuse the allocation if possible
281-
if let Some(inner) = Rc::get_mut(self) {
282-
inner.clear();
283-
} else {
284-
*self = Self::default();
285-
}
286-
}
287-
288278
type Iter<'a> = T::Iter<'a> where Self: 'a;
289279

290280
fn iter(&self) -> Self::Iter<'_> {
@@ -317,15 +307,6 @@ mod arc {
317307
std::ops::Deref::deref(self).is_empty()
318308
}
319309

320-
fn clear(&mut self) {
321-
// Try to reuse the allocation if possible
322-
if let Some(inner) = Arc::get_mut(self) {
323-
inner.clear();
324-
} else {
325-
*self = Self::default();
326-
}
327-
}
328-
329310
type Iter<'a> = T::Iter<'a> where Self: 'a;
330311

331312
fn iter(&self) -> Self::Iter<'_> {

timely/examples/columnar.rs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -179,15 +179,6 @@ mod container {
179179

180180
impl<C: columnar::ContainerBytes> timely::Container for Column<C> {
181181
fn len(&self) -> usize { self.borrow().len() }
182-
// This sets `self` to be an empty `Typed` variant, appropriate for pushing into.
183-
fn clear(&mut self) {
184-
match self {
185-
Column::Typed(t) => t.clear(),
186-
Column::Bytes(_) => *self = Column::Typed(Default::default()),
187-
Column::Align(_) => *self = Column::Typed(Default::default()),
188-
}
189-
}
190-
191182
type ItemRef<'a> = C::Ref<'a>;
192183
type Iter<'a> = IterOwn<C::Borrowed<'a>>;
193184
fn iter<'a>(&'a self) -> Self::Iter<'a> { self.borrow().into_index_iter() }

timely/src/dataflow/channels/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ impl<T, C: Container> Message<T, C> {
5151

5252
if let Some(message) = bundle {
5353
*buffer = message.data;
54-
buffer.clear();
5554
}
5655
}
5756
}

timely/src/dataflow/operators/core/input.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,6 @@ impl<T: Timestamp, CB: ContainerBuilder> Handle<T, CB> {
392392
Message::push_at(container, now_at.clone(), &mut pushers[index]);
393393
}
394394
}
395-
container.clear();
396395
}
397396

398397
/// Closes the current epoch, flushing if needed, shutting if needed, and updating the frontier.

0 commit comments

Comments
 (0)