Skip to content

Commit fe0a008

Browse files
Dandandanclaude
andcommitted
Hold primitive group keys in blocks too
The group keys are the other half of what a grouping stores per group, and they grew the same way: q32 keeps 12 bytes of key next to 32 bytes of aggregate state, all of it in allocations that were copied on every growth. `BlockedVec` moves to `datafusion-common` so `physical-plan` can use it, and `PrimitiveGroupValueBuilder` stores its values in it. Both comparison loops resolve the representation once and are compiled per representation, so the flat path is unchanged. The flat state no longer starts at a batch: reserving one up front costs 64KB per state vector, which `ordered_aggregate_spill.slt` showed exhausting a 600KB pool. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_0117yCevpWYN82FH6PLrM8qu
1 parent e3fe1da commit fe0a008

4 files changed

Lines changed: 474 additions & 369 deletions

File tree

Lines changed: 381 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,381 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
//! Per-group accumulator state, held in one allocation while it is small and in
19+
//! fixed-size blocks once it is large.
20+
21+
use std::mem::size_of;
22+
23+
/// Groups per block once blocked: 2 MB for an eight byte state, and a whole number of
24+
/// output batches, so a block can be handed to Arrow as a buffer of its own.
25+
///
26+
/// Counted in groups rather than bytes so that two states of different element types
27+
/// (`avg`'s sums and counts, say) always agree on where a block starts.
28+
pub const BLOCK_LEN: usize = 1 << 18;
29+
30+
/// Groups a state may reach before it is worth holding in blocks.
31+
///
32+
/// Below this the state is one plain `Vec`: growing it copies at most this many elements
33+
/// in total, which is not worth paying a second load on every group update to avoid.
34+
///
35+
/// Equal to [`BLOCK_LEN`] so that the flat state is exactly one block when it switches,
36+
/// and becomes block zero without being copied or split.
37+
pub const THRESHOLD_LEN: usize = BLOCK_LEN;
38+
39+
const BLOCK_SHIFT: u32 = BLOCK_LEN.trailing_zeros();
40+
const BLOCK_MASK: usize = BLOCK_LEN - 1;
41+
42+
/// The block and offset a group index falls in.
43+
#[inline]
44+
pub fn block_offset(index: usize) -> (usize, usize) {
45+
(index >> BLOCK_SHIFT, index & BLOCK_MASK)
46+
}
47+
48+
enum Storage<T> {
49+
/// One allocation. Growing copies, which is cheap while it is small.
50+
Flat(Vec<T>),
51+
/// Fixed-size blocks. Growing appends a block and never moves what is already there.
52+
Blocked(Vec<Vec<T>>),
53+
}
54+
55+
/// A growable sequence of per-group state, addressed by group index.
56+
///
57+
/// Growing one allocation copies everything already in it, and for a grouping with
58+
/// millions of groups that copying is a large part of what the aggregate does. Reaching
59+
/// through a block index costs a second load on *every* group update, though, so blocks
60+
/// are only worth it once the copying they avoid outweighs the lookups they add: this
61+
/// stays flat up to [`THRESHOLD_LEN`] groups and blocks above it.
62+
///
63+
/// Callers should take [`Self::storage_mut`] once and loop inside the arm rather than
64+
/// indexing through this type per group, so the representation is resolved once per
65+
/// batch instead of once per update.
66+
pub struct BlockedVec<T> {
67+
storage: Storage<T>,
68+
len: usize,
69+
}
70+
71+
impl<T: Clone> Default for BlockedVec<T> {
72+
fn default() -> Self {
73+
Self::new()
74+
}
75+
}
76+
77+
impl<T: Clone> std::fmt::Debug for BlockedVec<T> {
78+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
79+
f.debug_struct("BlockedVec")
80+
.field("len", &self.len)
81+
.field("blocked", &self.is_blocked())
82+
.finish()
83+
}
84+
}
85+
86+
/// A borrowed view of the state, taken once per batch so the update loop does not
87+
/// re-check which representation is in use on every group.
88+
pub enum StorageMut<'a, T> {
89+
Flat(&'a mut [T]),
90+
Blocked(&'a mut [Vec<T>]),
91+
}
92+
93+
/// [`StorageMut`] for readers.
94+
pub enum StorageRef<'a, T> {
95+
Flat(&'a [T]),
96+
Blocked(&'a [Vec<T>]),
97+
}
98+
99+
impl<T: Clone> BlockedVec<T> {
100+
pub fn new() -> Self {
101+
Self {
102+
storage: Storage::Flat(Vec::new()),
103+
len: 0,
104+
}
105+
}
106+
107+
pub fn len(&self) -> usize {
108+
self.len
109+
}
110+
111+
pub fn is_empty(&self) -> bool {
112+
self.len == 0
113+
}
114+
115+
/// Whether the state has moved to blocks.
116+
pub fn is_blocked(&self) -> bool {
117+
matches!(self.storage, Storage::Blocked(_))
118+
}
119+
120+
/// Bytes held, counting space reserved but not filled.
121+
pub fn capacity_bytes(&self) -> usize {
122+
let elements = match &self.storage {
123+
Storage::Flat(values) => values.capacity(),
124+
Storage::Blocked(blocks) => blocks.iter().map(|b| b.capacity()).sum(),
125+
};
126+
elements * size_of::<T>()
127+
}
128+
129+
/// Resolves the representation once, so a reading loop is free of the check.
130+
#[inline]
131+
pub fn storage(&self) -> StorageRef<'_, T> {
132+
match &self.storage {
133+
Storage::Flat(values) => StorageRef::Flat(values.as_slice()),
134+
Storage::Blocked(blocks) => StorageRef::Blocked(blocks.as_slice()),
135+
}
136+
}
137+
138+
/// Reads one group without a bounds check.
139+
///
140+
/// # Safety
141+
/// `index` must be less than [`Self::len`].
142+
#[inline]
143+
pub unsafe fn get_unchecked(&self, index: usize) -> &T {
144+
match &self.storage {
145+
Storage::Flat(values) => unsafe { values.get_unchecked(index) },
146+
Storage::Blocked(blocks) => {
147+
let (block, offset) = block_offset(index);
148+
unsafe { blocks.get_unchecked(block).get_unchecked(offset) }
149+
}
150+
}
151+
}
152+
153+
/// Appends one group.
154+
pub fn push(&mut self, value: T) {
155+
if self.len == THRESHOLD_LEN && !self.is_blocked() {
156+
self.switch_to_blocked();
157+
}
158+
match &mut self.storage {
159+
Storage::Flat(values) => values.push(value),
160+
Storage::Blocked(blocks) => {
161+
let (block, _) = block_offset(self.len);
162+
if block == blocks.len() {
163+
blocks.push(Vec::with_capacity(BLOCK_LEN));
164+
}
165+
blocks[block].push(value);
166+
}
167+
}
168+
self.len += 1;
169+
}
170+
171+
/// Resolves the representation once, so the caller's loop is free of the check.
172+
#[inline]
173+
pub fn storage_mut(&mut self) -> StorageMut<'_, T> {
174+
match &mut self.storage {
175+
Storage::Flat(values) => StorageMut::Flat(values.as_mut_slice()),
176+
Storage::Blocked(blocks) => StorageMut::Blocked(blocks.as_mut_slice()),
177+
}
178+
}
179+
180+
/// Grows to `total_num_groups`, filling with `value`.
181+
pub fn resize(&mut self, total_num_groups: usize, value: T) {
182+
if total_num_groups <= self.len {
183+
self.truncate(total_num_groups);
184+
return;
185+
}
186+
if total_num_groups > THRESHOLD_LEN && !self.is_blocked() {
187+
self.switch_to_blocked();
188+
}
189+
match &mut self.storage {
190+
Storage::Flat(values) => values.resize(total_num_groups, value),
191+
Storage::Blocked(blocks) => {
192+
let mut len = self.len;
193+
while len < total_num_groups {
194+
let (block, offset) = block_offset(len);
195+
if block == blocks.len() {
196+
blocks.push(Vec::with_capacity(BLOCK_LEN));
197+
}
198+
let take = std::cmp::min(BLOCK_LEN - offset, total_num_groups - len);
199+
blocks[block].resize(offset + take, value.clone());
200+
len += take;
201+
}
202+
}
203+
}
204+
self.len = total_num_groups;
205+
}
206+
207+
/// Hands the single allocation over as block zero. Paid once, at the threshold.
208+
///
209+
/// The threshold is one block, so what is already stored fits a block exactly and
210+
/// nothing has to be copied or split; it is only given room to fill that block out.
211+
fn switch_to_blocked(&mut self) {
212+
let Storage::Flat(values) = &mut self.storage else {
213+
return;
214+
};
215+
let mut rest = std::mem::take(values);
216+
if rest.len() <= BLOCK_LEN {
217+
// The usual case: growth switches at exactly one block, so the allocation
218+
// is handed over as it stands and only given room to fill the block out.
219+
rest.reserve_exact(BLOCK_LEN - rest.len());
220+
self.storage = Storage::Blocked(vec![rest]);
221+
return;
222+
}
223+
// `take_first` can leave more than a block behind, which has to be split.
224+
let mut blocks: Vec<Vec<T>> = Vec::with_capacity(rest.len().div_ceil(BLOCK_LEN));
225+
while rest.len() > BLOCK_LEN {
226+
let tail = rest.split_off(BLOCK_LEN);
227+
blocks.push(rest);
228+
rest = tail;
229+
}
230+
rest.reserve_exact(BLOCK_LEN - rest.len());
231+
blocks.push(rest);
232+
self.storage = Storage::Blocked(blocks);
233+
}
234+
235+
fn truncate(&mut self, len: usize) {
236+
if len >= self.len {
237+
return;
238+
}
239+
match &mut self.storage {
240+
Storage::Flat(values) => values.truncate(len),
241+
Storage::Blocked(blocks) => {
242+
blocks.truncate(len.div_ceil(BLOCK_LEN));
243+
if let Some(last) = blocks.last_mut() {
244+
let offset = len & BLOCK_MASK;
245+
if offset != 0 {
246+
last.truncate(offset);
247+
}
248+
}
249+
}
250+
}
251+
self.len = len;
252+
}
253+
254+
#[inline]
255+
pub fn get(&self, index: usize) -> Option<&T> {
256+
match &self.storage {
257+
Storage::Flat(values) => values.get(index),
258+
Storage::Blocked(blocks) => {
259+
let (block, offset) = block_offset(index);
260+
blocks.get(block)?.get(offset)
261+
}
262+
}
263+
}
264+
265+
/// Everything in one allocation, leaving this empty.
266+
///
267+
/// While flat this hands over the allocation as it stands. Once blocked it copies,
268+
/// which is what a per-block emit would avoid.
269+
pub fn take_contiguous(&mut self) -> Vec<T> {
270+
let storage = std::mem::replace(&mut self.storage, Storage::Flat(Vec::new()));
271+
self.len = 0;
272+
match storage {
273+
Storage::Flat(values) => values,
274+
Storage::Blocked(mut blocks) => match blocks.len() {
275+
0 => Vec::new(),
276+
1 => blocks.pop().unwrap_or_default(),
277+
_ => {
278+
let mut out =
279+
Vec::with_capacity(blocks.iter().map(|b| b.len()).sum());
280+
for block in &blocks {
281+
out.extend(block.iter().cloned());
282+
}
283+
out
284+
}
285+
},
286+
}
287+
}
288+
289+
/// Removes the first `n` elements, shifting the rest down.
290+
pub fn take_first(&mut self, n: usize) -> Vec<T> {
291+
let mut all = self.take_contiguous();
292+
let rest = all.split_off(std::cmp::min(n, all.len()));
293+
let len = rest.len();
294+
self.storage = Storage::Flat(rest);
295+
self.len = len;
296+
if len > THRESHOLD_LEN {
297+
self.switch_to_blocked();
298+
}
299+
all
300+
}
301+
}
302+
303+
#[cfg(test)]
304+
mod tests {
305+
use super::*;
306+
307+
#[test]
308+
fn stays_flat_below_threshold() {
309+
let mut v: BlockedVec<u64> = BlockedVec::new();
310+
v.resize(THRESHOLD_LEN, 0);
311+
assert!(!v.is_blocked(), "should still be one allocation");
312+
v.resize(THRESHOLD_LEN + 1, 0);
313+
assert!(v.is_blocked(), "should have switched to blocks");
314+
}
315+
316+
#[test]
317+
fn values_survive_the_switch() {
318+
let mut v: BlockedVec<u64> = BlockedVec::new();
319+
let n = THRESHOLD_LEN + BLOCK_LEN + 7;
320+
v.resize(1000, 0);
321+
for i in 0..1000 {
322+
unsafe { *v.get_unchecked_mut_for_test(i) = i as u64 };
323+
}
324+
v.resize(n, 0);
325+
for i in 1000..n {
326+
unsafe { *v.get_unchecked_mut_for_test(i) = i as u64 };
327+
}
328+
assert!(v.is_blocked());
329+
assert_eq!(v.len(), n);
330+
for i in (0..n).step_by(997) {
331+
assert_eq!(v.get(i).copied(), Some(i as u64), "at {i}");
332+
}
333+
let flat = v.take_contiguous();
334+
assert_eq!(flat.len(), n);
335+
assert!(flat.iter().enumerate().all(|(i, x)| *x == i as u64));
336+
}
337+
338+
#[test]
339+
fn resize_keeps_existing_values() {
340+
let mut v: BlockedVec<u64> = BlockedVec::new();
341+
v.resize(10, 1);
342+
v.resize(THRESHOLD_LEN + 5, 2);
343+
assert_eq!(v.get(0).copied(), Some(1));
344+
assert_eq!(v.get(9).copied(), Some(1));
345+
assert_eq!(v.get(10).copied(), Some(2));
346+
assert_eq!(v.len(), THRESHOLD_LEN + 5);
347+
}
348+
349+
#[test]
350+
fn take_first_shifts_down() {
351+
let mut v: BlockedVec<u64> = BlockedVec::new();
352+
let n = THRESHOLD_LEN + 100;
353+
v.resize(n, 0);
354+
for i in 0..n {
355+
unsafe { *v.get_unchecked_mut_for_test(i) = i as u64 };
356+
}
357+
let taken = v.take_first(50);
358+
assert_eq!(taken.len(), 50);
359+
assert!(taken.iter().enumerate().all(|(i, x)| *x == i as u64));
360+
assert_eq!(v.len(), n - 50);
361+
assert_eq!(v.get(0).copied(), Some(50));
362+
assert_eq!(v.get(v.len() - 1).copied(), Some(n as u64 - 1));
363+
}
364+
365+
impl<T: Clone> BlockedVec<T> {
366+
/// Indexing one group at a time, for tests only; the accumulators go through
367+
/// [`BlockedVec::storage_mut`] instead.
368+
///
369+
/// # Safety
370+
/// `index` must be less than [`BlockedVec::len`].
371+
unsafe fn get_unchecked_mut_for_test(&mut self, index: usize) -> &mut T {
372+
match &mut self.storage {
373+
Storage::Flat(values) => unsafe { values.get_unchecked_mut(index) },
374+
Storage::Blocked(blocks) => {
375+
let (block, offset) = block_offset(index);
376+
unsafe { blocks.get_unchecked_mut(block).get_unchecked_mut(offset) }
377+
}
378+
}
379+
}
380+
}
381+
}

‎datafusion/common/src/utils/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
//! This module provides the bisect function, which implements binary search.
1919
2020
pub(crate) mod aggregate;
21+
pub mod blocked_vec;
2122
pub mod expr;
2223
pub mod hex;
2324
pub mod memory;

0 commit comments

Comments
 (0)