From 3e81b8b6a8d6160fe3ba0fa0f78af66babc36aad Mon Sep 17 00:00:00 2001
From: Moritz Hoffmann <antiguru@gmail.com>
Date: Tue, 23 Apr 2024 10:44:43 -0400
Subject: [PATCH 1/5] Allow joins to produce arbitrary containers (#477)

* Allow joins to produce arbitrary containers

Add a `join_traces_core` function that allows the caller to specify the
output stream container type. The existing `join_traces` function forces
it to be vectors, by virtue of wrapping the output in a collection.

Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>

* Remove `join_traces_core`

Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>

---------

Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>
---
 src/operators/arrange/arrangement.rs |  1 +
 src/operators/join.rs                | 24 +++++++++++++++++-------
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/src/operators/arrange/arrangement.rs b/src/operators/arrange/arrangement.rs
index bffc2bdaa..9a5d926c0 100644
--- a/src/operators/arrange/arrangement.rs
+++ b/src/operators/arrange/arrangement.rs
@@ -427,6 +427,7 @@ where
     {
         use crate::operators::join::join_traces;
         join_traces(self, other, result)
+            .as_collection()
     }
 }
 
diff --git a/src/operators/join.rs b/src/operators/join.rs
index a31e682a5..311f06802 100644
--- a/src/operators/join.rs
+++ b/src/operators/join.rs
@@ -5,16 +5,17 @@
 //! + (b * c), and if this is not equal to the former term, little is known about the actual output.
 use std::cmp::Ordering;
 
+use timely::container::{PushContainer, PushInto};
 use timely::order::PartialOrder;
 use timely::progress::Timestamp;
-use timely::dataflow::Scope;
-use timely::dataflow::operators::generic::{Operator, OutputHandle};
+use timely::dataflow::{Scope, StreamCore};
+use timely::dataflow::operators::generic::{Operator, OutputHandleCore};
 use timely::dataflow::channels::pact::Pipeline;
 use timely::dataflow::operators::Capability;
 use timely::dataflow::channels::pushers::tee::Tee;
 
 use crate::hashable::Hashable;
-use crate::{Data, ExchangeData, Collection, AsCollection};
+use crate::{Data, ExchangeData, Collection};
 use crate::difference::{Semigroup, Abelian, Multiply};
 use crate::lattice::Lattice;
 use crate::operators::arrange::{Arranged, ArrangeByKey, ArrangeBySelf};
@@ -322,8 +323,14 @@ where
 /// even potentially unrelated to the input collection data. Importantly, the key and value types could be generic
 /// associated types (GATs) of the traces, and we would seemingly struggle to frame these types as trait arguments.
 ///
+/// The implementation produces a caller-specified container, with the requirement that the container
+/// can absorb `(D, G::Timestamp, R)` triples. Implementations can use [`AsCollection`] to wrap the
+/// output stream in a collection.
+///
 /// The "correctness" of this method depends heavily on the behavior of the supplied `result` function.
-pub fn join_traces<G, T1, T2, I,L,D,R>(arranged1: &Arranged<G,T1>, arranged2: &Arranged<G,T2>, mut result: L) -> Collection<G, D, R>
+///
+/// [`AsCollection`]: crate::collection::AsCollection
+pub fn join_traces<G, T1, T2, I,L,D,R,C>(arranged1: &Arranged<G,T1>, arranged2: &Arranged<G,T2>, mut result: L) -> StreamCore<G, C>
 where
     G: Scope<Timestamp=T1::Time>,
     T1: TraceReader+Clone+'static,
@@ -332,6 +339,8 @@ where
     R: Semigroup,
     I: IntoIterator<Item=(D, G::Timestamp, R)>,
     L: FnMut(T1::Key<'_>,T1::Val<'_>,T2::Val<'_>,&G::Timestamp,&T1::Diff,&T2::Diff)->I+'static,
+    C: PushContainer,
+    (D, G::Timestamp, R): PushInto<C>,
 {
     // Rename traces for symmetry from here on out.
     let mut trace1 = arranged1.trace.clone();
@@ -565,7 +574,6 @@ where
             }
         }
     })
-    .as_collection()
 }
 
 
@@ -617,10 +625,12 @@ where
 
     /// Process keys until at least `fuel` output tuples produced, or the work is exhausted.
     #[inline(never)]
-    fn work<L, I>(&mut self, output: &mut OutputHandle<T, (D, T, R), Tee<T, Vec<(D, T, R)>>>, mut logic: L, fuel: &mut usize)
-    where 
+    fn work<L, I, C>(&mut self, output: &mut OutputHandleCore<T, C, Tee<T, C>>, mut logic: L, fuel: &mut usize)
+    where
         I: IntoIterator<Item=(D, T, R)>,
         L: for<'a> FnMut(C1::Key<'a>, C1::Val<'a>, C2::Val<'a>, &T, &C1::Diff, &C2::Diff)->I,
+        C: PushContainer,
+        (D, T, R): PushInto<C>,
     {
 
         let meet = self.capability.time();

From d18497c41add25471c469c9d0e8fa94e0462c414 Mon Sep 17 00:00:00 2001
From: Frank McSherry <fmcsherry@me.com>
Date: Wed, 24 Apr 2024 15:03:28 -0400
Subject: [PATCH 2/5] Remove `ValOwned` (#476)

* Remove use of ValOwned

* Respond to feedback
---
 Cargo.toml                                |   2 +-
 dogsdogsdogs/examples/delta_query.rs      |   6 +-
 dogsdogsdogs/src/lib.rs                   |   2 +-
 dogsdogsdogs/src/operators/count.rs       |   2 +-
 dogsdogsdogs/src/operators/propose.rs     |  19 ++-
 dogsdogsdogs/src/operators/validate.rs    |   2 +-
 examples/cursors.rs                       |  19 +--
 examples/monoid-bfs.rs                    |   2 +-
 examples/spines.rs                        |   4 +-
 src/algorithms/graphs/propagate.rs        |   2 +-
 src/operators/arrange/agent.rs            |   1 -
 src/operators/arrange/arrangement.rs      | 186 ++--------------------
 src/operators/arrange/upsert.rs           |  20 +--
 src/operators/consolidate.rs              |   2 +-
 src/operators/reduce.rs                   |  93 ++++++-----
 src/trace/cursor/cursor_list.rs           |   1 -
 src/trace/cursor/mod.rs                   |  11 +-
 src/trace/implementations/ord_neu.rs      |   4 -
 src/trace/implementations/rhh.rs          |   2 -
 src/trace/implementations/spine_fueled.rs |   1 -
 src/trace/mod.rs                          |  18 +--
 src/trace/wrappers/enter.rs               |   4 -
 src/trace/wrappers/enter_at.rs            |   4 -
 src/trace/wrappers/filter.rs              |   4 -
 src/trace/wrappers/freeze.rs              |   4 -
 src/trace/wrappers/frontier.rs            |   4 -
 src/trace/wrappers/rc.rs                  |   1 -
 tests/trace.rs                            |   8 +-
 28 files changed, 115 insertions(+), 313 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index dc9ab0c0e..0cca0a4bd 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -21,7 +21,7 @@ members = [
     # "advent_of_code_2017",
     "dogsdogsdogs",
     "experiments",
-    "interactive",
+    #"interactive",
     "server",
     "server/dataflows/degr_dist",
     "server/dataflows/neighborhood",
diff --git a/dogsdogsdogs/examples/delta_query.rs b/dogsdogsdogs/examples/delta_query.rs
index 2f2a9ba10..dab7cc655 100644
--- a/dogsdogsdogs/examples/delta_query.rs
+++ b/dogsdogsdogs/examples/delta_query.rs
@@ -79,17 +79,17 @@ fn main() {
 
                 // Prior technology
                 //   dQ/dE1 := dE1(a,b), E2(b,c), E3(a,c)
-                let changes1 = propose(&changes, forward_key_neu.clone(), key2.clone());
+                let changes1 = propose(&changes, forward_key_neu.clone(), key2.clone(), Clone::clone);
                 let changes1 = validate(&changes1, forward_self_neu.clone(), key1.clone());
                 let changes1 = changes1.map(|((a,b),c)| (a,b,c));
 
                 //   dQ/dE2 := dE2(b,c), E1(a,b), E3(a,c)
-                let changes2 = propose(&changes, reverse_key_alt.clone(), key1.clone());
+                let changes2 = propose(&changes, reverse_key_alt.clone(), key1.clone(), Clone::clone);
                 let changes2 = validate(&changes2, reverse_self_neu.clone(), key2.clone());
                 let changes2 = changes2.map(|((b,c),a)| (a,b,c));
 
                 //   dQ/dE3 := dE3(a,c), E1(a,b), E2(b,c)
-                let changes3 = propose(&changes, forward_key_alt.clone(), key1.clone());
+                let changes3 = propose(&changes, forward_key_alt.clone(), key1.clone(), Clone::clone);
                 let changes3 = validate(&changes3, reverse_self_alt.clone(), key2.clone());
                 let changes3 = changes3.map(|((a,c),b)| (a,b,c));
 
diff --git a/dogsdogsdogs/src/lib.rs b/dogsdogsdogs/src/lib.rs
index 262e09609..1d90feb51 100644
--- a/dogsdogsdogs/src/lib.rs
+++ b/dogsdogsdogs/src/lib.rs
@@ -199,7 +199,7 @@ where
 
     fn propose(&mut self, prefixes: &Collection<G, P, R>) -> Collection<G, (P, V), R> {
         let propose = self.indices.propose_trace.import(&prefixes.scope());
-        operators::propose::propose(prefixes, propose, self.key_selector.clone())
+        operators::propose::propose(prefixes, propose, self.key_selector.clone(), |x| x.clone())
     }
 
     fn validate(&mut self, extensions: &Collection<G, (P, V), R>) -> Collection<G, (P, V), R> {
diff --git a/dogsdogsdogs/src/operators/count.rs b/dogsdogsdogs/src/operators/count.rs
index 68c39f1fa..d77c0b7e4 100644
--- a/dogsdogsdogs/src/operators/count.rs
+++ b/dogsdogsdogs/src/operators/count.rs
@@ -19,7 +19,7 @@ pub fn count<G, Tr, R, F, P>(
 ) -> Collection<G, (P, usize, usize), R>
 where
     G: Scope<Timestamp=Tr::Time>,
-    Tr: TraceReader<ValOwned=(), Diff=isize>+Clone+'static,
+    Tr: TraceReader<Diff=isize>+Clone+'static,
     Tr::KeyOwned: Hashable + Default,
     R: Monoid+Multiply<Output = R>+ExchangeData,
     F: Fn(&P)->Tr::KeyOwned+Clone+'static,
diff --git a/dogsdogsdogs/src/operators/propose.rs b/dogsdogsdogs/src/operators/propose.rs
index 164e42987..36024e6d4 100644
--- a/dogsdogsdogs/src/operators/propose.rs
+++ b/dogsdogsdogs/src/operators/propose.rs
@@ -4,7 +4,6 @@ use differential_dataflow::{ExchangeData, Collection, Hashable};
 use differential_dataflow::difference::{Monoid, Multiply};
 use differential_dataflow::operators::arrange::Arranged;
 use differential_dataflow::trace::TraceReader;
-use differential_dataflow::trace::cursor::MyTrait;
 
 /// Proposes extensions to a prefix stream.
 ///
@@ -14,11 +13,12 @@ use differential_dataflow::trace::cursor::MyTrait;
 /// create a join if the `prefixes` collection is also arranged and responds to changes that
 /// `arrangement` undergoes. More complicated patterns are also appropriate, as in the case
 /// of delta queries.
-pub fn propose<G, Tr, F, P>(
+pub fn propose<G, Tr, F, P, V, VF>(
     prefixes: &Collection<G, P, Tr::Diff>,
     arrangement: Arranged<G, Tr>,
     key_selector: F,
-) -> Collection<G, (P, Tr::ValOwned), Tr::Diff>
+    val_from: VF,
+) -> Collection<G, (P, V), Tr::Diff>
 where
     G: Scope<Timestamp=Tr::Time>,
     Tr: TraceReader+Clone+'static,
@@ -26,12 +26,14 @@ where
     Tr::Diff: Monoid+Multiply<Output = Tr::Diff>+ExchangeData,
     F: Fn(&P)->Tr::KeyOwned+Clone+'static,
     P: ExchangeData,
+    V: Clone + 'static,
+    VF: Fn(Tr::Val<'_>) -> V + 'static,
 {
     crate::operators::lookup_map(
         prefixes,
         arrangement,
         move |p: &P, k: &mut Tr::KeyOwned | { *k = key_selector(p); },
-        |prefix, diff, value, sum| ((prefix.clone(), value.into_owned()), diff.clone().multiply(sum)),
+        move |prefix, diff, value, sum| ((prefix.clone(), val_from(value)), diff.clone().multiply(sum)),
         Default::default(),
         Default::default(),
         Default::default(),
@@ -43,11 +45,12 @@ where
 /// Unlike `propose`, this method does not scale the multiplicity of matched
 /// prefixes by the number of matches in `arrangement`. This can be useful to
 /// avoid the need to prepare an arrangement of distinct extensions.
-pub fn propose_distinct<G, Tr, F, P>(
+pub fn propose_distinct<G, Tr, F, P, V, VF>(
     prefixes: &Collection<G, P, Tr::Diff>,
     arrangement: Arranged<G, Tr>,
     key_selector: F,
-) -> Collection<G, (P, Tr::ValOwned), Tr::Diff>
+    val_from: VF,
+) -> Collection<G, (P, V), Tr::Diff>
 where
     G: Scope<Timestamp=Tr::Time>,
     Tr: TraceReader+Clone+'static,
@@ -55,12 +58,14 @@ where
     Tr::Diff: Monoid+Multiply<Output = Tr::Diff>+ExchangeData,
     F: Fn(&P)->Tr::KeyOwned+Clone+'static,
     P: ExchangeData,
+    V: Clone + 'static,
+    VF: Fn(Tr::Val<'_>) -> V + 'static,
 {
     crate::operators::lookup_map(
         prefixes,
         arrangement,
         move |p: &P, k: &mut Tr::KeyOwned| { *k = key_selector(p); },
-        |prefix, diff, value, _sum| ((prefix.clone(), value.into_owned()), diff.clone()),
+        move |prefix, diff, value, _sum| ((prefix.clone(), val_from(value)), diff.clone()),
         Default::default(),
         Default::default(),
         Default::default(),
diff --git a/dogsdogsdogs/src/operators/validate.rs b/dogsdogsdogs/src/operators/validate.rs
index f1f0c273f..7ddbf38c5 100644
--- a/dogsdogsdogs/src/operators/validate.rs
+++ b/dogsdogsdogs/src/operators/validate.rs
@@ -19,7 +19,7 @@ pub fn validate<G, K, V, Tr, F, P>(
 ) -> Collection<G, (P, V), Tr::Diff>
 where
     G: Scope<Timestamp=Tr::Time>,
-    Tr: TraceReader<KeyOwned=(K,V), ValOwned=()>+Clone+'static,
+    Tr: TraceReader<KeyOwned=(K,V)>+Clone+'static,
     K: Ord+Hash+Clone+Default,
     V: ExchangeData+Hash+Default,
     Tr::Diff: Monoid+Multiply<Output = Tr::Diff>+ExchangeData,
diff --git a/examples/cursors.rs b/examples/cursors.rs
index a9632570c..e4da79e47 100644
--- a/examples/cursors.rs
+++ b/examples/cursors.rs
@@ -31,7 +31,6 @@
 //! Final graph: {(2, 1): 1, (3, 2): 1, (3, 4): 1, (4, 3): 1}
 //! ```
 
-use std::fmt::Debug;
 use std::collections::BTreeMap;
 
 use timely::dataflow::operators::probe::Handle;
@@ -77,7 +76,6 @@ fn main() {
                 graph_trace.set_physical_compaction(AntichainRef::new(&[i]));
                 graph_trace.set_logical_compaction(AntichainRef::new(&[i]));
                 worker.step_while(|| probe.less_than(&i));
-                dump_cursor(i, worker.index(), &mut graph_trace);
             }
         } else {
             /* Only worker 0 feeds inputs to the dataflow. */
@@ -92,13 +90,12 @@ fn main() {
                 graph_trace.set_physical_compaction(AntichainRef::new(&[i]));
                 graph_trace.set_logical_compaction(AntichainRef::new(&[i]));
                 worker.step_while(|| probe.less_than(graph.time()));
-                dump_cursor(i, worker.index(), &mut graph_trace);
             }
         }
 
         /* Return trace content after the last round. */
         let (mut cursor, storage) = graph_trace.cursor();
-        cursor.to_vec(&storage)
+        cursor.to_vec(Clone::clone, &storage)
     })
     .unwrap().join();
 
@@ -128,17 +125,3 @@ fn main() {
     expected_graph_content.insert((rounds, rounds+1), 1);
     assert_eq!(graph_content, expected_graph_content);
 }
-
-fn dump_cursor<Tr>(round: u32, index: usize, trace: &mut Tr)
-where
-    Tr: TraceReader,
-    Tr::KeyOwned: Debug,
-    Tr::ValOwned: Debug,
-    Tr::Time: Debug,
-    Tr::Diff: Debug,
-{
-    let (mut cursor, storage) = trace.cursor();
-    for ((k, v), diffs) in cursor.to_vec(&storage).iter() {
-        println!("round {}, w{} {:?}:{:?}: {:?}", round, index, *k, *v, diffs);
-    }
-}
diff --git a/examples/monoid-bfs.rs b/examples/monoid-bfs.rs
index f9835cef7..47563a065 100644
--- a/examples/monoid-bfs.rs
+++ b/examples/monoid-bfs.rs
@@ -145,7 +145,7 @@ where G::Timestamp: Lattice+Ord {
             .join_map(&edges, |_k,&(),d| *d)
             .concat(&roots)
             .map(|x| (x,()))
-            .reduce_core::<_,KeySpine<_,_,_>>("Reduce", |_key, input, output, updates| {
+            .reduce_core::<_,_,KeySpine<_,_,_>>("Reduce", Clone::clone, |_key, input, output, updates| {
                 if output.is_empty() || input[0].1 < output[0].1 {
                     updates.push(((), input[0].1));
                 }
diff --git a/examples/spines.rs b/examples/spines.rs
index 0dba1630f..d4d2e64bc 100644
--- a/examples/spines.rs
+++ b/examples/spines.rs
@@ -55,11 +55,11 @@ fn main() {
                     let data =
                     data.map(|x| (x.clone().into_bytes(), x.into_bytes()))
                         .arrange::<PreferredSpine<[u8],[u8],_,_>>()
-                        .reduce_abelian::<_, PreferredSpine<[u8],(),_,_>>("distinct", |_,_,output| output.push(((), 1)));
+                        .reduce_abelian::<_, _, _, PreferredSpine<[u8],(),_,_>>("distinct", |v| v.clone(), |_,_,output| output.push(((), 1)));
                     let keys =
                     keys.map(|x| (x.clone().into_bytes(), 7))
                         .arrange::<PreferredSpine<[u8],u8,_,_>>()
-                        .reduce_abelian::<_, PreferredSpine<[u8],(),_,_>>("distinct", |_,_,output| output.push(((), 1)));
+                        .reduce_abelian::<_, _, _, PreferredSpine<[u8],(),_,_>>("distinct", |v| v.clone(), |_,_,output| output.push(((), 1)));
 
                     keys.join_core(&data, |k,_v1,_v2| {
                         println!("{:?}", k.text);
diff --git a/src/algorithms/graphs/propagate.rs b/src/algorithms/graphs/propagate.rs
index 9aef8e08b..b73ff598b 100644
--- a/src/algorithms/graphs/propagate.rs
+++ b/src/algorithms/graphs/propagate.rs
@@ -96,7 +96,7 @@ where
         let labels =
         proposals
             .concat(&nodes)
-            .reduce_abelian::<_,ValSpine<_,_,_,_>>("Propagate", |_, s, t| t.push((s[0].0.clone(), R::from(1_i8))));
+            .reduce_abelian::<_,_,ValSpine<_,_,_,_>>("Propagate", |v| v.clone(), |_, s, t| t.push((s[0].0.clone(), R::from(1_i8))));
 
         let propagate: Collection<_, (N, L), R> =
         labels
diff --git a/src/operators/arrange/agent.rs b/src/operators/arrange/agent.rs
index c18b7e30e..012882e2b 100644
--- a/src/operators/arrange/agent.rs
+++ b/src/operators/arrange/agent.rs
@@ -46,7 +46,6 @@ where
     type Key<'a> = Tr::Key<'a>;
     type KeyOwned = Tr::KeyOwned;
     type Val<'a> = Tr::Val<'a>;
-    type ValOwned = Tr::ValOwned;
     type Time = Tr::Time;
     type Diff = Tr::Diff;
 
diff --git a/src/operators/arrange/arrangement.rs b/src/operators/arrange/arrangement.rs
index 9a5d926c0..c22749a27 100644
--- a/src/operators/arrange/arrangement.rs
+++ b/src/operators/arrange/arrangement.rs
@@ -23,7 +23,7 @@ use timely::dataflow::{Scope, Stream, StreamCore};
 use timely::dataflow::operators::generic::Operator;
 use timely::dataflow::channels::pact::{ParallelizationContract, Pipeline, Exchange};
 use timely::progress::Timestamp;
-use timely::progress::{Antichain, frontier::AntichainRef};
+use timely::progress::Antichain;
 use timely::dataflow::operators::Capability;
 
 use crate::{Data, ExchangeData, Collection, AsCollection, Hashable};
@@ -37,8 +37,6 @@ use trace::wrappers::enter_at::TraceEnter as TraceEnterAt;
 use trace::wrappers::enter_at::BatchEnter as BatchEnterAt;
 use trace::wrappers::filter::{TraceFilter, BatchFilter};
 
-use trace::cursor::MyTrait;
-
 use super::TraceAgent;
 
 /// An arranged collection of `(K,V)` values.
@@ -229,166 +227,6 @@ where
         })
         .as_collection()
     }
-
-    /// Report values associated with keys at certain times.
-    ///
-    /// This method consumes a stream of (key, time) queries and reports the corresponding stream of
-    /// (key, value, time, diff) accumulations in the `self` trace.
-    pub fn lookup(&self, queries: &Stream<G, (Tr::KeyOwned, G::Timestamp)>) -> Stream<G, (Tr::KeyOwned, Tr::ValOwned, G::Timestamp, Tr::Diff)>
-    where
-        Tr::KeyOwned: ExchangeData+Hashable,
-        Tr::ValOwned: ExchangeData,
-        Tr::Diff: ExchangeData,
-        Tr: 'static,
-    {
-        // while the arrangement is already correctly distributed, the query stream may not be.
-        let exchange = Exchange::new(move |update: &(Tr::KeyOwned,G::Timestamp)| update.0.hashed().into());
-        queries.binary_frontier(&self.stream, exchange, Pipeline, "TraceQuery", move |_capability, _info| {
-
-            let mut trace = Some(self.trace.clone());
-            // release `set_physical_compaction` capability.
-            trace.as_mut().unwrap().set_physical_compaction(Antichain::new().borrow());
-
-            let mut stash = Vec::new();
-            let mut capability: Option<Capability<G::Timestamp>> = None;
-
-            let mut active = Vec::new();
-            let mut retain = Vec::new();
-
-            let mut working: Vec<(G::Timestamp, Tr::ValOwned, Tr::Diff)> = Vec::new();
-            let mut working2: Vec<(Tr::ValOwned, Tr::Diff)> = Vec::new();
-
-            move |input1, input2, output| {
-
-                input1.for_each(|time, data| {
-                    // if the minimum capability "improves" retain it.
-                    if capability.is_none() || time.time().less_than(capability.as_ref().unwrap().time()) {
-                        capability = Some(time.retain());
-                    }
-                    stash.extend(data.iter().cloned());
-                });
-
-                // drain input2; we will consult `trace` directly.
-                input2.for_each(|_time, _data| { });
-
-                assert_eq!(capability.is_none(), stash.is_empty());
-
-                let mut drained = false;
-                if let Some(capability) = capability.as_mut() {
-                    if !input2.frontier().less_equal(capability.time()) {
-                        for datum in stash.drain(..) {
-                            if !input2.frontier().less_equal(&datum.1) {
-                                active.push(datum);
-                            }
-                            else {
-                                retain.push(datum);
-                            }
-                        }
-                        drained = !active.is_empty();
-
-                        ::std::mem::swap(&mut stash, &mut retain);    // retain now the stashed queries.
-
-                        // sort temp1 by key and then by time.
-                        active.sort_unstable_by(|x,y| x.0.cmp(&y.0));
-
-                        let (mut cursor, storage) = trace.as_mut().unwrap().cursor();
-                        let mut session = output.session(&capability);
-
-                        // // V0: Potentially quadratic under load.
-                        // for (key, time) in active.drain(..) {
-                        //     cursor.seek_key(&storage, &key);
-                        //     if cursor.get_key(&storage) == Some(&key) {
-                        //         while let Some(val) = cursor.get_val(&storage) {
-                        //             let mut count = R::zero();
-                        //             cursor.map_times(&storage, |t, d| if t.less_equal(&time) {
-                        //                 count = count + d;
-                        //             });
-                        //             if !count.is_zero() {
-                        //                 session.give((key.clone(), val.clone(), time.clone(), count));
-                        //             }
-                        //             cursor.step_val(&storage);
-                        //         }
-                        //     }
-                        // }
-
-                        // V1: Stable under load
-                        let mut active_finger = 0;
-                        while active_finger < active.len() {
-
-                            let key = &active[active_finger].0;
-                            let mut same_key = active_finger;
-                            while active.get(same_key).map(|x| &x.0) == Some(key) {
-                                same_key += 1;
-                            }
-
-                            cursor.seek_key_owned(&storage, key);
-                            if cursor.get_key(&storage).map(|k| k.equals(key)).unwrap_or(false) {
-
-                                let mut active = &active[active_finger .. same_key];
-
-                                while let Some(val) = cursor.get_val(&storage) {
-                                    cursor.map_times(&storage, |t,d| working.push((t.clone(), val.into_owned(), d.clone())));
-                                    cursor.step_val(&storage);
-                                }
-
-                                working.sort_by(|x,y| x.0.cmp(&y.0));
-                                for (time, val, diff) in working.drain(..) {
-                                    if !active.is_empty() && active[0].1.less_than(&time) {
-                                        crate::consolidation::consolidate(&mut working2);
-                                        while !active.is_empty() && active[0].1.less_than(&time) {
-                                            for (val, count) in working2.iter() {
-                                                session.give((key.clone(), val.clone(), active[0].1.clone(), count.clone()));
-                                            }
-                                            active = &active[1..];
-                                        }
-                                    }
-                                    working2.push((val, diff));
-                                }
-                                if !active.is_empty() {
-                                    crate::consolidation::consolidate(&mut working2);
-                                    while !active.is_empty() {
-                                        for (val, count) in working2.iter() {
-                                            session.give((key.clone(), val.clone(), active[0].1.clone(), count.clone()));
-                                        }
-                                        active = &active[1..];
-                                    }
-                                }
-                            }
-                            active_finger = same_key;
-                        }
-                        active.clear();
-                    }
-                }
-
-                if drained {
-                    if stash.is_empty() { capability = None; }
-                    if let Some(capability) = capability.as_mut() {
-                        let mut min_time = stash[0].1.clone();
-                        for datum in stash[1..].iter() {
-                            if datum.1.less_than(&min_time) {
-                                min_time = datum.1.clone();
-                            }
-                        }
-                        capability.downgrade(&min_time);
-                    }
-                }
-
-                // Determine new frontier on queries that may be issued.
-                // TODO: This code looks very suspect; explain better or fix.
-                let frontier = IntoIterator::into_iter([
-                    capability.as_ref().map(|c| c.time().clone()),
-                    input1.frontier().frontier().get(0).cloned(),
-                ]).flatten().min();
-
-                if let Some(frontier) = frontier {
-                    trace.as_mut().map(|t| t.set_logical_compaction(AntichainRef::new(&[frontier])));
-                }
-                else {
-                    trace = None;
-                }
-            }
-        })
-    }
 }
 
 
@@ -439,16 +277,17 @@ where
     T1: TraceReader + Clone + 'static,
 {
     /// A direct implementation of `ReduceCore::reduce_abelian`.
-    pub fn reduce_abelian<L, T2>(&self, name: &str, mut logic: L) -> Arranged<G, TraceAgent<T2>>
+    pub fn reduce_abelian<L, V, F, T2>(&self, name: &str, from: F, mut logic: L) -> Arranged<G, TraceAgent<T2>>
     where
         T2: for<'a> Trace<Key<'a>= T1::Key<'a>, Time=T1::Time>+'static,
-        T2::ValOwned: Data,
+        V: Data,
+        F: Fn(T2::Val<'_>) -> V + 'static,
         T2::Diff: Abelian,
         T2::Batch: Batch,
-        T2::Builder: Builder<Input = ((T1::KeyOwned, T2::ValOwned), T2::Time, T2::Diff)>,
-        L: FnMut(T1::Key<'_>, &[(T1::Val<'_>, T1::Diff)], &mut Vec<(<T2::Cursor as Cursor>::ValOwned, T2::Diff)>)+'static,
+        T2::Builder: Builder<Input = ((T1::KeyOwned, V), T2::Time, T2::Diff)>,
+        L: FnMut(T1::Key<'_>, &[(T1::Val<'_>, T1::Diff)], &mut Vec<(V, T2::Diff)>)+'static,
     {
-        self.reduce_core::<_,T2>(name, move |key, input, output, change| {
+        self.reduce_core::<_,V,F,T2>(name, from, move |key, input, output, change| {
             if !input.is_empty() {
                 logic(key, input, change);
             }
@@ -458,16 +297,17 @@ where
     }
 
     /// A direct implementation of `ReduceCore::reduce_core`.
-    pub fn reduce_core<L, T2>(&self, name: &str, logic: L) -> Arranged<G, TraceAgent<T2>>
+    pub fn reduce_core<L, V, F, T2>(&self, name: &str, from: F, logic: L) -> Arranged<G, TraceAgent<T2>>
     where
         T2: for<'a> Trace<Key<'a>=T1::Key<'a>, Time=T1::Time>+'static,
-        T2::ValOwned: Data,
+        V: Data,
+        F: Fn(T2::Val<'_>) -> V + 'static,
         T2::Batch: Batch,
-        T2::Builder: Builder<Input = ((T1::KeyOwned, T2::ValOwned), T2::Time, T2::Diff)>,
-        L: FnMut(T1::Key<'_>, &[(T1::Val<'_>, T1::Diff)], &mut Vec<(<T2::Cursor as Cursor>::ValOwned,T2::Diff)>, &mut Vec<(<T2::Cursor as Cursor>::ValOwned, T2::Diff)>)+'static,
+        T2::Builder: Builder<Input = ((T1::KeyOwned,V), T2::Time, T2::Diff)>,
+        L: FnMut(T1::Key<'_>, &[(T1::Val<'_>, T1::Diff)], &mut Vec<(V, T2::Diff)>, &mut Vec<(V, T2::Diff)>)+'static,
     {
         use crate::operators::reduce::reduce_trace;
-        reduce_trace(self, name, logic)
+        reduce_trace::<_,_,_,V,_,_>(self, name, from, logic)
     }
 }
 
diff --git a/src/operators/arrange/upsert.rs b/src/operators/arrange/upsert.rs
index 4ca9dc9b3..758ec8df3 100644
--- a/src/operators/arrange/upsert.rs
+++ b/src/operators/arrange/upsert.rs
@@ -59,7 +59,7 @@
 //!         use differential_dataflow::operators::arrange::upsert;
 //!
 //!         let stream = scope.input_from(&mut input);
-//!         let arranged = upsert::arrange_from_upsert::<_, ValSpine<Key, Val, _, _>>(&stream, &"test");
+//!         let arranged = upsert::arrange_from_upsert::<_, _, _, ValSpine<Key, Val, _, _>>(&stream, &"test", |v| v.clone());
 //!
 //!         arranged
 //!             .as_collection(|k,v| (k.clone(), v.clone()))
@@ -125,18 +125,20 @@ use super::TraceAgent;
 /// This method is only implemented for totally ordered times, as we do not yet
 /// understand what a "sequence" of upserts would mean for partially ordered
 /// timestamps.
-pub fn arrange_from_upsert<G, Tr>(
-    stream: &Stream<G, (Tr::KeyOwned, Option<Tr::ValOwned>, G::Timestamp)>,
+pub fn arrange_from_upsert<G, V, F, Tr>(
+    stream: &Stream<G, (Tr::KeyOwned, Option<V>, G::Timestamp)>,
     name: &str,
+    from: F,
 ) -> Arranged<G, TraceAgent<Tr>>
 where
     G: Scope<Timestamp=Tr::Time>,
     Tr: Trace+TraceReader<Diff=isize>+'static,
     Tr::KeyOwned: ExchangeData+Hashable+std::hash::Hash,
-    Tr::ValOwned: ExchangeData,
+    V: ExchangeData,
+    F: Fn(Tr::Val<'_>) -> V + 'static,
     Tr::Time: TotalOrder+ExchangeData,
     Tr::Batch: Batch,
-    Tr::Builder: Builder<Input = ((Tr::KeyOwned, Tr::ValOwned), Tr::Time, Tr::Diff)>,
+    Tr::Builder: Builder<Input = ((Tr::KeyOwned, V), Tr::Time, Tr::Diff)>,
 {
     let mut reader: Option<TraceAgent<Tr>> = None;
 
@@ -145,7 +147,7 @@ where
 
         let reader = &mut reader;
 
-        let exchange = Exchange::new(move |update: &(Tr::KeyOwned,Option<Tr::ValOwned>,G::Timestamp)| (update.0).hashed().into());
+        let exchange = Exchange::new(move |update: &(Tr::KeyOwned,Option<V>,G::Timestamp)| (update.0).hashed().into());
 
         stream.unary_frontier(exchange, name, move |_capability, info| {
 
@@ -175,7 +177,7 @@ where
             let mut prev_frontier = Antichain::from_elem(<G::Timestamp as Timestamp>::minimum());
 
             // For stashing input upserts, ordered increasing by time (`BinaryHeap` is a max-heap).
-            let mut priority_queue = BinaryHeap::<std::cmp::Reverse<(G::Timestamp, Tr::KeyOwned, Option<Tr::ValOwned>)>>::new();
+            let mut priority_queue = BinaryHeap::<std::cmp::Reverse<(G::Timestamp, Tr::KeyOwned, Option<V>)>>::new();
             let mut updates = Vec::new();
 
             move |input, output| {
@@ -245,7 +247,7 @@ where
                                     use trace::cursor::MyTrait;
 
                                     // The prior value associated with the key.
-                                    let mut prev_value: Option<Tr::ValOwned> = None;
+                                    let mut prev_value: Option<V> = None;
 
                                     // Attempt to find the key in the trace.
                                     trace_cursor.seek_key_owned(&trace_storage, &key);
@@ -257,7 +259,7 @@ where
                                             assert!(count == 0 || count == 1);
                                             if count == 1 {
                                                 assert!(prev_value.is_none());
-                                                prev_value = Some(val.into_owned());
+                                                prev_value = Some(from(val));
                                             }
                                             trace_cursor.step_val(&trace_storage);
                                         }
diff --git a/src/operators/consolidate.rs b/src/operators/consolidate.rs
index b67c1c4b8..46b96528c 100644
--- a/src/operators/consolidate.rs
+++ b/src/operators/consolidate.rs
@@ -51,7 +51,7 @@ where
     /// As `consolidate` but with the ability to name the operator and specify the trace type.
     pub fn consolidate_named<Tr>(&self, name: &str) -> Self
     where
-        Tr: crate::trace::Trace<KeyOwned = D,ValOwned = (),Time=G::Timestamp,Diff=R>+'static,
+        Tr: crate::trace::Trace<KeyOwned = D,Time=G::Timestamp,Diff=R>+'static,
         Tr::Batch: crate::trace::Batch,
         Tr::Batcher: Batcher<Input=Vec<((D,()),G::Timestamp,R)>>,
     {
diff --git a/src/operators/reduce.rs b/src/operators/reduce.rs
index 37d98d8ca..db80a5d44 100644
--- a/src/operators/reduce.rs
+++ b/src/operators/reduce.rs
@@ -87,7 +87,7 @@ where
 {
     fn reduce_named<L, V2: Data, R2: Abelian>(&self, name: &str, logic: L) -> Collection<G, (K, V2), R2>
         where L: FnMut(&K, &[(&V, R)], &mut Vec<(V2, R2)>)+'static {
-        self.reduce_abelian::<_,ValSpine<_,_,_,_>>(name, logic)
+        self.reduce_abelian::<_,V2,_,ValSpine<_,_,_,_>>(name, |val| val.clone(), logic)
             .as_collection(|k,v| (k.clone(), v.clone()))
     }
 }
@@ -163,7 +163,7 @@ where
     T1: for<'a> TraceReader<Key<'a>=&'a K, KeyOwned=K, Val<'a>=&'a (), Diff=R1>+Clone+'static,
 {
     fn threshold_named<R2: Abelian, F: FnMut(&K,&R1)->R2+'static>(&self, name: &str, mut thresh: F) -> Collection<G, K, R2> {
-        self.reduce_abelian::<_,KeySpine<_,_,_>>(name, move |k,s,t| t.push(((), thresh(k, &s[0].1))))
+        self.reduce_abelian::<_,(),_,KeySpine<_,_,_>>(name, |&()| (), move |k,s,t| t.push(((), thresh(k, &s[0].1))))
             .as_collection(|k,_| k.clone())
     }
 }
@@ -213,13 +213,13 @@ where
     T1: for<'a> TraceReader<Key<'a>=&'a K, KeyOwned=K, Val<'a>=&'a (), Diff=R>+Clone+'static,
 {
     fn count_core<R2: Abelian + From<i8>>(&self) -> Collection<G, (K, R), R2> {
-        self.reduce_abelian::<_,ValSpine<_,_,_,_>>("Count", |_k,s,t| t.push((s[0].1.clone(), R2::from(1i8))))
+        self.reduce_abelian::<_,R,_,ValSpine<_,_,_,_>>("Count", |r| r.clone(), |_k,s,t| t.push((s[0].1.clone(), R2::from(1i8))))
             .as_collection(|k,c| (k.clone(), c.clone()))
     }
 }
 
 /// Extension trait for the `reduce_core` differential dataflow method.
-pub trait ReduceCore<G: Scope, K: ToOwned + ?Sized, V: ToOwned + ?Sized, R: Semigroup> where G::Timestamp: Lattice+Ord {
+pub trait ReduceCore<G: Scope, K: ToOwned + ?Sized, V: Data, R: Semigroup> where G::Timestamp: Lattice+Ord {
     /// Applies `reduce` to arranged data, and returns an arrangement of output data.
     ///
     /// This method is used by the more ergonomic `reduce`, `distinct`, and `count` methods, although
@@ -238,23 +238,24 @@ pub trait ReduceCore<G: Scope, K: ToOwned + ?Sized, V: ToOwned + ?Sized, R: Semi
     ///     let trace =
     ///     scope.new_collection_from(1 .. 10u32).1
     ///          .map(|x| (x, x))
-    ///          .reduce_abelian::<_,ValSpine<_,_,_,_>>(
+    ///          .reduce_abelian::<_,_,ValSpine<_,_,_,_>>(
     ///             "Example",
+    ///              Clone::clone, 
     ///              move |_key, src, dst| dst.push((*src[0].0, 1))
     ///          )
     ///          .trace;
     /// });
     /// ```
-    fn reduce_abelian<L, T2>(&self, name: &str, mut logic: L) -> Arranged<G, TraceAgent<T2>>
+    fn reduce_abelian<L, F, T2>(&self, name: &str, from: F, mut logic: L) -> Arranged<G, TraceAgent<T2>>
         where
             T2: for<'a> Trace<Key<'a>= &'a K, Time=G::Timestamp>+'static,
-            T2::ValOwned: Data,
+            F: Fn(T2::Val<'_>) -> V + 'static,
             T2::Diff: Abelian,
             T2::Batch: Batch,
-            T2::Builder: Builder<Input = ((K::Owned, T2::ValOwned), T2::Time, T2::Diff)>,
-            L: FnMut(&K, &[(&V, R)], &mut Vec<(<T2::Cursor as Cursor>::ValOwned, T2::Diff)>)+'static,
+            T2::Builder: Builder<Input = ((K::Owned, V), T2::Time, T2::Diff)>,
+            L: FnMut(&K, &[(&V, R)], &mut Vec<(V, T2::Diff)>)+'static,
         {
-            self.reduce_core::<_,T2>(name, move |key, input, output, change| {
+            self.reduce_core::<_,_,T2>(name, from, move |key, input, output, change| {
                 if !input.is_empty() {
                     logic(key, input, change);
                 }
@@ -268,13 +269,13 @@ pub trait ReduceCore<G: Scope, K: ToOwned + ?Sized, V: ToOwned + ?Sized, R: Semi
     /// Unlike `reduce_arranged`, this method may be called with an empty `input`,
     /// and it may not be safe to index into the first element.
     /// At least one of the two collections will be non-empty.
-    fn reduce_core<L, T2>(&self, name: &str, logic: L) -> Arranged<G, TraceAgent<T2>>
+    fn reduce_core<L, F, T2>(&self, name: &str, from: F, logic: L) -> Arranged<G, TraceAgent<T2>>
         where
             T2: for<'a> Trace<Key<'a>=&'a K, Time=G::Timestamp>+'static,
-            T2::ValOwned: Data,
+            F: Fn(T2::Val<'_>) -> V + 'static,
             T2::Batch: Batch,
-            T2::Builder: Builder<Input = ((K::Owned, T2::ValOwned), T2::Time, T2::Diff)>,
-            L: FnMut(&K, &[(&V, R)], &mut Vec<(<T2::Cursor as Cursor>::ValOwned,T2::Diff)>, &mut Vec<(<T2::Cursor as Cursor>::ValOwned, T2::Diff)>)+'static,
+            T2::Builder: Builder<Input = ((K::Owned, V), T2::Time, T2::Diff)>,
+            L: FnMut(&K, &[(&V, R)], &mut Vec<(V,T2::Diff)>, &mut Vec<(V, T2::Diff)>)+'static,
             ;
 }
 
@@ -283,35 +284,36 @@ where
     G: Scope,
     G::Timestamp: Lattice+Ord,
     K: ExchangeData+Hashable,
-    K: ToOwned<Owned = K>,
     V: ExchangeData,
     R: ExchangeData+Semigroup,
 {
-    fn reduce_core<L, T2>(&self, name: &str, logic: L) -> Arranged<G, TraceAgent<T2>>
+    fn reduce_core<L, F, T2>(&self, name: &str, from: F, logic: L) -> Arranged<G, TraceAgent<T2>>
         where
-            T2::ValOwned: Data,
+            V: Data,
+            F: Fn(T2::Val<'_>) -> V + 'static,
             T2: for<'a> Trace<Key<'a>=&'a K, Time=G::Timestamp>+'static,
             T2::Batch: Batch,
-            T2::Builder: Builder<Input = ((K::Owned, T2::ValOwned), T2::Time, T2::Diff)>,
-            L: FnMut(&K, &[(&V, R)], &mut Vec<(<T2::Cursor as Cursor>::ValOwned,T2::Diff)>, &mut Vec<(<T2::Cursor as Cursor>::ValOwned, T2::Diff)>)+'static,
+            T2::Builder: Builder<Input = ((K, V), T2::Time, T2::Diff)>,
+            L: FnMut(&K, &[(&V, R)], &mut Vec<(V,T2::Diff)>, &mut Vec<(V, T2::Diff)>)+'static,
     {
         self.arrange_by_key_named(&format!("Arrange: {}", name))
-            .reduce_core(name, logic)
+            .reduce_core(name, from, logic)
     }
 }
 
 /// A key-wise reduction of values in an input trace.
 ///
 /// This method exists to provide reduce functionality without opinions about qualifying trace types.
-pub fn reduce_trace<G, T1, T2, L>(trace: &Arranged<G, T1>, name: &str, mut logic: L) -> Arranged<G, TraceAgent<T2>>
+pub fn reduce_trace<G, T1, T2, V, F, L>(trace: &Arranged<G, T1>, name: &str, from: F, mut logic: L) -> Arranged<G, TraceAgent<T2>>
 where
     G: Scope<Timestamp=T1::Time>,
     T1: TraceReader + Clone + 'static,
     T2: for<'a> Trace<Key<'a>=T1::Key<'a>, Time=T1::Time> + 'static,
-    T2::ValOwned: Data,
+    V: Data,
+    F: Fn(T2::Val<'_>) -> V + 'static,
     T2::Batch: Batch,
-    T2::Builder: Builder<Input = ((T1::KeyOwned, T2::ValOwned), T2::Time, T2::Diff)>,
-    L: FnMut(T1::Key<'_>, &[(T1::Val<'_>, T1::Diff)], &mut Vec<(T2::ValOwned,T2::Diff)>, &mut Vec<(T2::ValOwned, T2::Diff)>)+'static,
+    T2::Builder: Builder<Input = ((T1::KeyOwned, V), T2::Time, T2::Diff)>,
+    L: FnMut(T1::Key<'_>, &[(T1::Val<'_>, T1::Diff)], &mut Vec<(V,T2::Diff)>, &mut Vec<(V, T2::Diff)>)+'static,
 {
     let mut result_trace = None;
 
@@ -445,7 +447,7 @@ where
                         //
                         // TODO: It would be better if all updates went into one batch, but timely dataflow prevents
                         //       this as long as it requires that there is only one capability for each message.
-                        let mut buffers = Vec::<(G::Timestamp, Vec<(<T2::Cursor as Cursor>::ValOwned, G::Timestamp, T2::Diff)>)>::new();
+                        let mut buffers = Vec::<(G::Timestamp, Vec<(V, G::Timestamp, T2::Diff)>)>::new();
                         let mut builders = Vec::new();
                         for cap in capabilities.iter() {
                             buffers.push((cap.time().clone(), Vec::new()));
@@ -472,7 +474,7 @@ where
 
                             use std::borrow::Borrow;
                             use crate::trace::cursor::MyTrait;
-                            
+
                             // Determine the next key we will work on; could be synthetic, could be from a batch.
                             let key1 = exposed.get(exposed_position).map(|x| <_ as MyTrait>::borrow_as(&x.0));
                             let key2 = batch_cursor.get_key(batch_storage);
@@ -505,6 +507,7 @@ where
                                 (&mut output_cursor, output_storage),
                                 (&mut batch_cursor, batch_storage),
                                 &mut interesting_times,
+                                &from,
                                 &mut logic,
                                 &upper_limit,
                                 &mut buffers[..],
@@ -622,30 +625,33 @@ fn sort_dedup<T: Ord>(list: &mut Vec<T>) {
     list.dedup();
 }
 
-trait PerKeyCompute<'a, C1, C2, C3>
+trait PerKeyCompute<'a, C1, C2, C3, V>
 where
     C1: Cursor,
     C2: Cursor<Key<'a> = C1::Key<'a>, Time = C1::Time>,
     C3: Cursor<Key<'a> = C1::Key<'a>, Val<'a> = C1::Val<'a>, Time = C1::Time, Diff = C1::Diff>,
+    V: Clone + Ord,
 {
     fn new() -> Self;
-    fn compute<L>(
+    fn compute<F, L>(
         &mut self,
         key: C1::Key<'a>,
         source_cursor: (&mut C1, &'a C1::Storage),
         output_cursor: (&mut C2, &'a C2::Storage),
         batch_cursor: (&mut C3, &'a C3::Storage),
         times: &mut Vec<C1::Time>,
+        from: &F,
         logic: &mut L,
         upper_limit: &Antichain<C1::Time>,
-        outputs: &mut [(C2::Time, Vec<(C2::ValOwned, C2::Time, C2::Diff)>)],
+        outputs: &mut [(C2::Time, Vec<(V, C2::Time, C2::Diff)>)],
         new_interesting: &mut Vec<C1::Time>) -> (usize, usize)
     where
+        F: Fn(C2::Val<'_>) -> V,
         L: FnMut(
             C1::Key<'a>, 
             &[(C1::Val<'a>, C1::Diff)],
-            &mut Vec<(C2::ValOwned, C2::Diff)>,
-            &mut Vec<(C2::ValOwned, C2::Diff)>,
+            &mut Vec<(V, C2::Diff)>,
+            &mut Vec<(V, C2::Diff)>,
         );
 }
 
@@ -664,30 +670,32 @@ mod history_replay {
 
     /// The `HistoryReplayer` is a compute strategy based on moving through existing inputs, interesting times, etc in
     /// time order, maintaining consolidated representations of updates with respect to future interesting times.
-    pub struct HistoryReplayer<'a, C1, C2, C3>
+    pub struct HistoryReplayer<'a, C1, C2, C3, V>
     where
         C1: Cursor,
         C2: Cursor<Key<'a> = C1::Key<'a>, Time = C1::Time>,
         C3: Cursor<Key<'a> = C1::Key<'a>, Val<'a> = C1::Val<'a>, Time = C1::Time, Diff = C1::Diff>,
+        V: Clone + Ord,
     {
         input_history: ValueHistory<'a, C1>,
         output_history: ValueHistory<'a, C2>,
         batch_history: ValueHistory<'a, C3>,
         input_buffer: Vec<(C1::Val<'a>, C1::Diff)>,
-        output_buffer: Vec<(C2::ValOwned, C2::Diff)>,
-        update_buffer: Vec<(C2::ValOwned, C2::Diff)>,
-        output_produced: Vec<((C2::ValOwned, C2::Time), C2::Diff)>,
+        output_buffer: Vec<(V, C2::Diff)>,
+        update_buffer: Vec<(V, C2::Diff)>,
+        output_produced: Vec<((V, C2::Time), C2::Diff)>,
         synth_times: Vec<C1::Time>,
         meets: Vec<C1::Time>,
         times_current: Vec<C1::Time>,
         temporary: Vec<C1::Time>,
     }
 
-    impl<'a, C1, C2, C3> PerKeyCompute<'a, C1, C2, C3> for HistoryReplayer<'a, C1, C2, C3>
+    impl<'a, C1, C2, C3, V> PerKeyCompute<'a, C1, C2, C3, V> for HistoryReplayer<'a, C1, C2, C3, V>
     where
         C1: Cursor,
         C2: Cursor<Key<'a> = C1::Key<'a>, Time = C1::Time>,
         C3: Cursor<Key<'a> = C1::Key<'a>, Val<'a> = C1::Val<'a>, Time = C1::Time, Diff = C1::Diff>,
+        V: Clone + Ord,
     {
         fn new() -> Self {
             HistoryReplayer {
@@ -705,23 +713,25 @@ mod history_replay {
             }
         }
         #[inline(never)]
-        fn compute<L>(
+        fn compute<F, L>(
             &mut self,
             key: C1::Key<'a>,
             (source_cursor, source_storage): (&mut C1, &'a C1::Storage),
             (output_cursor, output_storage): (&mut C2, &'a C2::Storage),
             (batch_cursor, batch_storage): (&mut C3, &'a C3::Storage),
             times: &mut Vec<C1::Time>,
+            from: &F,
             logic: &mut L,
             upper_limit: &Antichain<C1::Time>,
-            outputs: &mut [(C2::Time, Vec<(C2::ValOwned, C2::Time, C2::Diff)>)],
+            outputs: &mut [(C2::Time, Vec<(V, C2::Time, C2::Diff)>)],
             new_interesting: &mut Vec<C1::Time>) -> (usize, usize)
         where
+            F: Fn(C2::Val<'_>) -> V,
             L: FnMut(
                 C1::Key<'a>, 
                 &[(C1::Val<'a>, C1::Diff)],
-                &mut Vec<(C2::ValOwned, C2::Diff)>,
-                &mut Vec<(C2::ValOwned, C2::Diff)>,
+                &mut Vec<(V, C2::Diff)>,
+                &mut Vec<(V, C2::Diff)>,
             )
         {
 
@@ -884,8 +894,7 @@ mod history_replay {
                         meet.as_ref().map(|meet| output_replay.advance_buffer_by(meet));
                         for &((value, ref time), ref diff) in output_replay.buffer().iter() {
                             if time.less_equal(&next_time) {
-                                use crate::trace::cursor::MyTrait;
-                                self.output_buffer.push((<_ as MyTrait>::into_owned(value), diff.clone()));
+                                self.output_buffer.push((from(value), diff.clone()));
                             }
                             else {
                                 self.temporary.push(next_time.join(time));
diff --git a/src/trace/cursor/cursor_list.rs b/src/trace/cursor/cursor_list.rs
index e884ca55b..98bf6a808 100644
--- a/src/trace/cursor/cursor_list.rs
+++ b/src/trace/cursor/cursor_list.rs
@@ -88,7 +88,6 @@ impl<C: Cursor> Cursor for CursorList<C> {
     type Key<'a> = C::Key<'a>;
     type KeyOwned = C::KeyOwned;
     type Val<'a> = C::Val<'a>;
-    type ValOwned = C::ValOwned;
     type Time = C::Time;
     type Diff = C::Diff;
 
diff --git a/src/trace/cursor/mod.rs b/src/trace/cursor/mod.rs
index 46fb4a6ba..927dba4b2 100644
--- a/src/trace/cursor/mod.rs
+++ b/src/trace/cursor/mod.rs
@@ -63,9 +63,7 @@ pub trait Cursor {
     /// Owned version of the above.
     type KeyOwned: Ord + Clone;
     /// Values associated with keys.
-    type Val<'a>: Copy + Clone + MyTrait<'a, Owned = Self::ValOwned> + for<'b> PartialOrd<Self::Val<'b>>;
-    /// Owned version of the above.
-    type ValOwned: Ord + Clone;
+    type Val<'a>: Copy + Clone + MyTrait<'a> + for<'b> PartialOrd<Self::Val<'b>>;
     /// Timestamps associated with updates
     type Time: Timestamp + Lattice + Ord + Clone;
     /// Associated update.
@@ -121,7 +119,10 @@ pub trait Cursor {
     fn rewind_vals(&mut self, storage: &Self::Storage);
 
     /// Rewinds the cursor and outputs its contents to a Vec
-    fn to_vec(&mut self, storage: &Self::Storage) -> Vec<((Self::KeyOwned, Self::ValOwned), Vec<(Self::Time, Self::Diff)>)> {
+    fn to_vec<V, F>(&mut self, from: F, storage: &Self::Storage) -> Vec<((Self::KeyOwned, V), Vec<(Self::Time, Self::Diff)>)>
+    where 
+        F: Fn(Self::Val<'_>) -> V,
+    {
         let mut out = Vec::new();
         self.rewind_keys(storage);
         self.rewind_vals(storage);
@@ -131,7 +132,7 @@ pub trait Cursor {
                 self.map_times(storage, |ts, r| {
                     kv_out.push((ts.clone(), r.clone()));
                 });
-                out.push(((self.key(storage).into_owned(), self.val(storage).into_owned()), kv_out));
+                out.push(((self.key(storage).into_owned(), from(self.val(storage))), kv_out));
                 self.step_val(storage);
             }
             self.step_key(storage);
diff --git a/src/trace/implementations/ord_neu.rs b/src/trace/implementations/ord_neu.rs
index ce0c6532a..42e45aefe 100644
--- a/src/trace/implementations/ord_neu.rs
+++ b/src/trace/implementations/ord_neu.rs
@@ -140,7 +140,6 @@ mod val_batch {
         type Key<'a> = <L::KeyContainer as BatchContainer>::ReadItem<'a>;
         type KeyOwned = <L::Target as Update>::Key;
         type Val<'a> = <L::ValContainer as BatchContainer>::ReadItem<'a>;
-        type ValOwned = <L::Target as Update>::Val;
         type Time = <L::Target as Update>::Time;
         type Diff = <L::Target as Update>::Diff;
 
@@ -447,7 +446,6 @@ mod val_batch {
         type Key<'a> = <L::KeyContainer as BatchContainer>::ReadItem<'a>;
         type KeyOwned = <L::Target as Update>::Key;
         type Val<'a> = <L::ValContainer as BatchContainer>::ReadItem<'a>;
-        type ValOwned = <L::Target as Update>::Val;
         type Time = <L::Target as Update>::Time;
         type Diff = <L::Target as Update>::Diff;
 
@@ -700,7 +698,6 @@ mod key_batch {
         type Key<'a> = <L::KeyContainer as BatchContainer>::ReadItem<'a>;
         type KeyOwned = <L::Target as Update>::Key;
         type Val<'a> = &'a ();
-        type ValOwned = ();
         type Time = <L::Target as Update>::Time;
         type Diff = <L::Target as Update>::Diff;
 
@@ -918,7 +915,6 @@ mod key_batch {
         type Key<'a> = <L::KeyContainer as BatchContainer>::ReadItem<'a>;
         type KeyOwned = <L::Target as Update>::Key;
         type Val<'a> = &'a ();
-        type ValOwned = ();
         type Time = <L::Target as Update>::Time;
         type Diff = <L::Target as Update>::Diff;
 
diff --git a/src/trace/implementations/rhh.rs b/src/trace/implementations/rhh.rs
index f29d39a8b..b01d1dae3 100644
--- a/src/trace/implementations/rhh.rs
+++ b/src/trace/implementations/rhh.rs
@@ -256,7 +256,6 @@ mod val_batch {
         type Key<'a> = <L::KeyContainer as BatchContainer>::ReadItem<'a>;
         type KeyOwned = <L::Target as Update>::Key;
         type Val<'a> = <L::ValContainer as BatchContainer>::ReadItem<'a>;
-        type ValOwned = <L::Target as Update>::Val;
         type Time = <L::Target as Update>::Time;
         type Diff = <L::Target as Update>::Diff;
 
@@ -608,7 +607,6 @@ mod val_batch {
         type Key<'a> = <L::KeyContainer as BatchContainer>::ReadItem<'a>;
         type KeyOwned = <L::Target as Update>::Key;
         type Val<'a> = <L::ValContainer as BatchContainer>::ReadItem<'a>;
-        type ValOwned = <L::Target as Update>::Val;
         type Time = <L::Target as Update>::Time;
         type Diff = <L::Target as Update>::Diff;
 
diff --git a/src/trace/implementations/spine_fueled.rs b/src/trace/implementations/spine_fueled.rs
index c616e2b7d..faea1dfdb 100644
--- a/src/trace/implementations/spine_fueled.rs
+++ b/src/trace/implementations/spine_fueled.rs
@@ -112,7 +112,6 @@ where
     type Key<'a> = B::Key<'a>;
     type KeyOwned = B::KeyOwned;
     type Val<'a> = B::Val<'a>;
-    type ValOwned = B::ValOwned;
     type Time = B::Time;
     type Diff = B::Diff;
 
diff --git a/src/trace/mod.rs b/src/trace/mod.rs
index 145c2990f..d1f878342 100644
--- a/src/trace/mod.rs
+++ b/src/trace/mod.rs
@@ -56,22 +56,20 @@ pub trait TraceReader {
     /// Owned version of the above.
     type KeyOwned: Ord + Clone;
     /// Values associated with keys.
-    type Val<'a>: Copy + Clone + MyTrait<'a, Owned = Self::ValOwned>;
-    /// Owned version of the above.
-    type ValOwned: Ord + Clone;
+    type Val<'a>: Copy + Clone + MyTrait<'a>;
     /// Timestamps associated with updates
     type Time: Timestamp + Lattice + Ord + Clone;
     /// Associated update.
     type Diff: Semigroup;
 
     /// The type of an immutable collection of updates.
-    type Batch: for<'a> BatchReader<Key<'a> = Self::Key<'a>, KeyOwned = Self::KeyOwned, Val<'a> = Self::Val<'a>, ValOwned = Self::ValOwned, Time = Self::Time, Diff = Self::Diff>+Clone+'static;
+    type Batch: for<'a> BatchReader<Key<'a> = Self::Key<'a>, KeyOwned = Self::KeyOwned, Val<'a> = Self::Val<'a>, Time = Self::Time, Diff = Self::Diff>+Clone+'static;
 
     /// Storage type for `Self::Cursor`. Likely related to `Self::Batch`.
     type Storage;
 
     /// The type used to enumerate the collections contents.
-    type Cursor: for<'a> Cursor<Storage=Self::Storage, Key<'a> = Self::Key<'a>, KeyOwned = Self::KeyOwned, Val<'a> = Self::Val<'a>, ValOwned = Self::ValOwned, Time = Self::Time, Diff = Self::Diff>;
+    type Cursor: for<'a> Cursor<Storage=Self::Storage, Key<'a> = Self::Key<'a>, KeyOwned = Self::KeyOwned, Val<'a> = Self::Val<'a>, Time = Self::Time, Diff = Self::Diff>;
 
     /// Provides a cursor over updates contained in the trace.
     fn cursor(&mut self) -> (Self::Cursor, Self::Storage) {
@@ -264,16 +262,14 @@ where
     /// Owned version of the above.
     type KeyOwned: Ord + Clone;
     /// Values associated with keys.
-    type Val<'a>: Copy + Clone + MyTrait<'a, Owned = Self::ValOwned>;
-    /// Owned version of the above.
-    type ValOwned: Ord + Clone;
+    type Val<'a>: Copy + Clone + MyTrait<'a>;
     /// Timestamps associated with updates
     type Time: Timestamp + Lattice + Ord + Clone;
     /// Associated update.
     type Diff: Semigroup;
 
     /// The type used to enumerate the batch's contents.
-    type Cursor: for<'a> Cursor<Storage=Self, Key<'a> = Self::Key<'a>, KeyOwned = Self::KeyOwned, Val<'a> = Self::Val<'a>, ValOwned = Self::ValOwned, Time = Self::Time, Diff = Self::Diff>;
+    type Cursor: for<'a> Cursor<Storage=Self, Key<'a> = Self::Key<'a>, KeyOwned = Self::KeyOwned, Val<'a> = Self::Val<'a>, Time = Self::Time, Diff = Self::Diff>;
     /// Acquires a cursor to the batch's contents.
     fn cursor(&self) -> Self::Cursor;
     /// The number of updates in the batch.
@@ -384,7 +380,6 @@ pub mod rc_blanket_impls {
         type Key<'a> = B::Key<'a>;
         type KeyOwned = B::KeyOwned;
         type Val<'a> = B::Val<'a>;
-        type ValOwned = B::ValOwned;
         type Time = B::Time;
         type Diff = B::Diff;
 
@@ -419,7 +414,6 @@ pub mod rc_blanket_impls {
         type Key<'a> = C::Key<'a>;
         type KeyOwned = C::KeyOwned;
         type Val<'a> = C::Val<'a>;
-        type ValOwned = C::ValOwned;
         type Time = C::Time;
         type Diff = C::Diff;
 
@@ -490,7 +484,6 @@ pub mod abomonated_blanket_impls {
         type Key<'a> = B::Key<'a>;
         type KeyOwned = B::KeyOwned;
         type Val<'a> = B::Val<'a>;
-        type ValOwned = B::ValOwned;
         type Time = B::Time;
         type Diff = B::Diff;
 
@@ -525,7 +518,6 @@ pub mod abomonated_blanket_impls {
         type Key<'a> = C::Key<'a>;
         type KeyOwned = C::KeyOwned;
         type Val<'a> = C::Val<'a>;
-        type ValOwned = C::ValOwned;
         type Time = C::Time;
         type Diff = C::Diff;
 
diff --git a/src/trace/wrappers/enter.rs b/src/trace/wrappers/enter.rs
index 49f3a1823..ce59e955e 100644
--- a/src/trace/wrappers/enter.rs
+++ b/src/trace/wrappers/enter.rs
@@ -34,7 +34,6 @@ where
     type Key<'a> = Tr::Key<'a>;
     type KeyOwned = Tr::KeyOwned;
     type Val<'a> = Tr::Val<'a>;
-    type ValOwned = Tr::ValOwned;
     type Time = TInner;
     type Diff = Tr::Diff;
 
@@ -118,7 +117,6 @@ where
     type Key<'a> = B::Key<'a>;
     type KeyOwned = B::KeyOwned;
     type Val<'a> = B::Val<'a>;
-    type ValOwned = B::ValOwned;
     type Time = TInner;
     type Diff = B::Diff;
 
@@ -172,7 +170,6 @@ where
     type Key<'a> = C::Key<'a>;
     type KeyOwned = C::KeyOwned;
     type Val<'a> = C::Val<'a>;
-    type ValOwned = C::ValOwned;
     type Time = TInner;
     type Diff = C::Diff;
 
@@ -225,7 +222,6 @@ where
     type Key<'a> = C::Key<'a>;
     type KeyOwned = C::KeyOwned;
     type Val<'a> = C::Val<'a>;
-    type ValOwned = C::ValOwned;
     type Time = TInner;
     type Diff = C::Diff;
 
diff --git a/src/trace/wrappers/enter_at.rs b/src/trace/wrappers/enter_at.rs
index 7f67234d9..7e157c409 100644
--- a/src/trace/wrappers/enter_at.rs
+++ b/src/trace/wrappers/enter_at.rs
@@ -52,7 +52,6 @@ where
     type Key<'a> = Tr::Key<'a>;
     type KeyOwned = Tr::KeyOwned;
     type Val<'a> = Tr::Val<'a>;
-    type ValOwned = Tr::ValOwned;
     type Time = TInner;
     type Diff = Tr::Diff;
 
@@ -141,7 +140,6 @@ where
     type Key<'a> = B::Key<'a>;
     type KeyOwned = B::KeyOwned;
     type Val<'a> = B::Val<'a>;
-    type ValOwned = B::ValOwned;
     type Time = TInner;
     type Diff = B::Diff;
 
@@ -199,7 +197,6 @@ where
     type Key<'a> = C::Key<'a>;
     type KeyOwned = C::KeyOwned;
     type Val<'a> = C::Val<'a>;
-    type ValOwned = C::ValOwned;
     type Time = TInner;
     type Diff = C::Diff;
 
@@ -258,7 +255,6 @@ where
     type Key<'a> = C::Key<'a>;
     type KeyOwned = C::KeyOwned;
     type Val<'a> = C::Val<'a>;
-    type ValOwned = C::ValOwned;
     type Time = TInner;
     type Diff = C::Diff;
 
diff --git a/src/trace/wrappers/filter.rs b/src/trace/wrappers/filter.rs
index 385285529..b4f320700 100644
--- a/src/trace/wrappers/filter.rs
+++ b/src/trace/wrappers/filter.rs
@@ -33,7 +33,6 @@ where
     type Key<'a> = Tr::Key<'a>;
     type KeyOwned = Tr::KeyOwned;
     type Val<'a> = Tr::Val<'a>;
-    type ValOwned = Tr::ValOwned;
     type Time = Tr::Time;
     type Diff = Tr::Diff;
 
@@ -87,7 +86,6 @@ where
     type Key<'a> = B::Key<'a>;
     type KeyOwned = B::KeyOwned;
     type Val<'a> = B::Val<'a>;
-    type ValOwned = B::ValOwned;
     type Time = B::Time;
     type Diff = B::Diff;
 
@@ -136,7 +134,6 @@ where
     type Key<'a> = C::Key<'a>;
     type KeyOwned = C::KeyOwned;
     type Val<'a> = C::Val<'a>;
-    type ValOwned = C::ValOwned;
     type Time = C::Time;
     type Diff = C::Diff;
 
@@ -191,7 +188,6 @@ where
     type Key<'a> = C::Key<'a>;
     type KeyOwned = C::KeyOwned;
     type Val<'a> = C::Val<'a>;
-    type ValOwned = C::ValOwned;
     type Time = C::Time;
     type Diff = C::Diff;
 
diff --git a/src/trace/wrappers/freeze.rs b/src/trace/wrappers/freeze.rs
index 9115cd244..310ef3157 100644
--- a/src/trace/wrappers/freeze.rs
+++ b/src/trace/wrappers/freeze.rs
@@ -78,7 +78,6 @@ where
     type Key<'a> = Tr::Key<'a>;
     type KeyOwned = Tr::KeyOwned;
     type Val<'a> = Tr::Val<'a>;
-    type ValOwned = Tr::ValOwned;
     type Time = Tr::Time;
     type Diff = Tr::Diff;
 
@@ -142,7 +141,6 @@ where
     type Key<'a> = B::Key<'a>;
     type KeyOwned = B::KeyOwned;
     type Val<'a> = B::Val<'a>;
-    type ValOwned = B::ValOwned;
     type Time = B::Time;
     type Diff = B::Diff;
 
@@ -186,7 +184,6 @@ where
     type Key<'a> = C::Key<'a>;
     type KeyOwned = C::KeyOwned;
     type Val<'a> = C::Val<'a>;
-    type ValOwned = C::ValOwned;
     type Time = C::Time;
     type Diff = C::Diff;
 
@@ -238,7 +235,6 @@ where
     type Key<'a> = C::Key<'a>;
     type KeyOwned = C::KeyOwned;
     type Val<'a> = C::Val<'a>;
-    type ValOwned = C::ValOwned;
     type Time = C::Time;
     type Diff = C::Diff;
 
diff --git a/src/trace/wrappers/frontier.rs b/src/trace/wrappers/frontier.rs
index 9d9d50307..02cce0dd1 100644
--- a/src/trace/wrappers/frontier.rs
+++ b/src/trace/wrappers/frontier.rs
@@ -35,7 +35,6 @@ impl<Tr: TraceReader> TraceReader for TraceFrontier<Tr> {
     type Key<'a> = Tr::Key<'a>;
     type KeyOwned = Tr::KeyOwned;
     type Val<'a> = Tr::Val<'a>;
-    type ValOwned = Tr::ValOwned;
     type Time = Tr::Time;
     type Diff = Tr::Diff;
 
@@ -86,7 +85,6 @@ impl<B: BatchReader> BatchReader for BatchFrontier<B> {
     type Key<'a> = B::Key<'a>;
     type KeyOwned = B::KeyOwned;
     type Val<'a> = B::Val<'a>;
-    type ValOwned = B::ValOwned;
     type Time = B::Time;
     type Diff = B::Diff;
 
@@ -131,7 +129,6 @@ impl<C: Cursor> Cursor for CursorFrontier<C, C::Time> {
     type Key<'a> = C::Key<'a>;
     type KeyOwned = C::KeyOwned;
     type Val<'a> = C::Val<'a>;
-    type ValOwned = C::ValOwned;
     type Time = C::Time;
     type Diff = C::Diff;
 
@@ -193,7 +190,6 @@ where
     type Key<'a> = C::Key<'a>;
     type KeyOwned = C::KeyOwned;
     type Val<'a> = C::Val<'a>;
-    type ValOwned = C::ValOwned;
     type Time = C::Time;
     type Diff = C::Diff;
 
diff --git a/src/trace/wrappers/rc.rs b/src/trace/wrappers/rc.rs
index ceda2e479..29d91d969 100644
--- a/src/trace/wrappers/rc.rs
+++ b/src/trace/wrappers/rc.rs
@@ -82,7 +82,6 @@ impl<Tr: TraceReader> TraceReader for TraceRc<Tr> {
     type Key<'a> = Tr::Key<'a>;
     type KeyOwned = Tr::KeyOwned;
     type Val<'a> = Tr::Val<'a>;
-    type ValOwned = Tr::ValOwned;
     type Time = Tr::Time;
     type Diff = Tr::Diff;
 
diff --git a/tests/trace.rs b/tests/trace.rs
index dac7eff22..a637b1700 100644
--- a/tests/trace.rs
+++ b/tests/trace.rs
@@ -35,11 +35,11 @@ fn test_trace() {
     let mut trace = get_trace();
 
     let (mut cursor1, storage1) = trace.cursor_through(AntichainRef::new(&[1])).unwrap();
-    let vec_1 = cursor1.to_vec(&storage1);
+    let vec_1 = cursor1.to_vec(|v| v.clone(), &storage1);
     assert_eq!(vec_1, vec![((1, 2), vec![(0, 1)])]);
 
     let (mut cursor2, storage2) = trace.cursor_through(AntichainRef::new(&[2])).unwrap();
-    let vec_2 = cursor2.to_vec(&storage2);
+    let vec_2 = cursor2.to_vec(|v| v.clone(), &storage2);
     println!("--> {:?}", vec_2);
     assert_eq!(vec_2, vec![
                ((1, 2), vec![(0, 1)]),
@@ -47,13 +47,13 @@ fn test_trace() {
     ]);
 
     let (mut cursor3, storage3) = trace.cursor_through(AntichainRef::new(&[3])).unwrap();
-    let vec_3 = cursor3.to_vec(&storage3);
+    let vec_3 = cursor3.to_vec(|v| v.clone(), &storage3);
     assert_eq!(vec_3, vec![
                ((1, 2), vec![(0, 1)]),
                ((2, 3), vec![(1, 1), (2, -1)]),
     ]);
 
     let (mut cursor4, storage4) = trace.cursor();
-    let vec_4 = cursor4.to_vec(&storage4);
+    let vec_4 = cursor4.to_vec(|v| v.clone(), &storage4);
     assert_eq!(vec_4, vec_3);
 }

From b281e50565c26d14fcbf4bcdab8e8ad01e1f4baa Mon Sep 17 00:00:00 2001
From: Moritz Hoffmann <antiguru@gmail.com>
Date: Fri, 26 Apr 2024 17:10:26 -0400
Subject: [PATCH 3/5] Merge batcher generic over containers (#474)

* Split default merge batcher

This change splits the default merge batcher implementation into a type
that maintains the outer part of its algorithm, specifically knows how
to maintain chains, and an inner part that knows how to maintain the
individual batches in chains. The benefit is that the outer part does
not need to know about the contents of the containers it holds on to
because that's encapsulated in the inner trait's implementation.

Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>

* Undo some changes, rip out old columnated batcher

Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>

* Address feedback

Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>

* Formatting and renaming

Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>

* Address feedback

Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>

---------

Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>
---
 src/operators/arrange/arrangement.rs          |   2 +-
 src/operators/reduce.rs                       |   2 +-
 src/trace/implementations/merge_batcher.rs    | 589 +++++++++++-------
 .../implementations/merge_batcher_col.rs      | 570 +++++++----------
 src/trace/implementations/mod.rs              |   3 -
 src/trace/implementations/ord_neu.rs          |  14 +-
 src/trace/implementations/rhh.rs              |  16 +-
 src/trace/mod.rs                              |   6 +-
 tests/trace.rs                                |   2 +-
 9 files changed, 630 insertions(+), 574 deletions(-)

diff --git a/src/operators/arrange/arrangement.rs b/src/operators/arrange/arrangement.rs
index c22749a27..ba7bf2045 100644
--- a/src/operators/arrange/arrangement.rs
+++ b/src/operators/arrange/arrangement.rs
@@ -464,7 +464,7 @@ where
 
             input.for_each(|cap, data| {
                 capabilities.insert(cap.retain());
-                batcher.push_batch(data);
+                batcher.push_container(data);
             });
 
             // The frontier may have advanced by multiple elements, which is an issue because
diff --git a/src/operators/reduce.rs b/src/operators/reduce.rs
index db80a5d44..dda549bca 100644
--- a/src/operators/reduce.rs
+++ b/src/operators/reduce.rs
@@ -240,7 +240,7 @@ pub trait ReduceCore<G: Scope, K: ToOwned + ?Sized, V: Data, R: Semigroup> where
     ///          .map(|x| (x, x))
     ///          .reduce_abelian::<_,_,ValSpine<_,_,_,_>>(
     ///             "Example",
-    ///              Clone::clone, 
+    ///              Clone::clone,
     ///              move |_key, src, dst| dst.push((*src[0].0, 1))
     ///          )
     ///          .trace;
diff --git a/src/trace/implementations/merge_batcher.rs b/src/trace/implementations/merge_batcher.rs
index 2cf27ed1b..45e2a60f8 100644
--- a/src/trace/implementations/merge_batcher.rs
+++ b/src/trace/implementations/merge_batcher.rs
@@ -5,165 +5,243 @@ use std::collections::VecDeque;
 use timely::communication::message::RefOrMut;
 use timely::logging::WorkerIdentifier;
 use timely::logging_core::Logger;
+use timely::progress::frontier::AntichainRef;
 use timely::progress::{frontier::Antichain, Timestamp};
+use timely::{Container, PartialOrder};
 
+use crate::consolidation::consolidate_updates;
 use crate::difference::Semigroup;
 use crate::logging::{BatcherEvent, DifferentialEvent};
 use crate::trace::{Batcher, Builder};
+use crate::Data;
 
 /// Creates batches from unordered tuples.
-pub struct MergeBatcher<K, V, T, D> {
-    sorter: MergeSorter<(K, V), T, D>,
+pub struct MergeBatcher<M, T>
+where
+    M: Merger,
+{
+    /// each power-of-two length list of allocations.
+    /// Do not push/pop directly but use the corresponding functions
+    /// ([`Self::chain_push`]/[`Self::chain_pop`]).
+    chains: Vec<Vec<M::Chunk>>,
+    /// Stash of empty chunks
+    stash: Vec<M::Chunk>,
+    /// Thing to accept data, merge chains, and talk to the builder.
+    merger: M,
+    /// Logger for size accounting.
+    logger: Option<Logger<DifferentialEvent, WorkerIdentifier>>,
+    /// Timely operator ID.
+    operator_id: usize,
+    /// Current lower frontier, we sealed up to here.
     lower: Antichain<T>,
+    /// The lower-bound frontier of the data, after the last call to seal.
     frontier: Antichain<T>,
 }
 
-impl<K, V, T, D> Batcher for MergeBatcher<K, V, T, D>
+impl<M, T> Batcher for MergeBatcher<M, T>
 where
-    K: Ord + Clone,
-    V: Ord + Clone,
+    M: Merger<Time = T>,
     T: Timestamp,
-    D: Semigroup,
 {
-    type Input = Vec<((K,V),T,D)>;
-    type Output = ((K,V),T,D);
+    type Input = M::Input;
+    type Output = M::Output;
     type Time = T;
 
     fn new(logger: Option<Logger<DifferentialEvent, WorkerIdentifier>>, operator_id: usize) -> Self {
-        MergeBatcher {
-            sorter: MergeSorter::new(logger, operator_id),
+        Self {
+            logger,
+            operator_id,
+            merger: M::default(),
+            chains: Vec::new(),
+            stash: Vec::new(),
             frontier: Antichain::new(),
             lower: Antichain::from_elem(T::minimum()),
         }
     }
 
-    #[inline(never)]
-    fn push_batch(&mut self, batch: RefOrMut<Self::Input>) {
-        // `batch` is either a shared reference or an owned allocations.
-        match batch {
-            RefOrMut::Ref(reference) => {
-                // This is a moment at which we could capture the allocations backing
-                // `batch` into a different form of region, rather than just  cloning.
-                let mut owned: Vec<_> = self.sorter.empty();
-                owned.clone_from(reference);
-                self.sorter.push(&mut owned);
-            },
-            RefOrMut::Mut(reference) => {
-                self.sorter.push(reference);
-            }
-        }
+    /// Push a container of data into this merge batcher. Updates the internal chain structure if
+    /// needed.
+    fn push_container(&mut self, container: RefOrMut<M::Input>) {
+        let chain = self.merger.accept(container, &mut self.stash);
+        self.insert_chain(chain);
     }
 
     // Sealing a batch means finding those updates with times not greater or equal to any time
     // in `upper`. All updates must have time greater or equal to the previously used `upper`,
     // which we call `lower`, by assumption that after sealing a batcher we receive no more
     // updates with times not greater or equal to `upper`.
-    #[inline(never)]
-    fn seal<B: Builder<Input=Self::Output, Time=Self::Time>>(&mut self, upper: Antichain<T>) -> B::Output {
-
-        let mut merged = Vec::new();
-        self.sorter.finish_into(&mut merged);
-
-        // Determine the number of distinct keys, values, and updates,
-        // and form a builder pre-sized for these numbers.
-        let mut builder = {
-            let mut keys = 0;
-            let mut vals = 0;
-            let mut upds = 0;
-            let mut prev_keyval = None;
-            for buffer in merged.iter() {
-                for ((key, val), time, _) in buffer.iter() {
-                    if !upper.less_equal(time) {
-                        if let Some((p_key, p_val)) = prev_keyval {
-                            if p_key != key {
-                                keys += 1;
-                                vals += 1;
-                            }
-                            else if p_val != val {
-                                vals += 1;
-                            }
-                            upds += 1;
-                        } else {
-                            keys += 1;
-                            vals += 1;
-                            upds += 1;
-                        }
-                        prev_keyval = Some((key, val));
-                    }
-                }
-            }
-            B::with_capacity(keys, vals, upds)
-        };
+    fn seal<B: Builder<Input = Self::Output, Time = Self::Time>>(&mut self, upper: Antichain<T>) -> B::Output {
+        // Finish
+        let chain = self.merger.finish(&mut self.stash);
+        if !chain.is_empty() {
+            self.chain_push(chain);
+        }
 
-        let mut kept = Vec::new();
-        let mut keep = Vec::new();
+        // Merge all remaining chains into a single chain.
+        while self.chains.len() > 1 {
+            let list1 = self.chain_pop().unwrap();
+            let list2 = self.chain_pop().unwrap();
+            let merged = self.merge_by(list1, list2);
+            self.chain_push(merged);
+        }
+        let merged = self.chain_pop().unwrap_or_default();
 
+        // Extract readied data.
+        let mut kept = Vec::new();
+        let mut readied = Vec::new();
         self.frontier.clear();
 
-        // TODO: Re-use buffer, rather than dropping.
-        for mut buffer in merged.drain(..) {
-            for ((key, val), time, diff) in buffer.drain(..) {
-                if upper.less_equal(&time) {
-                    self.frontier.insert(time.clone());
-                    if keep.len() == keep.capacity() && !keep.is_empty() {
-                        kept.push(keep);
-                        keep = self.sorter.empty();
-                    }
-                    keep.push(((key, val), time, diff));
-                }
-                else {
-                    builder.push(((key, val), time, diff));
-                }
-            }
-            // Recycling buffer.
-            self.sorter.push(&mut buffer);
-        }
+        self.merger.extract(merged, upper.borrow(), &mut self.frontier, &mut readied, &mut kept, &mut self.stash);
 
-        // Finish the kept data.
-        if !keep.is_empty() {
-            kept.push(keep);
-        }
         if !kept.is_empty() {
-            self.sorter.push_list(kept);
+            self.chain_push(kept);
         }
 
-        // Drain buffers (fast reclaimation).
-        // TODO : This isn't obviously the best policy, but "safe" wrt footprint.
-        //        In particular, if we are reading serialized input data, we may
-        //        prefer to keep these buffers around to re-fill, if possible.
-        let mut buffer = Vec::new();
-        self.sorter.push(&mut buffer);
-        // We recycle buffers with allocations (capacity, and not zero-sized).
-        while buffer.capacity() > 0 && std::mem::size_of::<((K,V),T,D)>() > 0 {
-            buffer = Vec::new();
-            self.sorter.push(&mut buffer);
-        }
+        self.stash.clear();
 
-        let seal = builder.done(self.lower.clone(), upper.clone(), Antichain::from_elem(T::minimum()));
+        let seal = M::seal::<B>(&mut readied, self.lower.borrow(), upper.borrow(), Antichain::from_elem(T::minimum()).borrow());
         self.lower = upper;
         seal
     }
 
     /// The frontier of elements remaining after the most recent call to `self.seal`.
-    fn frontier(&mut self) -> timely::progress::frontier::AntichainRef<T> {
+    #[inline]
+    fn frontier(&mut self) -> AntichainRef<T> {
         self.frontier.borrow()
     }
 }
 
-struct MergeSorter<D, T, R> {
-    /// each power-of-two length list of allocations. Do not push/pop directly but use the corresponding functions.
-    queue: Vec<Vec<Vec<(D, T, R)>>>,
-    stash: Vec<Vec<(D, T, R)>>,
-    logger: Option<Logger<DifferentialEvent, WorkerIdentifier>>,
-    operator_id: usize,
+impl<M, T> MergeBatcher<M, T>
+where
+    M: Merger,
+{
+    /// Insert a chain and maintain chain properties: Chains are geometrically sized and ordered
+    /// by decreasing length.
+    fn insert_chain(&mut self, chain: Vec<M::Chunk>) {
+        if !chain.is_empty() {
+            self.chain_push(chain);
+            while self.chains.len() > 1 && (self.chains[self.chains.len() - 1].len() >= self.chains[self.chains.len() - 2].len() / 2) {
+                let list1 = self.chain_pop().unwrap();
+                let list2 = self.chain_pop().unwrap();
+                let merged = self.merge_by(list1, list2);
+                self.chain_push(merged);
+            }
+        }
+    }
+
+    // merges two sorted input lists into one sorted output list.
+    fn merge_by(&mut self, list1: Vec<M::Chunk>, list2: Vec<M::Chunk>) -> Vec<M::Chunk> {
+        // TODO: `list1` and `list2` get dropped; would be better to reuse?
+        let mut output = Vec::with_capacity(list1.len() + list2.len());
+        self.merger.merge(list1, list2, &mut output, &mut self.stash);
+
+        output
+    }
+
+    /// Pop a chain and account size changes.
+    #[inline]
+    fn chain_pop(&mut self) -> Option<Vec<M::Chunk>> {
+        let chain = self.chains.pop();
+        self.account(chain.iter().flatten().map(M::account), -1);
+        chain
+    }
+
+    /// Push a chain and account size changes.
+    #[inline]
+    fn chain_push(&mut self, chain: Vec<M::Chunk>) {
+        self.account(chain.iter().map(M::account), 1);
+        self.chains.push(chain);
+    }
+
+    /// Account size changes. Only performs work if a logger exists.
+    ///
+    /// Calculate the size based on the iterator passed along, with each attribute
+    /// multiplied by `diff`. Usually, one wants to pass 1 or -1 as the diff.
+    #[inline]
+    fn account<I: IntoIterator<Item = (usize, usize, usize, usize)>>(&self, items: I, diff: isize) {
+        if let Some(logger) = &self.logger {
+            let (mut records, mut size, mut capacity, mut allocations) = (0isize, 0isize, 0isize, 0isize);
+            for (records_, size_, capacity_, allocations_) in items {
+                records = records.saturating_add_unsigned(records_);
+                size = size.saturating_add_unsigned(size_);
+                capacity = capacity.saturating_add_unsigned(capacity_);
+                allocations = allocations.saturating_add_unsigned(allocations_);
+            }
+            logger.log(BatcherEvent {
+                operator: self.operator_id,
+                records_diff: records * diff,
+                size_diff: size * diff,
+                capacity_diff: capacity * diff,
+                allocations_diff: allocations * diff,
+            })
+        }
+    }
+}
+
+impl<M: Merger, T> Drop for MergeBatcher<M, T> {
+    fn drop(&mut self) {
+        // Cleanup chain to retract accounting information.
+        while self.chain_pop().is_some() {}
+    }
 }
 
-impl<D: Ord, T: Ord, R: Semigroup> MergeSorter<D, T, R> {
+/// A trait to describe interesting moments in a merge batcher.
+pub trait Merger: Default {
+    /// The type of update containers received from inputs.
+    type Input;
+    /// The internal representation of chunks of data.
+    type Chunk: Container;
+    /// The output type
+    /// TODO: This should be replaced by `Chunk` or another container once the builder understands
+    /// building from a complete chain.
+    type Output;
+    /// The type of time in frontiers to extract updates.
+    type Time;
+    /// Accept a fresh container of input data.
+    fn accept(&mut self, container: RefOrMut<Self::Input>, stash: &mut Vec<Self::Chunk>) -> Vec<Self::Chunk>;
+    /// Finish processing any stashed data.
+    fn finish(&mut self, stash: &mut Vec<Self::Chunk>) -> Vec<Self::Chunk>;
+    /// Merge chains into an output chain.
+    fn merge(&mut self, list1: Vec<Self::Chunk>, list2: Vec<Self::Chunk>, output: &mut Vec<Self::Chunk>, stash: &mut Vec<Self::Chunk>);
+    /// Extract ready updates based on the `upper` frontier.
+    fn extract(
+        &mut self,
+        merged: Vec<Self::Chunk>,
+        upper: AntichainRef<Self::Time>,
+        frontier: &mut Antichain<Self::Time>,
+        readied: &mut Vec<Self::Chunk>,
+        kept: &mut Vec<Self::Chunk>,
+        stash: &mut Vec<Self::Chunk>,
+    );
+
+    /// Build from a chain
+    /// TODO: We can move this entirely to `MergeBatcher` once builders can accepts chains.
+    fn seal<B: Builder<Input = Self::Output, Time = Self::Time>>(
+        chain: &mut Vec<Self::Chunk>,
+        lower: AntichainRef<Self::Time>,
+        upper: AntichainRef<Self::Time>,
+        since: AntichainRef<Self::Time>,
+    ) -> B::Output;
+
+    /// Account size and allocation changes. Returns a tuple of (records, size, capacity, allocations).
+    fn account(chunk: &Self::Chunk) -> (usize, usize, usize, usize);
+}
+
+/// A merger that knows how to accept and maintain chains of vectors.
+pub struct VecMerger<T> {
+    pending: Vec<T>,
+}
 
-    const BUFFER_SIZE_BYTES: usize = 1 << 13;
+impl<T> Default for VecMerger<T> {
+    fn default() -> Self {
+        Self { pending: Vec::default() }
+    }
+}
 
-    fn buffer_size() -> usize {
-        let size = ::std::mem::size_of::<(D, T, R)>();
+impl<T> VecMerger<T> {
+    const BUFFER_SIZE_BYTES: usize = 8 << 10;
+    fn chunk_capacity(&self) -> usize {
+        let size = ::std::mem::size_of::<T>();
         if size == 0 {
             Self::BUFFER_SIZE_BYTES
         } else if size <= Self::BUFFER_SIZE_BYTES {
@@ -173,102 +251,128 @@ impl<D: Ord, T: Ord, R: Semigroup> MergeSorter<D, T, R> {
         }
     }
 
-    #[inline]
-    fn new(logger: Option<Logger<DifferentialEvent, WorkerIdentifier>>, operator_id: usize) -> Self {
-        Self {
-            logger,
-            operator_id,
-            queue: Vec::new(),
-            stash: Vec::new(),
-        }
+    fn pending_capacity(&self) -> usize {
+        self.chunk_capacity() * 2
     }
 
+    /// Helper to get pre-sized vector from the stash.
     #[inline]
-    pub fn empty(&mut self) -> Vec<(D, T, R)> {
-        self.stash.pop().unwrap_or_else(|| Vec::with_capacity(Self::buffer_size()))
+    fn empty(&self, stash: &mut Vec<Vec<T>>) -> Vec<T> {
+        stash.pop().unwrap_or_else(|| Vec::with_capacity(self.chunk_capacity()))
     }
 
+    /// Helper to return a chunk to the stash.
     #[inline]
-    pub fn push(&mut self, batch: &mut Vec<(D, T, R)>) {
-        // TODO: Reason about possible unbounded stash growth. How to / should we return them?
-        // TODO: Reason about mis-sized vectors, from deserialized data; should probably drop.
-        let mut batch = if self.stash.len() > 2 {
-            ::std::mem::replace(batch, self.stash.pop().unwrap())
-        }
-        else {
-            ::std::mem::take(batch)
-        };
-
-        if !batch.is_empty() {
-            crate::consolidation::consolidate_updates(&mut batch);
-            self.account([batch.len()], 1);
-            self.queue_push(vec![batch]);
-            while self.queue.len() > 1 && (self.queue[self.queue.len()-1].len() >= self.queue[self.queue.len()-2].len() / 2) {
-                let list1 = self.queue_pop().unwrap();
-                let list2 = self.queue_pop().unwrap();
-                let merged = self.merge_by(list1, list2);
-                self.queue_push(merged);
-            }
+    fn recycle(&self, mut chunk: Vec<T>, stash: &mut Vec<Vec<T>>) {
+        // TODO: Should we limit the size of `stash`?
+        if chunk.capacity() == self.chunk_capacity() {
+            chunk.clear();
+            stash.push(chunk);
         }
     }
+}
 
-    // This is awkward, because it isn't a power-of-two length any more, and we don't want
-    // to break it down to be so.
-    pub fn push_list(&mut self, list: Vec<Vec<(D, T, R)>>) {
-        while self.queue.len() > 1 && self.queue[self.queue.len()-1].len() < list.len() {
-            let list1 = self.queue_pop().unwrap();
-            let list2 = self.queue_pop().unwrap();
-            let merged = self.merge_by(list1, list2);
-            self.queue_push(merged);
+impl<K, V, T, R> Merger for VecMerger<((K, V), T, R)>
+where
+    K: Data,
+    V: Data,
+    T: Ord + PartialOrder + Clone + 'static,
+    R: Semigroup + 'static,
+{
+    type Time = T;
+    type Input = Vec<((K, V), T, R)>;
+    type Chunk = Vec<((K, V), T, R)>;
+    type Output = ((K, V), T, R);
+
+    fn accept(&mut self, container: RefOrMut<Self::Input>, stash: &mut Vec<Self::Chunk>) -> Vec<Self::Chunk> {
+        // Ensure `self.pending` has the desired capacity. We should never have a larger capacity
+        // because we don't write more than capacity elements into the buffer.
+        if self.pending.capacity() < self.pending_capacity() {
+            self.pending.reserve(self.pending_capacity() - self.pending.len());
         }
-        self.queue_push(list);
-    }
 
-    #[inline(never)]
-    pub fn finish_into(&mut self, target: &mut Vec<Vec<(D, T, R)>>) {
-        while self.queue.len() > 1 {
-            let list1 = self.queue_pop().unwrap();
-            let list2 = self.queue_pop().unwrap();
-            let merged = self.merge_by(list1, list2);
-            self.queue_push(merged);
-        }
+        // Form a chain from what's in pending.
+        // This closure does the following:
+        // * If pending is full, consolidate.
+        // * If after consolidation it's more than half full, peel off a chain of full blocks,
+        //   leaving behind any partial block in pending.
+        // * Merge the new chain with `final_chain` and return it in-place.
+        let form_chain = |this: &mut Self, final_chain: &mut Vec<Self::Chunk>, stash: &mut _| {
+            if this.pending.len() == this.pending.capacity() {
+                consolidate_updates(&mut this.pending);
+                if this.pending.len() >= this.chunk_capacity() {
+                    let mut chain = Vec::default();
+                    while this.pending.len() > this.chunk_capacity() {
+                        let mut chunk = this.empty(stash);
+                        chunk.extend(this.pending.drain(..chunk.capacity()));
+                        chain.push(chunk);
+                    }
+                    if final_chain.is_empty() {
+                        *final_chain = chain;
+                    } else if !chain.is_empty() {
+                        let mut output = Vec::default();
+                        this.merge(std::mem::take(final_chain), chain, &mut output, stash);
+                        *final_chain = output;
+                    }
+                }
+            }
+        };
 
-        if let Some(mut last) = self.queue_pop() {
-            ::std::mem::swap(&mut last, target);
+        let mut final_chain = Vec::default();
+        // `container` is either a shared reference or an owned allocations.
+        match container {
+            RefOrMut::Ref(vec) => {
+                let mut slice = &vec[..];
+                while !slice.is_empty() {
+                    let (head, tail) = slice.split_at(std::cmp::min(self.pending.capacity() - self.pending.len(), slice.len()));
+                    slice = tail;
+                    self.pending.extend_from_slice(head);
+                    form_chain(self, &mut final_chain, stash);
+                }
+            }
+            RefOrMut::Mut(vec) => {
+                while !vec.is_empty() {
+                    self.pending.extend(vec.drain(..std::cmp::min(self.pending.capacity() - self.pending.len(), vec.len())));
+                    form_chain(self, &mut final_chain, stash);
+                }
+            }
         }
+        final_chain
     }
 
-    // merges two sorted input lists into one sorted output list.
-    #[inline(never)]
-    fn merge_by(&mut self, list1: Vec<Vec<(D, T, R)>>, list2: Vec<Vec<(D, T, R)>>) -> Vec<Vec<(D, T, R)>> {
-        self.account(list1.iter().chain(list2.iter()).map(Vec::len), -1);
-
-        use std::cmp::Ordering;
-
-        // TODO: `list1` and `list2` get dropped; would be better to reuse?
-        let mut output = Vec::with_capacity(list1.len() + list2.len());
-        let mut result = self.empty();
+    fn finish(&mut self, stash: &mut Vec<Self::Chunk>) -> Vec<Self::Chunk> {
+        // Extract all data from `pending`.
+        consolidate_updates(&mut self.pending);
+        let mut chain = Vec::default();
+        while !self.pending.is_empty() {
+            let mut chunk = self.empty(stash);
+            chunk.extend(self.pending.drain(..std::cmp::min(chunk.capacity(), self.pending.len())));
+            chain.push(chunk);
+        }
+        chain
+    }
 
+    fn merge(&mut self, list1: Vec<Self::Chunk>, list2: Vec<Self::Chunk>, output: &mut Vec<Self::Chunk>, stash: &mut Vec<Self::Chunk>) {
         let mut list1 = list1.into_iter();
         let mut list2 = list2.into_iter();
-
         let mut head1 = VecDeque::from(list1.next().unwrap_or_default());
         let mut head2 = VecDeque::from(list2.next().unwrap_or_default());
 
+        let mut result = self.empty(stash);
+
         // while we have valid data in each input, merge.
         while !head1.is_empty() && !head2.is_empty() {
-
             while (result.capacity() - result.len()) > 0 && !head1.is_empty() && !head2.is_empty() {
-
                 let cmp = {
                     let x = head1.front().unwrap();
                     let y = head2.front().unwrap();
                     (&x.0, &x.1).cmp(&(&y.0, &y.1))
                 };
+                use std::cmp::Ordering;
                 match cmp {
-                    Ordering::Less    => result.push(head1.pop_front().unwrap()),
+                    Ordering::Less => result.push(head1.pop_front().unwrap()),
                     Ordering::Greater => result.push(head2.pop_front().unwrap()),
-                    Ordering::Equal   => {
+                    Ordering::Equal => {
                         let (data1, time1, mut diff1) = head1.pop_front().unwrap();
                         let (_data2, _time2, diff2) = head2.pop_front().unwrap();
                         diff1.plus_equals(&diff2);
@@ -281,81 +385,126 @@ impl<D: Ord, T: Ord, R: Semigroup> MergeSorter<D, T, R> {
 
             if result.capacity() == result.len() {
                 output.push(result);
-                result = self.empty();
+                result = self.empty(stash);
             }
 
             if head1.is_empty() {
                 let done1 = Vec::from(head1);
-                if done1.capacity() == Self::buffer_size() { self.stash.push(done1); }
+                self.recycle(done1, stash);
                 head1 = VecDeque::from(list1.next().unwrap_or_default());
             }
             if head2.is_empty() {
                 let done2 = Vec::from(head2);
-                if done2.capacity() == Self::buffer_size() { self.stash.push(done2); }
+                self.recycle(done2, stash);
                 head2 = VecDeque::from(list2.next().unwrap_or_default());
             }
         }
 
-        if !result.is_empty() { output.push(result); }
-        else if result.capacity() > 0 { self.stash.push(result); }
+        if !result.is_empty() {
+            output.push(result);
+        } else {
+            self.recycle(result, stash);
+        }
 
         if !head1.is_empty() {
-            let mut result = self.empty();
-            for item1 in head1 { result.push(item1); }
+            let mut result = self.empty(stash);
+            for item1 in head1 {
+                result.push(item1);
+            }
             output.push(result);
         }
         output.extend(list1);
 
         if !head2.is_empty() {
-            let mut result = self.empty();
-            for item2 in head2 { result.push(item2); }
+            let mut result = self.empty(stash);
+            for item2 in head2 {
+                result.push(item2);
+            }
             output.push(result);
         }
         output.extend(list2);
-
-        output
     }
-}
 
-impl<D, T, R> MergeSorter<D, T, R> {
-    /// Pop a batch from `self.queue` and account size changes.
-    #[inline]
-    fn queue_pop(&mut self) -> Option<Vec<Vec<(D, T, R)>>> {
-        let batch = self.queue.pop();
-        self.account(batch.iter().flatten().map(Vec::len), -1);
-        batch
-    }
-
-    /// Push a batch to `self.queue` and account size changes.
-    #[inline]
-    fn queue_push(&mut self, batch: Vec<Vec<(D, T, R)>>) {
-        self.account(batch.iter().map(Vec::len), 1);
-        self.queue.push(batch);
+    fn extract(
+        &mut self,
+        merged: Vec<Self::Chunk>,
+        upper: AntichainRef<Self::Time>,
+        frontier: &mut Antichain<Self::Time>,
+        readied: &mut Vec<Self::Chunk>,
+        kept: &mut Vec<Self::Chunk>,
+        stash: &mut Vec<Self::Chunk>,
+    ) {
+        let mut keep = self.empty(stash);
+        let mut ready = self.empty(stash);
+
+        for mut buffer in merged {
+            for (data, time, diff) in buffer.drain(..) {
+                if upper.less_equal(&time) {
+                    frontier.insert_ref(&time);
+                    if keep.len() == keep.capacity() && !keep.is_empty() {
+                        kept.push(keep);
+                        keep = self.empty(stash);
+                    }
+                    keep.push((data, time, diff));
+                } else {
+                    if ready.len() == ready.capacity() && !ready.is_empty() {
+                        readied.push(ready);
+                        ready = self.empty(stash);
+                    }
+                    ready.push((data, time, diff));
+                }
+            }
+            // Recycling buffer.
+            self.recycle(buffer, stash);
+        }
+        // Finish the kept data.
+        if !keep.is_empty() {
+            kept.push(keep);
+        }
+        if !ready.is_empty() {
+            readied.push(ready);
+        }
     }
 
-    /// Account size changes. Only performs work if a logger exists.
-    ///
-    /// Calculate the size based on the [`TimelyStack`]s passed along, with each attribute
-    /// multiplied by `diff`. Usually, one wants to pass 1 or -1 as the diff.
-    fn account<I: IntoIterator<Item=usize>>(&self, items: I, diff: isize) {
-        if let Some(logger) = &self.logger {
-            let mut records= 0isize;
-            for len in items {
-                records = records.saturating_add_unsigned(len);
+    fn seal<B: Builder<Input = Self::Output, Time = Self::Time>>(
+        chain: &mut Vec<Self::Chunk>,
+        lower: AntichainRef<Self::Time>,
+        upper: AntichainRef<Self::Time>,
+        since: AntichainRef<Self::Time>,
+    ) -> B::Output {
+        let mut keys = 0;
+        let mut vals = 0;
+        let mut upds = 0;
+        let mut prev_keyval = None;
+        for buffer in chain.iter() {
+            for ((key, val), time, _) in buffer.iter() {
+                if !upper.less_equal(time) {
+                    if let Some((p_key, p_val)) = prev_keyval {
+                        if p_key != key {
+                            keys += 1;
+                            vals += 1;
+                        } else if p_val != val {
+                            vals += 1;
+                        }
+                    } else {
+                        keys += 1;
+                        vals += 1;
+                    }
+                    upds += 1;
+                    prev_keyval = Some((key, val));
+                }
             }
-            logger.log(BatcherEvent {
-                operator: self.operator_id,
-                records_diff: records * diff,
-                size_diff: 0,
-                capacity_diff: 0,
-                allocations_diff: 0,
-            })
         }
+        let mut builder = B::with_capacity(keys, vals, upds);
+
+        for datum in chain.drain(..).flatten() {
+            builder.push(datum);
+        }
+
+        builder.done(lower.to_owned(), upper.to_owned(), since.to_owned())
     }
-}
 
-impl<D, T, R> Drop for MergeSorter<D, T, R> {
-    fn drop(&mut self) {
-        while self.queue_pop().is_some() { }
+    fn account(chunk: &Self::Chunk) -> (usize, usize, usize, usize) {
+        (chunk.len(), 0, 0, 0)
     }
 }
diff --git a/src/trace/implementations/merge_batcher_col.rs b/src/trace/implementations/merge_batcher_col.rs
index 21416170d..72ec79efa 100644
--- a/src/trace/implementations/merge_batcher_col.rs
+++ b/src/trace/implementations/merge_batcher_col.rs
@@ -1,210 +1,31 @@
 //! A general purpose `Batcher` implementation based on radix sort for TimelyStack.
 
-use timely::Container;
+use crate::consolidation::consolidate_updates;
+use std::cmp::Ordering;
 use timely::communication::message::RefOrMut;
 use timely::container::columnation::{Columnation, TimelyStack};
-use timely::logging::WorkerIdentifier;
-use timely::logging_core::Logger;
-use timely::progress::{frontier::Antichain, Timestamp};
+use timely::progress::frontier::{Antichain, AntichainRef};
+use timely::{Container, Data, PartialOrder};
 
 use crate::difference::Semigroup;
-use crate::logging::{BatcherEvent, DifferentialEvent};
-use crate::trace::{Batcher, Builder};
+use crate::trace::implementations::merge_batcher::Merger;
+use crate::trace::Builder;
 
-/// Creates batches from unordered tuples.
-pub struct ColumnatedMergeBatcher<K, V, T, D>
-where
-    K: Columnation + 'static,
-    V: Columnation + 'static,
-    T: Columnation + 'static,
-    D: Columnation + 'static,
-{
-    sorter: MergeSorterColumnation<(K, V), T, D>,
-    lower: Antichain<T>,
-    frontier: Antichain<T>,
+/// A merger for timely stacks
+pub struct ColumnationMerger<T> {
+    pending: Vec<T>,
 }
 
-impl<K, V, T, D> Batcher for ColumnatedMergeBatcher<K, V, T, D>
-where
-    K: Columnation + Ord + Clone + 'static,
-    V: Columnation + Ord + Clone + 'static,
-    T: Columnation + Timestamp + 'static,
-    D: Columnation + Semigroup + 'static,
-{
-    type Input = Vec<((K,V),T,D)>;
-    type Output = ((K,V),T,D);
-    type Time = T;
-
-    fn new(logger: Option<Logger<DifferentialEvent, WorkerIdentifier>>, operator_id: usize) -> Self {
-        ColumnatedMergeBatcher {
-            sorter: MergeSorterColumnation::new(logger, operator_id),
-            frontier: Antichain::new(),
-            lower: Antichain::from_elem(<T as Timestamp>::minimum()),
-        }
-    }
-
-    #[inline]
-    fn push_batch(&mut self, batch: RefOrMut<Self::Input>) {
-        // `batch` is either a shared reference or an owned allocations.
-        match batch {
-            RefOrMut::Ref(reference) => {
-                // This is a moment at which we could capture the allocations backing
-                // `batch` into a different form of region, rather than just  cloning.
-                self.sorter.push(&mut reference.clone());
-            },
-            RefOrMut::Mut(reference) => {
-                self.sorter.push(reference);
-            }
-        }
-    }
-
-    // Sealing a batch means finding those updates with times not greater or equal to any time
-    // in `upper`. All updates must have time greater or equal to the previously used `upper`,
-    // which we call `lower`, by assumption that after sealing a batcher we receive no more
-    // updates with times not greater or equal to `upper`.
-    #[inline]
-    fn seal<B: Builder<Input=Self::Output, Time=Self::Time>>(&mut self, upper: Antichain<T>) -> B::Output {
-
-        let mut merged = Default::default();
-        self.sorter.finish_into(&mut merged);
-
-        // Determine the number of distinct keys, values, and updates,
-        // and form a builder pre-sized for these numbers.
-        let mut builder = {
-            let mut keys = 0;
-            let mut vals = 0;
-            let mut upds = 0;
-            let mut prev_keyval = None;
-            for buffer in merged.iter() {
-                for ((key, val), time, _) in buffer.iter() {
-                    if !upper.less_equal(time) {
-                        if let Some((p_key, p_val)) = prev_keyval {
-                            if p_key != key {
-                                keys += 1;
-                                vals += 1;
-                            }
-                            else if p_val != val {
-                                vals += 1;
-                            }
-                            upds += 1;
-                        } else {
-                            keys += 1;
-                            vals += 1;
-                            upds += 1;
-                        }
-                        prev_keyval = Some((key, val));
-                    }
-                }
-            }
-            B::with_capacity(keys, vals, upds)
-        };
-
-        let mut kept = Vec::new();
-        let mut keep = TimelyStack::default();
-
-        self.frontier.clear();
-
-        for buffer in merged.drain(..) {
-            for datum @ ((_key, _val), time, _diff) in &buffer[..] {
-                if upper.less_equal(time) {
-                    self.frontier.insert(time.clone());
-                    if keep.is_empty() {
-                        if keep.capacity() != MergeSorterColumnation::<(K, V), T, D>::buffer_size() {
-                            keep = self.sorter.empty();
-                        }
-                    } else if keep.len() == keep.capacity() {
-                        kept.push(keep);
-                        keep = self.sorter.empty();
-                    }
-                    keep.copy(datum);
-                }
-                else {
-                    builder.copy(datum);
-                }
-            }
-            // Recycling buffer.
-            self.sorter.recycle(buffer);
-        }
-
-        // Finish the kept data.
-        if !keep.is_empty() {
-            kept.push(keep);
-        }
-        if !kept.is_empty() {
-            self.sorter.push_list(kept);
-        }
-
-        // Drain buffers (fast reclamation).
-        self.sorter.clear_stash();
-
-        let seal = builder.done(self.lower.clone(), upper.clone(), Antichain::from_elem(T::minimum()));
-        self.lower = upper;
-        seal
-    }
-
-    /// The frontier of elements remaining after the most recent call to `self.seal`.
-    fn frontier(&mut self) -> timely::progress::frontier::AntichainRef<T> {
-        self.frontier.borrow()
-    }
-}
-
-struct TimelyStackQueue<T: Columnation> {
-    list: TimelyStack<T>,
-    head: usize,
-}
-
-impl<T: Columnation> Default for TimelyStackQueue<T> {
+impl<T> Default for ColumnationMerger<T> {
     fn default() -> Self {
-        Self::from(Default::default())
-    }
-}
-
-impl<T: Columnation> TimelyStackQueue<T> {
-
-    fn pop(&mut self) -> &T {
-        self.head += 1;
-        &self.list[self.head - 1]
-    }
-
-    fn peek(&self) -> &T {
-        &self.list[self.head]
-    }
-
-    fn from(list: TimelyStack<T>) -> Self {
-        TimelyStackQueue {
-            list,
-            head: 0,
-        }
-    }
-
-    fn done(self) -> TimelyStack<T> {
-        self.list
-    }
-
-    fn is_empty(&self) -> bool { self.head == self.list[..].len() }
-
-    /// Return an iterator over the remaining elements.
-    fn iter(&self) -> impl Iterator<Item=&T> + Clone + ExactSizeIterator {
-        self.list[self.head..].iter()
+        Self { pending: Vec::default() }
     }
 }
 
-struct MergeSorterColumnation<D: Columnation + 'static, T: Columnation + 'static, R: Columnation + 'static> {
-    /// each power-of-two length list of allocations. Do not push/pop directly but use the corresponding functions.
-    queue: Vec<Vec<TimelyStack<(D, T, R)>>>,
-    stash: Vec<TimelyStack<(D, T, R)>>,
-    pending: Vec<(D, T, R)>,
-    logger: Option<Logger<DifferentialEvent, WorkerIdentifier>>,
-    operator_id: usize,
-}
-
-impl<D: Ord+Columnation+'static, T: Ord+Columnation+'static, R: Semigroup+Columnation+'static> MergeSorterColumnation<D, T, R> {
-
+impl<T: Columnation> ColumnationMerger<T> {
     const BUFFER_SIZE_BYTES: usize = 64 << 10;
-
-    /// Buffer size (number of elements) to use for new/empty buffers.
-    const fn buffer_size() -> usize {
-        let size = std::mem::size_of::<(D, T, R)>();
+    fn chunk_capacity(&self) -> usize {
+        let size = ::std::mem::size_of::<T>();
         if size == 0 {
             Self::BUFFER_SIZE_BYTES
         } else if size <= Self::BUFFER_SIZE_BYTES {
@@ -214,130 +35,137 @@ impl<D: Ord+Columnation+'static, T: Ord+Columnation+'static, R: Semigroup+Column
         }
     }
 
-    /// Buffer size for pending updates, currently 2 * [`Self::buffer_size`].
-    const fn pending_buffer_size() -> usize {
-        Self::buffer_size() * 2
+    /// Buffer size for pending updates, currently 2 * [`Self::chunk_capacity`].
+    fn pending_capacity(&self) -> usize {
+        self.chunk_capacity() * 2
     }
 
-    fn new(logger: Option<Logger<DifferentialEvent, WorkerIdentifier>>, operator_id: usize) -> Self {
-        Self {
-            logger,
-            operator_id,
-            queue: Vec::new(),
-            stash: Vec::new(),
-            pending: Vec::new(),
-        }
-    }
-
-    fn empty(&mut self) -> TimelyStack<(D, T, R)> {
-        self.stash.pop().unwrap_or_else(|| TimelyStack::with_capacity(Self::buffer_size()))
-    }
-
-    /// Remove all elements from the stash.
-    fn clear_stash(&mut self) {
-        self.stash.clear();
+    /// Helper to get pre-sized vector from the stash.
+    #[inline]
+    fn empty(&self, stash: &mut Vec<TimelyStack<T>>) -> TimelyStack<T> {
+        stash.pop().unwrap_or_else(|| TimelyStack::with_capacity(self.chunk_capacity()))
     }
 
-    /// Insert an empty buffer into the stash. Panics if the buffer is not empty.
-    fn recycle(&mut self, mut buffer: TimelyStack<(D, T, R)>) {
-        if buffer.capacity() == Self::buffer_size() && self.stash.len() < 2 {
-            buffer.clear();
-            self.stash.push(buffer);
+    /// Helper to return a chunk to the stash.
+    #[inline]
+    fn recycle(&self, mut chunk: TimelyStack<T>, stash: &mut Vec<TimelyStack<T>>) {
+        // TODO: Should we limit the size of `stash`?
+        if chunk.capacity() == self.chunk_capacity() {
+            chunk.clear();
+            stash.push(chunk);
         }
     }
+}
 
-    fn push(&mut self, batch: &mut Vec<(D, T, R)>) {
-        // Ensure `self.pending` has a capacity of `Self::pending_buffer_size`.
-        if self.pending.capacity() < Self::pending_buffer_size() {
-            self.pending.reserve(Self::pending_buffer_size() - self.pending.capacity());
+impl<K, V, T, R> Merger for ColumnationMerger<((K, V), T, R)>
+where
+    K: Columnation + Ord + Data,
+    V: Columnation + Ord + Data,
+    T: Columnation + Ord + PartialOrder + Data,
+    R: Columnation + Semigroup + 'static,
+{
+    type Time = T;
+    type Input = Vec<((K, V), T, R)>;
+    type Chunk = TimelyStack<((K, V), T, R)>;
+    type Output = ((K, V), T, R);
+
+    fn accept(&mut self, container: RefOrMut<Self::Input>, stash: &mut Vec<Self::Chunk>) -> Vec<Self::Chunk> {
+        // Ensure `self.pending` has the desired capacity. We should never have a larger capacity
+        // because we don't write more than capacity elements into the buffer.
+        if self.pending.capacity() < self.pending_capacity() {
+            self.pending.reserve(self.pending_capacity() - self.pending.len());
         }
 
-        while !batch.is_empty() {
-            self.pending.extend(batch.drain(..std::cmp::min(batch.len(), self.pending.capacity() - self.pending.len())));
-            if self.pending.len() == self.pending.capacity() {
-                crate::consolidation::consolidate_updates(&mut self.pending);
-                if self.pending.len() > self.pending.capacity() / 2 {
-                    // Flush if `self.pending` is more than half full after consolidation.
-                    self.flush_pending();
+        // Form a chain from what's in pending.
+        // This closure does the following:
+        // * If pending is full, consolidate.
+        // * If after consolidation it's more than half full, peel off a chain of full blocks,
+        //   leaving behind any partial block in pending.
+        // * Merge the new chain with `final_chain` and return it in-place.
+        let form_chain = |this: &mut Self, final_chain: &mut Vec<Self::Chunk>, stash: &mut _| {
+            if this.pending.len() == this.pending.capacity() {
+                consolidate_updates(&mut this.pending);
+                if this.pending.len() >= this.chunk_capacity() {
+                    let mut chain = Vec::default();
+                    while this.pending.len() > this.chunk_capacity() {
+                        let mut chunk = this.empty(stash);
+                        for datum in this.pending.drain(..chunk.capacity()) {
+                            chunk.copy(&datum);
+                        }
+                        chain.push(chunk);
+                    }
+                    if final_chain.is_empty() {
+                        *final_chain = chain;
+                    } else if !chain.is_empty() {
+                        let mut output = Vec::default();
+                        this.merge(std::mem::take(final_chain), chain, &mut output, stash);
+                        *final_chain = output;
+                    }
                 }
             }
-        }
-    }
+        };
 
-    /// Move all elements in `pending` into `queue`. The data in `pending` must be compacted and
-    /// sorted. After this function returns, `self.pending` is empty.
-    fn flush_pending(&mut self) {
-        if !self.pending.is_empty() {
-            let mut stack = self.empty();
-            stack.reserve_items(self.pending.iter());
-            for tuple in self.pending.drain(..) {
-                stack.copy(&tuple);
+        let mut final_chain = Vec::default();
+        // `container` is either a shared reference or an owned allocations.
+        match container {
+            RefOrMut::Ref(vec) => {
+                let mut slice = &vec[..];
+                while !slice.is_empty() {
+                    let (head, tail) = slice.split_at(std::cmp::min(self.pending.capacity() - self.pending.len(), slice.len()));
+                    slice = tail;
+                    self.pending.extend_from_slice(head);
+                    form_chain(self, &mut final_chain, stash);
+                }
             }
-            self.queue_push(vec![stack]);
-            while self.queue.len() > 1 && (self.queue[self.queue.len()-1].len() >= self.queue[self.queue.len()-2].len() / 2) {
-                let list1 = self.queue_pop().unwrap();
-                let list2 = self.queue_pop().unwrap();
-                let merged = self.merge_by(list1, list2);
-                self.queue_push(merged);
+            RefOrMut::Mut(vec) => {
+                while !vec.is_empty() {
+                    self.pending.extend(vec.drain(..std::cmp::min(self.pending.capacity() - self.pending.len(), vec.len())));
+                    form_chain(self, &mut final_chain, stash);
+                }
             }
         }
+        final_chain
     }
 
-    // This is awkward, because it isn't a power-of-two length any more, and we don't want
-    // to break it down to be so.
-    fn push_list(&mut self, list: Vec<TimelyStack<(D, T, R)>>) {
-        while self.queue.len() > 1 && self.queue[self.queue.len()-1].len() < list.len() {
-            let list1 = self.queue_pop().unwrap();
-            let list2 = self.queue_pop().unwrap();
-            let merged = self.merge_by(list1, list2);
-            self.queue_push(merged);
-        }
-        self.queue_push(list);
-    }
-
-    fn finish_into(&mut self, target: &mut Vec<TimelyStack<(D, T, R)>>) {
-        crate::consolidation::consolidate_updates(&mut self.pending);
-        self.flush_pending();
-        while self.queue.len() > 1 {
-            let list1 = self.queue_pop().unwrap();
-            let list2 = self.queue_pop().unwrap();
-            let merged = self.merge_by(list1, list2);
-            self.queue_push(merged);
-        }
-
-        if let Some(mut last) = self.queue_pop() {
-            std::mem::swap(&mut last, target);
+    fn finish(&mut self, stash: &mut Vec<Self::Chunk>) -> Vec<Self::Chunk> {
+        // Extract all data from `pending`.
+        consolidate_updates(&mut self.pending);
+        let mut chain = Vec::default();
+        while !self.pending.is_empty() {
+            let mut chunk = self.empty(stash);
+            for datum in self.pending.drain(..std::cmp::min(chunk.capacity(), self.pending.len())) {
+                chunk.copy(&datum);
+            }
+            chain.push(chunk);
         }
+        chain
     }
 
-    // merges two sorted input lists into one sorted output list.
-    fn merge_by(&mut self, list1: Vec<TimelyStack<(D, T, R)>>, list2: Vec<TimelyStack<(D, T, R)>>) -> Vec<TimelyStack<(D, T, R)>> {
-        use std::cmp::Ordering;
-
-        // TODO: `list1` and `list2` get dropped; would be better to reuse?
-        let mut output = Vec::with_capacity(list1.len() + list2.len());
-        let mut result = self.empty();
-
+    fn merge(&mut self, list1: Vec<Self::Chunk>, list2: Vec<Self::Chunk>, output: &mut Vec<Self::Chunk>, stash: &mut Vec<Self::Chunk>) {
         let mut list1 = list1.into_iter();
         let mut list2 = list2.into_iter();
 
         let mut head1 = TimelyStackQueue::from(list1.next().unwrap_or_default());
         let mut head2 = TimelyStackQueue::from(list2.next().unwrap_or_default());
 
+        let mut result = self.empty(stash);
+
         // while we have valid data in each input, merge.
         while !head1.is_empty() && !head2.is_empty() {
-
             while (result.capacity() - result.len()) > 0 && !head1.is_empty() && !head2.is_empty() {
-
                 let cmp = {
                     let x = head1.peek();
                     let y = head2.peek();
                     (&x.0, &x.1).cmp(&(&y.0, &y.1))
                 };
                 match cmp {
-                    Ordering::Less    => { result.copy(head1.pop()); }
-                    Ordering::Greater => { result.copy(head2.pop()); }
-                    Ordering::Equal   => {
+                    Ordering::Less => {
+                        result.copy(head1.pop());
+                    }
+                    Ordering::Greater => {
+                        result.copy(head2.pop());
+                    }
+                    Ordering::Equal => {
                         let (data1, time1, diff1) = head1.pop();
                         let (_data2, _time2, diff2) = head2.pop();
                         let mut diff1 = diff1.clone();
@@ -351,15 +179,15 @@ impl<D: Ord+Columnation+'static, T: Ord+Columnation+'static, R: Semigroup+Column
 
             if result.capacity() == result.len() {
                 output.push(result);
-                result = self.empty();
+                result = self.empty(stash);
             }
 
             if head1.is_empty() {
-                self.recycle(head1.done());
+                self.recycle(head1.done(), stash);
                 head1 = TimelyStackQueue::from(list1.next().unwrap_or_default());
             }
             if head2.is_empty() {
-                self.recycle(head2.done());
+                self.recycle(head2.done(), stash);
                 head2 = TimelyStackQueue::from(list2.next().unwrap_or_default());
             }
         }
@@ -367,74 +195,156 @@ impl<D: Ord+Columnation+'static, T: Ord+Columnation+'static, R: Semigroup+Column
         if result.len() > 0 {
             output.push(result);
         } else {
-            self.recycle(result);
+            self.recycle(result, stash);
         }
 
         if !head1.is_empty() {
-            let mut result = self.empty();
+            let mut result = self.empty(stash);
             result.reserve_items(head1.iter());
-            for item in head1.iter() { result.copy(item); }
+            for item in head1.iter() {
+                result.copy(item);
+            }
             output.push(result);
         }
         output.extend(list1);
 
         if !head2.is_empty() {
-            let mut result = self.empty();
+            let mut result = self.empty(stash);
             result.reserve_items(head2.iter());
-            for item in head2.iter() { result.copy(item); }
+            for item in head2.iter() {
+                result.copy(item);
+            }
             output.push(result);
         }
         output.extend(list2);
+    }
+
+    fn extract(
+        &mut self,
+        merged: Vec<Self::Chunk>,
+        upper: AntichainRef<Self::Time>,
+        frontier: &mut Antichain<Self::Time>,
+        readied: &mut Vec<Self::Chunk>,
+        kept: &mut Vec<Self::Chunk>,
+        stash: &mut Vec<Self::Chunk>,
+    ) {
+        let mut keep = self.empty(stash);
+        let mut ready = self.empty(stash);
+
+        for buffer in merged {
+            for d @ (_data, time, _diff) in buffer.iter() {
+                if upper.less_equal(time) {
+                    frontier.insert_ref(time);
+                    if keep.len() == keep.capacity() && !keep.is_empty() {
+                        kept.push(keep);
+                        keep = self.empty(stash);
+                    }
+                    keep.copy(d);
+                } else {
+                    if ready.len() == ready.capacity() && !ready.is_empty() {
+                        readied.push(ready);
+                        ready = self.empty(stash);
+                    }
+                    ready.copy(d);
+                }
+            }
+            // Recycling buffer.
+            self.recycle(buffer, stash);
+        }
+        // Finish the kept data.
+        if !keep.is_empty() {
+            kept.push(keep);
+        }
+        if !ready.is_empty() {
+            readied.push(ready);
+        }
+    }
 
-        output
+    fn seal<B: Builder<Input = Self::Output, Time = Self::Time>>(
+        chain: &mut Vec<Self::Chunk>,
+        lower: AntichainRef<Self::Time>,
+        upper: AntichainRef<Self::Time>,
+        since: AntichainRef<Self::Time>,
+    ) -> B::Output {
+        let mut keys = 0;
+        let mut vals = 0;
+        let mut upds = 0;
+        let mut prev_keyval = None;
+        for buffer in chain.iter() {
+            for ((key, val), time, _) in buffer.iter() {
+                if !upper.less_equal(time) {
+                    if let Some((p_key, p_val)) = prev_keyval {
+                        if p_key != key {
+                            keys += 1;
+                            vals += 1;
+                        } else if p_val != val {
+                            vals += 1;
+                        }
+                    } else {
+                        keys += 1;
+                        vals += 1;
+                    }
+                    upds += 1;
+                    prev_keyval = Some((key, val));
+                }
+            }
+        }
+        let mut builder = B::with_capacity(keys, vals, upds);
+
+        for datum in chain.iter().flat_map(|ts| ts.iter()) {
+            builder.copy(datum);
+        }
+
+        builder.done(lower.to_owned(), upper.to_owned(), since.to_owned())
+    }
+
+    fn account(chunk: &Self::Chunk) -> (usize, usize, usize, usize) {
+        let (mut size, mut capacity, mut allocations) = (0, 0, 0);
+        let cb = |siz, cap| {
+            size += siz;
+            capacity += cap;
+            allocations += 1;
+        };
+        chunk.heap_size(cb);
+        (chunk.len(), size, capacity, allocations)
     }
 }
 
-impl<D: Columnation + 'static, T: Columnation + 'static, R: Columnation + 'static> MergeSorterColumnation<D, T, R> {
-    /// Pop a batch from `self.queue` and account size changes.
-    #[inline]
-    fn queue_pop(&mut self) -> Option<Vec<TimelyStack<(D, T, R)>>> {
-        let batch = self.queue.pop();
-        self.account(batch.iter().flatten(), -1);
-        batch
+struct TimelyStackQueue<T: Columnation> {
+    list: TimelyStack<T>,
+    head: usize,
+}
+
+impl<T: Columnation> Default for TimelyStackQueue<T> {
+    fn default() -> Self {
+        Self::from(Default::default())
     }
+}
 
-    /// Push a batch to `self.queue` and account size changes.
-    #[inline]
-    fn queue_push(&mut self, batch: Vec<TimelyStack<(D, T, R)>>) {
-        self.account(&batch, 1);
-        self.queue.push(batch);
+impl<T: Columnation> TimelyStackQueue<T> {
+    fn pop(&mut self) -> &T {
+        self.head += 1;
+        &self.list[self.head - 1]
     }
 
-    /// Account size changes. Only performs work if a logger exists.
-    ///
-    /// Calculate the size based on the [`TimelyStack`]s passed along, with each attribute
-    /// multiplied by `diff`. Usually, one wants to pass 1 or -1 as the diff.
-    fn account<'a, I: IntoIterator<Item=&'a TimelyStack<(D, T, R)>>>(&self, items: I, diff: isize) {
-        if let Some(logger) = &self.logger {
-            let (mut records, mut siz, mut capacity, mut allocations) = (0isize, 0isize, 0isize, 0isize);
-            for stack in items {
-                records = records.saturating_add_unsigned(stack.len());
-                stack.heap_size(|s, c| {
-                    siz = siz.saturating_add_unsigned(s);
-                    capacity = capacity.saturating_add_unsigned(c);
-                    allocations += isize::from(c > 0);
-                });
-            }
-            logger.log(BatcherEvent {
-                operator: self.operator_id,
-                records_diff: records * diff,
-                size_diff: siz * diff,
-                capacity_diff: capacity * diff,
-                allocations_diff: allocations * diff,
-            })
-        }
+    fn peek(&self) -> &T {
+        &self.list[self.head]
     }
 
-}
+    fn from(list: TimelyStack<T>) -> Self {
+        TimelyStackQueue { list, head: 0 }
+    }
 
-impl<D: Columnation + 'static, T: Columnation + 'static, R: Columnation + 'static> Drop for MergeSorterColumnation<D, T, R> {
-    fn drop(&mut self) {
-        while self.queue_pop().is_some() { }
+    fn done(self) -> TimelyStack<T> {
+        self.list
+    }
+
+    fn is_empty(&self) -> bool {
+        self.head == self.list[..].len()
+    }
+
+    /// Return an iterator over the remaining elements.
+    fn iter(&self) -> impl Iterator<Item = &T> + Clone {
+        self.list[self.head..].iter()
     }
 }
diff --git a/src/trace/implementations/mod.rs b/src/trace/implementations/mod.rs
index d700f2b6b..cd0b8fd9a 100644
--- a/src/trace/implementations/mod.rs
+++ b/src/trace/implementations/mod.rs
@@ -42,9 +42,6 @@ pub mod spine_fueled;
 
 pub mod merge_batcher;
 pub mod merge_batcher_col;
-
-pub use self::merge_batcher::MergeBatcher as Batcher;
-
 pub mod ord_neu;
 pub mod rhh;
 pub mod huffman_container;
diff --git a/src/trace/implementations/ord_neu.rs b/src/trace/implementations/ord_neu.rs
index 42e45aefe..a5afee109 100644
--- a/src/trace/implementations/ord_neu.rs
+++ b/src/trace/implementations/ord_neu.rs
@@ -11,8 +11,8 @@
 use std::rc::Rc;
 
 use crate::trace::implementations::spine_fueled::Spine;
-use crate::trace::implementations::merge_batcher::MergeBatcher;
-use crate::trace::implementations::merge_batcher_col::ColumnatedMergeBatcher;
+use crate::trace::implementations::merge_batcher::{MergeBatcher, VecMerger};
+use crate::trace::implementations::merge_batcher_col::ColumnationMerger;
 use crate::trace::rc_blanket_impls::RcBuilder;
 
 use super::{Update, Layout, Vector, TStack, Preferred};
@@ -23,7 +23,7 @@ pub use self::key_batch::{OrdKeyBatch, OrdKeyBuilder};
 /// A trace implementation using a spine of ordered lists.
 pub type OrdValSpine<K, V, T, R> = Spine<
     Rc<OrdValBatch<Vector<((K,V),T,R)>>>,
-    MergeBatcher<K,V,T,R>,
+    MergeBatcher<VecMerger<((K, V), T, R)>, T>,
     RcBuilder<OrdValBuilder<Vector<((K,V),T,R)>>>,
 >;
 // /// A trace implementation for empty values using a spine of ordered lists.
@@ -32,14 +32,14 @@ pub type OrdValSpine<K, V, T, R> = Spine<
 /// A trace implementation backed by columnar storage.
 pub type ColValSpine<K, V, T, R> = Spine<
     Rc<OrdValBatch<TStack<((K,V),T,R)>>>,
-    ColumnatedMergeBatcher<K,V,T,R>,
+    MergeBatcher<ColumnationMerger<((K,V),T,R)>, T>,
     RcBuilder<OrdValBuilder<TStack<((K,V),T,R)>>>,
 >;
 
 /// A trace implementation using a spine of ordered lists.
 pub type OrdKeySpine<K, T, R> = Spine<
     Rc<OrdKeyBatch<Vector<((K,()),T,R)>>>,
-    MergeBatcher<K,(),T,R>,
+    MergeBatcher<VecMerger<((K, ()), T, R)>, T>,
     RcBuilder<OrdKeyBuilder<Vector<((K,()),T,R)>>>,
 >;
 // /// A trace implementation for empty values using a spine of ordered lists.
@@ -48,14 +48,14 @@ pub type OrdKeySpine<K, T, R> = Spine<
 /// A trace implementation backed by columnar storage.
 pub type ColKeySpine<K, T, R> = Spine<
     Rc<OrdKeyBatch<TStack<((K,()),T,R)>>>,
-    ColumnatedMergeBatcher<K,(),T,R>,
+    MergeBatcher<ColumnationMerger<((K,()),T,R)>, T>,
     RcBuilder<OrdKeyBuilder<TStack<((K,()),T,R)>>>,
 >;
 
 /// A trace implementation backed by columnar storage.
 pub type PreferredSpine<K, V, T, R> = Spine<
     Rc<OrdValBatch<Preferred<K,V,T,R>>>,
-    ColumnatedMergeBatcher<<K as ToOwned>::Owned,<V as ToOwned>::Owned,T,R>,
+    MergeBatcher<ColumnationMerger<((<K as ToOwned>::Owned,<V as ToOwned>::Owned),T,R)>,T>,
     RcBuilder<OrdValBuilder<Preferred<K,V,T,R>>>,
 >;
 
diff --git a/src/trace/implementations/rhh.rs b/src/trace/implementations/rhh.rs
index b01d1dae3..64cac268b 100644
--- a/src/trace/implementations/rhh.rs
+++ b/src/trace/implementations/rhh.rs
@@ -6,11 +6,14 @@
 //! for example wrapped types that implement `Ord` that way.
 
 use std::rc::Rc;
+use std::cmp::Ordering;
+
+use abomonation_derive::Abomonation;
 
 use crate::Hashable;
+use crate::trace::implementations::merge_batcher::{MergeBatcher, VecMerger};
+use crate::trace::implementations::merge_batcher_col::ColumnationMerger;
 use crate::trace::implementations::spine_fueled::Spine;
-use crate::trace::implementations::merge_batcher::MergeBatcher;
-use crate::trace::implementations::merge_batcher_col::ColumnatedMergeBatcher;
 use crate::trace::rc_blanket_impls::RcBuilder;
 
 use super::{Update, Layout, Vector, TStack};
@@ -20,7 +23,7 @@ use self::val_batch::{RhhValBatch, RhhValBuilder};
 /// A trace implementation using a spine of ordered lists.
 pub type VecSpine<K, V, T, R> = Spine<
     Rc<RhhValBatch<Vector<((K,V),T,R)>>>,
-    MergeBatcher<K,V,T,R>,
+    MergeBatcher<VecMerger<((K, V), T, R)>, T>,
     RcBuilder<RhhValBuilder<Vector<((K,V),T,R)>>>,
 >;
 // /// A trace implementation for empty values using a spine of ordered lists.
@@ -29,7 +32,7 @@ pub type VecSpine<K, V, T, R> = Spine<
 /// A trace implementation backed by columnar storage.
 pub type ColSpine<K, V, T, R> = Spine<
     Rc<RhhValBatch<TStack<((K,V),T,R)>>>,
-    ColumnatedMergeBatcher<K,V,T,R>,
+    MergeBatcher<ColumnationMerger<((K,V),T,R)>, T>,
     RcBuilder<RhhValBuilder<TStack<((K,V),T,R)>>>,
 >;
 // /// A trace implementation backed by columnar storage.
@@ -47,9 +50,6 @@ pub struct HashWrapper<T: std::hash::Hash + Hashable> {
     pub inner: T
 }
 
-use std::cmp::Ordering;
-use abomonation_derive::Abomonation;
-
 impl<T: PartialOrd + std::hash::Hash + Hashable> PartialOrd for HashWrapper<T>
 where <T as Hashable>::Output: PartialOrd {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
@@ -839,4 +839,4 @@ mod key_batch {
 
     // Copy the above, once it works!
 
-}
\ No newline at end of file
+}
diff --git a/src/trace/mod.rs b/src/trace/mod.rs
index d1f878342..afee7c22e 100644
--- a/src/trace/mod.rs
+++ b/src/trace/mod.rs
@@ -310,8 +310,8 @@ pub trait Batcher {
     type Time: Timestamp;
     /// Allocates a new empty batcher.
     fn new(logger: Option<Logger<DifferentialEvent, WorkerIdentifier>>, operator_id: usize) -> Self;
-    /// Adds an unordered batch of elements to the batcher.
-    fn push_batch(&mut self, batch: RefOrMut<Self::Input>);
+    /// Adds an unordered container of elements to the batcher.
+    fn push_container(&mut self, batch: RefOrMut<Self::Input>);
     /// Returns all updates not greater or equal to an element of `upper`.
     fn seal<B: Builder<Input=Self::Output, Time=Self::Time>>(&mut self, upper: Antichain<Self::Time>) -> B::Output;
     /// Returns the lower envelope of contained update times.
@@ -326,7 +326,7 @@ pub trait Builder: Sized {
     type Time: Timestamp;
     /// Output batch type.
     type Output;
-    
+
     /// Allocates an empty builder.
     ///
     /// Ideally we deprecate this and insist all non-trivial building happens via `with_capacity()`.
diff --git a/tests/trace.rs b/tests/trace.rs
index a637b1700..acf1db350 100644
--- a/tests/trace.rs
+++ b/tests/trace.rs
@@ -15,7 +15,7 @@ fn get_trace() -> ValSpine<u64, u64, usize, i64> {
         let mut batcher = <IntegerTrace as Trace>::Batcher::new(None, 0);
 
         use timely::communication::message::RefOrMut;
-        batcher.push_batch(RefOrMut::Mut(&mut vec![
+        batcher.push_container(RefOrMut::Mut(&mut vec![
             ((1, 2), 0, 1),
             ((2, 3), 1, 1),
             ((2, 3), 2, -1),

From 1eb052cbfd0e7295ee078b42432d5580db0cfed2 Mon Sep 17 00:00:00 2001
From: Moritz Hoffmann <antiguru@gmail.com>
Date: Tue, 30 Apr 2024 17:30:15 -0400
Subject: [PATCH 4/5] `join_traces` uses timely's container builder (#478)

join_traces can produce arbitrary containers.

---------

Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>
---
 examples/spines.rs                   |   4 +-
 server/Cargo.toml                    |   2 +-
 src/capture.rs                       |   2 +-
 src/consolidation.rs                 | 123 +++++++++++++++++++++++++++
 src/dynamic/mod.rs                   |   2 +-
 src/operators/arrange/arrangement.rs |  12 ++-
 src/operators/consolidate.rs         |   2 +-
 src/operators/join.rs                |  99 +++++++++++----------
 8 files changed, 193 insertions(+), 53 deletions(-)

diff --git a/examples/spines.rs b/examples/spines.rs
index d4d2e64bc..4ae540e5d 100644
--- a/examples/spines.rs
+++ b/examples/spines.rs
@@ -55,11 +55,11 @@ fn main() {
                     let data =
                     data.map(|x| (x.clone().into_bytes(), x.into_bytes()))
                         .arrange::<PreferredSpine<[u8],[u8],_,_>>()
-                        .reduce_abelian::<_, _, _, PreferredSpine<[u8],(),_,_>>("distinct", |v| v.clone(), |_,_,output| output.push(((), 1)));
+                        .reduce_abelian::<_, _, _, PreferredSpine<[u8],(),_,_>>("distinct", |_| (), |_,_,output| output.push(((), 1)));
                     let keys =
                     keys.map(|x| (x.clone().into_bytes(), 7))
                         .arrange::<PreferredSpine<[u8],u8,_,_>>()
-                        .reduce_abelian::<_, _, _, PreferredSpine<[u8],(),_,_>>("distinct", |v| v.clone(), |_,_,output| output.push(((), 1)));
+                        .reduce_abelian::<_, _, _, PreferredSpine<[u8],(),_,_>>("distinct", |_| (), |_,_,output| output.push(((), 1)));
 
                     keys.join_core(&data, |k,_v1,_v2| {
                         println!("{:?}", k.text);
diff --git a/server/Cargo.toml b/server/Cargo.toml
index 364b2ae51..ed79bb187 100644
--- a/server/Cargo.toml
+++ b/server/Cargo.toml
@@ -10,7 +10,7 @@ path="../"
 [dependencies]
 rand="0.3.13"
 libloading="*"
-timely = { git = "https://github.com/TimelyDataflow/timely-dataflow" }
+timely = { workspace = true }
 
 #[workspace]
 #members = [
diff --git a/src/capture.rs b/src/capture.rs
index 0a8cd03aa..33f706b85 100644
--- a/src/capture.rs
+++ b/src/capture.rs
@@ -399,7 +399,7 @@ pub mod source {
                     for message in source.by_ref() {
                         match message {
                             Message::Updates(mut updates) => {
-                                updates_session.give_vec(&mut updates);
+                                updates_session.give_container(&mut updates);
                             }
                             Message::Progress(progress) => {
                                 // We must send a copy of each progress message to all workers,
diff --git a/src/consolidation.rs b/src/consolidation.rs
index b4087bcc9..637df0eff 100644
--- a/src/consolidation.rs
+++ b/src/consolidation.rs
@@ -10,6 +10,9 @@
 //! you need specific behavior, it may be best to defensively copy, paste, and maintain the
 //! specific behavior you require.
 
+use std::collections::VecDeque;
+use timely::container::{ContainerBuilder, PushContainer, PushInto};
+use crate::Data;
 use crate::difference::Semigroup;
 
 /// Sorts and consolidates `vec`.
@@ -145,6 +148,84 @@ pub fn consolidate_updates_slice<D: Ord, T: Ord, R: Semigroup>(slice: &mut [(D,
     offset
 }
 
+/// A container builder that consolidates data in-places into fixed-sized containers. Does not
+/// maintain FIFO ordering.
+#[derive(Default)]
+pub struct ConsolidatingContainerBuilder<C>{
+    current: C,
+    empty: Vec<C>,
+    outbound: VecDeque<C>,
+}
+
+impl<D,T,R> ConsolidatingContainerBuilder<Vec<(D, T, R)>>
+where
+    D: Data,
+    T: Data,
+    R: Semigroup,
+{
+    /// Flush `self.current` up to the biggest `multiple` of elements. Pass 1 to flush all elements.
+    // TODO: Can we replace `multiple` by a bool?
+    fn consolidate_and_flush_through(&mut self, multiple: usize) {
+        let preferred_capacity = <Vec<(D,T,R)>>::preferred_capacity();
+        consolidate_updates(&mut self.current);
+        let mut drain = self.current.drain(..(self.current.len()/multiple)*multiple).peekable();
+        while drain.peek().is_some() {
+            let mut container = self.empty.pop().unwrap_or_else(|| Vec::with_capacity(preferred_capacity));
+            container.extend((&mut drain).take(preferred_capacity));
+            self.outbound.push_back(container);
+        }
+    }
+}
+
+impl<D,T,R> ContainerBuilder for ConsolidatingContainerBuilder<Vec<(D, T, R)>>
+where
+    D: Data,
+    T: Data,
+    R: Semigroup,
+{
+    type Container = Vec<(D,T,R)>;
+
+    /// Push an element.
+    ///
+    /// Precondition: `current` is not allocated or has space for at least one element.
+    #[inline]
+    fn push<P: PushInto<Self::Container>>(&mut self, item: P) {
+        let preferred_capacity = <Vec<(D,T,R)>>::preferred_capacity();
+        if self.current.capacity() < preferred_capacity * 2 {
+            self.current.reserve(preferred_capacity * 2 - self.current.capacity());
+        }
+        item.push_into(&mut self.current);
+        if self.current.len() == self.current.capacity() {
+            // Flush complete containers.
+            self.consolidate_and_flush_through(preferred_capacity);
+        }
+    }
+
+    fn push_container(&mut self, container: &mut Self::Container) {
+        for item in container.drain(..) {
+            self.push(item);
+        }
+    }
+
+    fn extract(&mut self) -> Option<&mut Vec<(D,T,R)>> {
+        if let Some(container) = self.outbound.pop_front() {
+            self.empty.push(container);
+            self.empty.last_mut()
+        } else {
+            None
+        }
+    }
+
+    fn finish(&mut self) -> Option<&mut Vec<(D,T,R)>> {
+        // Flush all
+        self.consolidate_and_flush_through(1);
+        // Remove all but two elements from the stash of empty to avoid memory leaks. We retain
+        // two to match `current` capacity.
+        self.empty.truncate(2);
+        self.extract()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -211,4 +292,46 @@ mod tests {
             assert_eq!(input, output);
         }
     }
+
+    #[test]
+    fn test_consolidating_container_builder() {
+        let mut ccb = <ConsolidatingContainerBuilder<Vec<(usize, usize, usize)>>>::default();
+        for _ in 0..1024 {
+            ccb.push((0, 0, 0));
+        }
+        assert_eq!(ccb.extract(), None);
+        assert_eq!(ccb.finish(), None);
+
+        for i in 0..1024 {
+            ccb.push((i, 0, 1));
+        }
+
+        let mut collected = Vec::default();
+        while let Some(container) = ccb.finish() {
+            collected.append(container);
+        }
+        // The output happens to be sorted, but it's not guaranteed.
+        collected.sort();
+        for i in 0..1024 {
+            assert_eq!((i, 0, 1), collected[i]);
+        }
+
+        ccb = Default::default();
+        ccb.push_container(&mut Vec::default());
+        assert_eq!(ccb.extract(), None);
+        assert_eq!(ccb.finish(), None);
+
+        ccb.push_container(&mut Vec::from_iter((0..1024).map(|i| (i, 0, 1))));
+        ccb.push_container(&mut Vec::from_iter((0..1024).map(|i| (i, 0, 1))));
+        collected.clear();
+        while let Some(container) = ccb.finish() {
+            collected.append(container);
+        }
+        // The output happens to be sorted, but it's not guaranteed.
+        consolidate_updates(&mut collected);
+        for i in 0..1024 {
+            assert_eq!((i, 0, 2), collected[i]);
+        }
+
+    }
 }
diff --git a/src/dynamic/mod.rs b/src/dynamic/mod.rs
index 4d1eeff43..18208d3ae 100644
--- a/src/dynamic/mod.rs
+++ b/src/dynamic/mod.rs
@@ -60,7 +60,7 @@ where
                     vec.truncate(level - 1);
                     time.inner = PointStamp::new(vec);
                 }
-                output.session(&new_cap).give_vec(&mut vector);
+                output.session(&new_cap).give_container(&mut vector);
             });
         });
 
diff --git a/src/operators/arrange/arrangement.rs b/src/operators/arrange/arrangement.rs
index ba7bf2045..13bbb915b 100644
--- a/src/operators/arrange/arrangement.rs
+++ b/src/operators/arrange/arrangement.rs
@@ -255,7 +255,7 @@ where
         self.join_core_internal_unsafe(other, result)
     }
     /// A direct implementation of the `JoinCore::join_core_internal_unsafe` method.
-    pub fn join_core_internal_unsafe<T2,I,L,D,ROut> (&self, other: &Arranged<G,T2>, result: L) -> Collection<G,D,ROut>
+    pub fn join_core_internal_unsafe<T2,I,L,D,ROut> (&self, other: &Arranged<G,T2>, mut result: L) -> Collection<G,D,ROut>
     where
         T2: for<'a> TraceReader<Key<'a>=T1::Key<'a>, Time=T1::Time>+Clone+'static,
         D: Data,
@@ -264,7 +264,15 @@ where
         L: FnMut(T1::Key<'_>, T1::Val<'_>,T2::Val<'_>,&G::Timestamp,&T1::Diff,&T2::Diff)->I+'static,
     {
         use crate::operators::join::join_traces;
-        join_traces(self, other, result)
+        join_traces::<_, _, _, _, crate::consolidation::ConsolidatingContainerBuilder<_>>(
+            self,
+            other,
+            move |k, v1, v2, t, d1, d2, c| {
+                for datum in result(k, v1, v2, t, d1, d2) {
+                    c.give(datum);
+                }
+            }
+        )
             .as_collection()
     }
 }
diff --git a/src/operators/consolidate.rs b/src/operators/consolidate.rs
index 46b96528c..ac0ef7f8f 100644
--- a/src/operators/consolidate.rs
+++ b/src/operators/consolidate.rs
@@ -99,7 +99,7 @@ where
                     input.for_each(|time, data| {
                         data.swap(&mut vector);
                         crate::consolidation::consolidate_updates(&mut vector);
-                        output.session(&time).give_vec(&mut vector);
+                        output.session(&time).give_container(&mut vector);
                     })
                 }
             })
diff --git a/src/operators/join.rs b/src/operators/join.rs
index 311f06802..03bdc1e0d 100644
--- a/src/operators/join.rs
+++ b/src/operators/join.rs
@@ -4,13 +4,16 @@
 //! the multiplication distributes over addition. That is, we will repeatedly evaluate (a + b) * c as (a * c)
 //! + (b * c), and if this is not equal to the former term, little is known about the actual output.
 use std::cmp::Ordering;
+use timely::Container;
 
-use timely::container::{PushContainer, PushInto};
+use timely::container::{ContainerBuilder, PushContainer, PushInto};
 use timely::order::PartialOrder;
 use timely::progress::Timestamp;
 use timely::dataflow::{Scope, StreamCore};
 use timely::dataflow::operators::generic::{Operator, OutputHandleCore};
 use timely::dataflow::channels::pact::Pipeline;
+use timely::dataflow::channels::pushers::buffer::Session;
+use timely::dataflow::channels::pushers::Counter;
 use timely::dataflow::operators::Capability;
 use timely::dataflow::channels::pushers::tee::Tee;
 
@@ -316,6 +319,41 @@ where
     }
 }
 
+/// The session passed to join closures.
+pub type JoinSession<'a, T, CB, C> = Session<'a, T, EffortBuilder<CB>, Counter<T, C, Tee<T, C>>>;
+
+/// A container builder that tracks the length of outputs to estimate the effort of join closures.
+#[derive(Default, Debug)]
+pub struct EffortBuilder<CB>(pub std::cell::Cell<usize>, pub CB);
+
+impl<CB: ContainerBuilder> ContainerBuilder for EffortBuilder<CB> {
+    type Container = CB::Container;
+
+    #[inline]
+    fn push<T: PushInto<Self::Container>>(&mut self, item: T) where Self::Container: PushContainer {
+        self.1.push(item)
+    }
+
+    #[inline]
+    fn push_container(&mut self, container: &mut Self::Container) {
+        self.1.push_container(container)
+    }
+
+    #[inline]
+    fn extract(&mut self) -> Option<&mut Self::Container> {
+        let extracted = self.1.extract();
+        self.0.replace(self.0.take() + extracted.as_ref().map_or(0, |e| e.len()));
+        extracted
+    }
+
+    #[inline]
+    fn finish(&mut self) -> Option<&mut Self::Container> {
+        let finished = self.1.finish();
+        self.0.replace(self.0.take() + finished.as_ref().map_or(0, |e| e.len()));
+        finished
+    }
+}
+
 /// An equijoin of two traces, sharing a common key type.
 ///
 /// This method exists to provide join functionality without opinions on the specific input types, keys and values,
@@ -323,24 +361,19 @@ where
 /// even potentially unrelated to the input collection data. Importantly, the key and value types could be generic
 /// associated types (GATs) of the traces, and we would seemingly struggle to frame these types as trait arguments.
 ///
-/// The implementation produces a caller-specified container, with the requirement that the container
-/// can absorb `(D, G::Timestamp, R)` triples. Implementations can use [`AsCollection`] to wrap the
+/// The implementation produces a caller-specified container. Implementations can use [`AsCollection`] to wrap the
 /// output stream in a collection.
 ///
 /// The "correctness" of this method depends heavily on the behavior of the supplied `result` function.
 ///
 /// [`AsCollection`]: crate::collection::AsCollection
-pub fn join_traces<G, T1, T2, I,L,D,R,C>(arranged1: &Arranged<G,T1>, arranged2: &Arranged<G,T2>, mut result: L) -> StreamCore<G, C>
+pub fn join_traces<G, T1, T2, L, CB>(arranged1: &Arranged<G,T1>, arranged2: &Arranged<G,T2>, mut result: L) -> StreamCore<G, CB::Container>
 where
     G: Scope<Timestamp=T1::Time>,
     T1: TraceReader+Clone+'static,
     T2: for<'a> TraceReader<Key<'a>=T1::Key<'a>, Time=T1::Time>+Clone+'static,
-    D: Data,
-    R: Semigroup,
-    I: IntoIterator<Item=(D, G::Timestamp, R)>,
-    L: FnMut(T1::Key<'_>,T1::Val<'_>,T2::Val<'_>,&G::Timestamp,&T1::Diff,&T2::Diff)->I+'static,
-    C: PushContainer,
-    (D, G::Timestamp, R): PushInto<C>,
+    L: FnMut(T1::Key<'_>,T1::Val<'_>,T2::Val<'_>,&G::Timestamp,&T1::Diff,&T2::Diff,&mut JoinSession<T1::Time, CB, CB::Container>)+'static,
+    CB: ContainerBuilder + 'static,
 {
     // Rename traces for symmetry from here on out.
     let mut trace1 = arranged1.trace.clone();
@@ -512,7 +545,7 @@ where
             while !todo1.is_empty() && fuel > 0 {
                 todo1.front_mut().unwrap().work(
                     output,
-                    |k,v2,v1,t,r2,r1| result(k,v1,v2,t,r1,r2),
+                    |k,v2,v1,t,r2,r1,c| result(k,v1,v2,t,r1,r2,c),
                     &mut fuel
                 );
                 if !todo1.front().unwrap().work_remains() { todo1.pop_front(); }
@@ -523,7 +556,7 @@ where
             while !todo2.is_empty() && fuel > 0 {
                 todo2.front_mut().unwrap().work(
                     output,
-                    |k,v1,v2,t,r1,r2| result(k,v1,v2,t,r1,r2),
+                    |k,v1,v2,t,r1,r2,c| result(k,v1,v2,t,r1,r2,c),
                     &mut fuel
                 );
                 if !todo2.front().unwrap().work_remains() { todo2.pop_front(); }
@@ -582,13 +615,11 @@ where
 /// The structure wraps cursors which allow us to play out join computation at whatever rate we like.
 /// This allows us to avoid producing and buffering massive amounts of data, without giving the timely
 /// dataflow system a chance to run operators that can consume and aggregate the data.
-struct Deferred<T, R, C1, C2, D>
+struct Deferred<T, C1, C2>
 where
     T: Timestamp+Lattice+Ord,
-    R: Semigroup,
     C1: Cursor<Time=T>,
     C2: for<'a> Cursor<Key<'a>=C1::Key<'a>, Time=T>,
-    D: Ord+Clone+Data,
 {
     trace: C1,
     trace_storage: C1::Storage,
@@ -596,16 +627,13 @@ where
     batch_storage: C2::Storage,
     capability: Capability<T>,
     done: bool,
-    temp: Vec<((D, T), R)>,
 }
 
-impl<T, R, C1, C2, D> Deferred<T, R, C1, C2, D>
+impl<T, C1, C2> Deferred<T, C1, C2>
 where
     C1: Cursor<Time=T>,
     C2: for<'a> Cursor<Key<'a>=C1::Key<'a>, Time=T>,
     T: Timestamp+Lattice+Ord,
-    R: Semigroup,
-    D: Clone+Data,
 {
     fn new(trace: C1, trace_storage: C1::Storage, batch: C2, batch_storage: C2::Storage, capability: Capability<T>) -> Self {
         Deferred {
@@ -615,7 +643,6 @@ where
             batch_storage,
             capability,
             done: false,
-            temp: Vec::new(),
         }
     }
 
@@ -625,18 +652,15 @@ where
 
     /// Process keys until at least `fuel` output tuples produced, or the work is exhausted.
     #[inline(never)]
-    fn work<L, I, C>(&mut self, output: &mut OutputHandleCore<T, C, Tee<T, C>>, mut logic: L, fuel: &mut usize)
+    fn work<L, CB: ContainerBuilder>(&mut self, output: &mut OutputHandleCore<T, EffortBuilder<CB>, Tee<T, CB::Container>>, mut logic: L, fuel: &mut usize)
     where
-        I: IntoIterator<Item=(D, T, R)>,
-        L: for<'a> FnMut(C1::Key<'a>, C1::Val<'a>, C2::Val<'a>, &T, &C1::Diff, &C2::Diff)->I,
-        C: PushContainer,
-        (D, T, R): PushInto<C>,
+        L: for<'a> FnMut(C1::Key<'a>, C1::Val<'a>, C2::Val<'a>, &T, &C1::Diff, &C2::Diff, &mut JoinSession<T, CB, CB::Container>),
     {
 
         let meet = self.capability.time();
 
         let mut effort = 0;
-        let mut session = output.session(&self.capability);
+        let mut session = output.session_with_builder(&self.capability);
 
         let trace_storage = &self.trace_storage;
         let batch_storage = &self.batch_storage;
@@ -644,7 +668,6 @@ where
         let trace = &mut self.trace;
         let batch = &mut self.batch;
 
-        let temp = &mut self.temp;
         let mut thinker = JoinThinker::new();
 
         while batch.key_valid(batch_storage) && trace.key_valid(trace_storage) && effort < *fuel {
@@ -657,28 +680,15 @@ where
                     thinker.history1.edits.load(trace, trace_storage, |time| time.join(meet));
                     thinker.history2.edits.load(batch, batch_storage, |time| time.clone());
 
-                    assert_eq!(temp.len(), 0);
-
                     // populate `temp` with the results in the best way we know how.
                     thinker.think(|v1,v2,t,r1,r2| {
                         let key = batch.key(batch_storage);
-                        for (d, t, r) in logic(key, v1, v2, &t, r1, r2) {
-                            temp.push(((d, t), r));
-                        }
+                        logic(key, v1, v2, &t, r1, r2, &mut session);
                     });
 
-                    // TODO: This consolidation is optional, and it may not be very
-                    //       helpful. We might try harder to understand whether we
-                    //       should do this work here, or downstream at consumers.
-                    // TODO: Perhaps `thinker` should have the buffer, do smarter
-                    //       consolidation, and then deposit results in `session`.
-                    crate::consolidation::consolidate(temp);
-
-                    effort += temp.len();
-                    for ((d, t), r) in temp.drain(..) {
-                        session.give((d, t, r));
-                    }
-
+                    // TODO: Effort isn't perfectly tracked as we might still have some data in the
+                    // session at the moment it's dropped.
+                    effort += session.builder().0.take();
                     batch.step_key(batch_storage);
                     trace.step_key(trace_storage);
 
@@ -687,7 +697,6 @@ where
                 }
             }
         }
-
         self.done = !batch.key_valid(batch_storage) || !trace.key_valid(trace_storage);
 
         if effort > *fuel { *fuel = 0; }

From de6a353400e7b39addb65a7b95a62ee5a1b40bae Mon Sep 17 00:00:00 2001
From: Frank McSherry <fmcsherry@me.com>
Date: Mon, 6 May 2024 09:39:56 -0400
Subject: [PATCH 5/5] Re-introduce safe implementation (#481)

---
 src/consolidation.rs | 118 +++++++++++++++++++------------------------
 1 file changed, 51 insertions(+), 67 deletions(-)

diff --git a/src/consolidation.rs b/src/consolidation.rs
index 637df0eff..3801729e8 100644
--- a/src/consolidation.rs
+++ b/src/consolidation.rs
@@ -37,49 +37,40 @@ pub fn consolidate_from<T: Ord, R: Semigroup>(vec: &mut Vec<(T, R)>, offset: usi
 /// Sorts and consolidates a slice, returning the valid prefix length.
 pub fn consolidate_slice<T: Ord, R: Semigroup>(slice: &mut [(T, R)]) -> usize {
 
-    // We could do an insertion-sort like initial scan which builds up sorted, consolidated runs.
-    // In a world where there are not many results, we may never even need to call in to merge sort.
-    slice.sort_by(|x,y| x.0.cmp(&y.0));
-
-    let slice_ptr = slice.as_mut_ptr();
-
-    // Counts the number of distinct known-non-zero accumulations. Indexes the write location.
-    let mut offset = 0;
-    for index in 1 .. slice.len() {
-
-        // The following unsafe block elides various bounds checks, using the reasoning that `offset`
-        // is always strictly less than `index` at the beginning of each iteration. This is initially
-        // true, and in each iteration `offset` can increase by at most one (whereas `index` always
-        // increases by one). As `index` is always in bounds, and `offset` starts at zero, it too is
-        // always in bounds.
-        //
-        // LLVM appears to struggle to optimize out Rust's split_at_mut, which would prove disjointness
-        // using run-time tests.
-        unsafe {
-
-            assert!(offset < index);
-
-            // LOOP INVARIANT: offset < index
-            let ptr1 = slice_ptr.add(offset);
-            let ptr2 = slice_ptr.add(index);
-
-            if (*ptr1).0 == (*ptr2).0 {
-                (*ptr1).1.plus_equals(&(*ptr2).1);
+    if slice.len() > 1 {
+
+        // We could do an insertion-sort like initial scan which builds up sorted, consolidated runs.
+        // In a world where there are not many results, we may never even need to call in to merge sort.
+        slice.sort_by(|x,y| x.0.cmp(&y.0));
+
+        // Counts the number of distinct known-non-zero accumulations. Indexes the write location.
+        let mut offset = 0;
+        let mut accum = slice[offset].1.clone();
+
+        for index in 1 .. slice.len() {
+            if slice[index].0 == slice[index-1].0 {
+                accum.plus_equals(&slice[index].1);
             }
             else {
-                if !(*ptr1).1.is_zero() {
+                if !accum.is_zero() {
+                    slice.swap(offset, index-1);
+                    slice[offset].1.clone_from(&accum);
                     offset += 1;
                 }
-                let ptr1 = slice_ptr.add(offset);
-                std::ptr::swap(ptr1, ptr2);
+                accum.clone_from(&slice[index].1);
             }
         }
+        if !accum.is_zero() {
+            slice.swap(offset, slice.len()-1);
+            slice[offset].1 = accum;
+            offset += 1;
+        }
+
+        offset
     }
-    if offset < slice.len() && !slice[offset].1.is_zero() {
-        offset += 1;
+    else {
+        slice.iter().filter(|x| !x.1.is_zero()).count()
     }
-
-    offset
 }
 
 /// Sorts and consolidates `vec`.
@@ -104,50 +95,43 @@ pub fn consolidate_updates_from<D: Ord, T: Ord, R: Semigroup>(vec: &mut Vec<(D,
 /// Sorts and consolidates a slice, returning the valid prefix length.
 pub fn consolidate_updates_slice<D: Ord, T: Ord, R: Semigroup>(slice: &mut [(D, T, R)]) -> usize {
 
-    // We could do an insertion-sort like initial scan which builds up sorted, consolidated runs.
-    // In a world where there are not many results, we may never even need to call in to merge sort.
-    slice.sort_unstable_by(|x,y| (&x.0, &x.1).cmp(&(&y.0, &y.1)));
-
-    let slice_ptr = slice.as_mut_ptr();
+    if slice.len() > 1 {
 
-    // Counts the number of distinct known-non-zero accumulations. Indexes the write location.
-    let mut offset = 0;
-    for index in 1 .. slice.len() {
+        // We could do an insertion-sort like initial scan which builds up sorted, consolidated runs.
+        // In a world where there are not many results, we may never even need to call in to merge sort.
+        slice.sort_unstable_by(|x,y| (&x.0, &x.1).cmp(&(&y.0, &y.1)));
 
-        // The following unsafe block elides various bounds checks, using the reasoning that `offset`
-        // is always strictly less than `index` at the beginning of each iteration. This is initially
-        // true, and in each iteration `offset` can increase by at most one (whereas `index` always
-        // increases by one). As `index` is always in bounds, and `offset` starts at zero, it too is
-        // always in bounds.
-        //
-        // LLVM appears to struggle to optimize out Rust's split_at_mut, which would prove disjointness
-        // using run-time tests.
-        unsafe {
+        // Counts the number of distinct known-non-zero accumulations. Indexes the write location.
+        let mut offset = 0;
+        let mut accum = slice[offset].2.clone();
 
-            // LOOP INVARIANT: offset < index
-            let ptr1 = slice_ptr.add(offset);
-            let ptr2 = slice_ptr.add(index);
-
-            if (*ptr1).0 == (*ptr2).0 && (*ptr1).1 == (*ptr2).1 {
-                (*ptr1).2.plus_equals(&(*ptr2).2);
+        for index in 1 .. slice.len() {
+            if (slice[index].0 == slice[index-1].0) && (slice[index].1 == slice[index-1].1) {
+                accum.plus_equals(&slice[index].2);
             }
             else {
-                if !(*ptr1).2.is_zero() {
+                if !accum.is_zero() {
+                    slice.swap(offset, index-1);
+                    slice[offset].2.clone_from(&accum);
                     offset += 1;
                 }
-                let ptr1 = slice_ptr.add(offset);
-                std::ptr::swap(ptr1, ptr2);
+                accum.clone_from(&slice[index].2);
             }
-
         }
+        if !accum.is_zero() {
+            slice.swap(offset, slice.len()-1);
+            slice[offset].2 = accum;
+            offset += 1;
+        }
+
+        offset
     }
-    if offset < slice.len() && !slice[offset].2.is_zero() {
-        offset += 1;
+    else {
+        slice.iter().filter(|x| !x.2.is_zero()).count()
     }
-
-    offset
 }
 
+
 /// A container builder that consolidates data in-places into fixed-sized containers. Does not
 /// maintain FIFO ordering.
 #[derive(Default)]
@@ -332,6 +316,6 @@ mod tests {
         for i in 0..1024 {
             assert_eq!((i, 0, 2), collected[i]);
         }
-
+        
     }
 }