From 0811649523c8728e56478ef71ceb4b882de8eb42 Mon Sep 17 00:00:00 2001 From: Frank McSherry Date: Fri, 31 Jul 2026 12:35:36 -0400 Subject: [PATCH 1/3] reduce: ids() borrows the leaf via corgi::leaf_slice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ids() had its own copy of 'is this a u64 leaf' (bare Prim, or a 1-field Prod of one) duplicating what corgi can answer, and reached it via clone().into_u64() — the clone bumps the Arc, so into_u64's try-unwrap always fails and copies regardless, even for a freshly-gathered column with a single holder. leaf_slice answers the shape question once, in the layer that owns shapes, and hands back a borrow. 10 lines to 5, one shape-test instead of two. NOTE measured flat (scc 100 rounds: 51.05s vs 50.87s) — the copies removed were real but small; this is a simplification, not a performance change. Requires a corgi pin bump. Co-Authored-By: Claude Fable 5 --- interactive/src/corgi/reduce.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/interactive/src/corgi/reduce.rs b/interactive/src/corgi/reduce.rs index 1435d0a59..f89891cc8 100644 --- a/interactive/src/corgi/reduce.rs +++ b/interactive/src/corgi/reduce.rs @@ -169,14 +169,13 @@ fn concat_columns(blocks: &[CValue]) -> CValue { /// Applied CONSISTENTLY at every id site (both value presentations AND the freshly-produced /// `reduce_brackets` outputs), else `desired − current` nets across mismatched ids for the same value. fn ids(col: &CValue) -> Vec { - match corgi::shape_of_value(col) { - Shape::Prim(64) => col.clone().into_u64("ids"), - Shape::Prod(ref fs) if fs.len() == 1 && matches!(fs[0], Shape::Prim(64)) => match col { - CValue::Prod(fields) => fields[0].clone().into_u64("ids"), - _ => unreachable!("shape Prod but value not Prod"), - }, - _ => corgi::hash(col).into_u64("ids"), + // Value-as-id: borrow the leaf and copy once, rather than `clone().into_u64()` — the + // clone bumps the `Arc`, so `into_u64`'s try-unwrap always fails and copies anyway, + // even for a freshly-gathered column with one holder. + if let Some(sl) = corgi::arrange::leaf_slice(col) { + return sl.to_vec(); } + corgi::hash(col).into_u64("ids") } /// The `changed` set as a needle column in the chunks' own key shape — possible exactly @@ -248,7 +247,16 @@ where } } else { for (ci, ch) in chunks.iter().enumerate() { - let kh = ids(ch.keys()); + // Borrow the key leaf when there is one (`ids`' value-as-id fast paths); only + // structural keys need the hash, and only they pay a materialization. A shared + // column's `Arc` cannot be unwrapped, so `ids` would copy the whole key column + // here, once per chunk per retire, to read values it never mutates. + let hashed: Option> = corgi::arrange::leaf_slice(ch.keys()).is_none().then(|| ids(ch.keys())); + let kh: &[u64] = match (&hashed, corgi::arrange::leaf_slice(ch.keys())) { + (Some(v), _) => &v[..], + (None, Some(sl)) => sl, + (None, None) => unreachable!("leaf_slice absent implies hashed present"), + }; for i in 0..kh.len() { if changed.binary_search(&kh[i]).is_ok() { tags.push(ci); From a401a5d2d39811e7618763de4fc1aab3cf866c69 Mon Sep 17 00:00:00 2001 From: Frank McSherry Date: Fri, 31 Jul 2026 16:44:29 -0400 Subject: [PATCH 2/3] Bump the corgi pin to master (c4626fc) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DDIR pinned 1301b281, a revision that existed on no branch — it lived only on the side line where corgi's arrange API was developed. That API is now on corgi's master (wip#9), and the read-path fixes on top of it (wip#8), so the pin can name master. The bump also delivers find_ranges' native u64 path, measured end-to-end here: scc, 100 rounds x batch 100: 63.66s -> 51.19s (-20%) reach, 1000 rounds x batch 100: 2.09s -> 1.56s (-25%) scc load-shaped @100k: 3.53s -> 3.50s (unchanged) Gate green, debug and release. Co-Authored-By: Claude Fable 5 --- interactive/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interactive/Cargo.toml b/interactive/Cargo.toml index c31ee2212..b3e3a028d 100644 --- a/interactive/Cargo.toml +++ b/interactive/Cargo.toml @@ -14,7 +14,7 @@ workspace = true [dependencies] columnar = { workspace = true } # The columnar kernels for the interpreted backend, pinned by git rev. -corgi = { git = "https://github.com/frankmcsherry/wip", rev = "1301b281501d5e70ab63f9405412770a3250d985" } +corgi = { git = "https://github.com/frankmcsherry/wip", rev = "c4626fce02288594c9806e9b747a19598d680e0a" } differential-dataflow = { workspace = true } mimalloc = "0.1.48" serde = { version = "1.0", features = ["derive"] } From b4cfd29867084b776f5b9d3449fad7a3e75e556e Mon Sep 17 00:00:00 2001 From: Frank McSherry Date: Fri, 31 Jul 2026 17:11:43 -0400 Subject: [PATCH 3/3] reduce: record that bounded windows were measured and rejected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment said single-window presentation was 'a later refinement', which invites an experiment that does not pay. Measured: WINDOW = 1<<14 costs 33% time (scc, 100 rounds x batch 100: 84.4s vs 63.7s) and returns 4.4% memory (356MB -> 340MB peak RSS). The seek path removed windowing's asymptotic barrier but not its constant — per-window, per-chunk seek setup multiplies by the window count — and the presentation was never the memory peak; the trace is. Doc only; replaces an inviting TODO with the measurement. Co-Authored-By: Claude Fable 5 --- interactive/src/corgi/reduce.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/interactive/src/corgi/reduce.rs b/interactive/src/corgi/reduce.rs index f89891cc8..68ceacd21 100644 --- a/interactive/src/corgi/reduce.rs +++ b/interactive/src/corgi/reduce.rs @@ -454,8 +454,13 @@ where } fn next_window(&mut self, instance: &ReduceInstance<'_, CBatch, CBatch>, changed: &[u64], cursor: &mut usize) -> Option> { - // Single window: present ALL remaining changed keys at once (bounded-memory windowing is a - // later refinement). `changed` is ascending, so `binary_search` is the changed-key filter. + // Single window: present ALL remaining changed keys at once. This is NOT a deferred + // refinement — bounded windows were measured and rejected: at WINDOW = 1<<14, scc + // (100 rounds x batch 100) cost 84.4s against 63.7s, a 33% regression, while peak RSS + // fell only 356MB -> 340MB. Two reasons: the per-window, per-chunk seek setup is a + // fixed cost that multiplies by the window count, and the presentation is not the + // memory peak in the first place (the trace is). `changed` is ascending, so + // `binary_search` is the changed-key filter. if *cursor >= changed.len() { return None; }