Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: teach BitPackedEncoding canonical_into #2324

Merged
merged 23 commits into from
Feb 14, 2025
Merged

feat: teach BitPackedEncoding canonical_into #2324

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
3a24da2
feat: teach BitPackedEncoding canonical_into
danking Feb 11, 2025
77b779c
unnecessary import
danking Feb 11, 2025
10da4b4
fix PrimitiveBuilder::patch
danking Feb 11, 2025
1045a96
with_inputs
danking Feb 12, 2025
e4cbde9
better benchmarks: more sizes, more chunks, patches, move tests
danking Feb 12, 2025
409a98c
append_validity_mask
danking Feb 12, 2025
715c7df
fix patch to loop over validity and then separately loop over values
danking Feb 12, 2025
eeee805
extract macro into parameterized method in PrimitiveBuilder::patch
danking Feb 12, 2025
89a4e90
add nullable arrays
danking Feb 12, 2025
fa243bf
patch validity, but correct this time
danking Feb 12, 2025
e063b93
do not trigger validity materialization for non-null validity
danking Feb 12, 2025
348e3c4
Merge remote-tracking branch 'origin/develop' into dk/bitpacking-builder
danking Feb 12, 2025
05ac017
use new append validity mask
danking Feb 12, 2025
8308c6a
extract function out of match
danking Feb 13, 2025
2639bb1
guard on non-nullability not validity
danking Feb 13, 2025
db8bdfe
one loop
danking Feb 13, 2025
801eeeb
fix test code
danking Feb 13, 2025
32ec159
Merge remote-tracking branch 'origin/develop' into dk/bitpacking-builder
danking Feb 13, 2025
e6d3435
safety comment
danking Feb 14, 2025
2fb3a35
special handling of last chunk, no extra allocations
danking Feb 14, 2025
8b1f1c7
constants for bench args
danking Feb 14, 2025
6dd57c0
correct the case of a single chunk which is both offset and short
danking Feb 14, 2025
549f68e
Merge remote-tracking branch 'origin/develop' into dk/bitpacking-builder
danking Feb 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions encodings/fastlanes/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ arrow-buffer = { workspace = true }
fastlanes = { workspace = true }
itertools = { workspace = true }
num-traits = { workspace = true }
rand = { workspace = true, optional = true }
rkyv = { workspace = true }
serde = { workspace = true }
vortex-array = { workspace = true }
Expand All @@ -36,6 +37,10 @@ criterion = { workspace = true }
divan = { workspace = true }
rand = { workspace = true }
vortex-array = { workspace = true, features = ["test-harness"] }
vortex-fastlanes = { path = ".", features = ["test-harness"] }

[features]
test-harness = ["dep:rand"]

[[bench]]
name = "bitpacking_take"
Expand All @@ -48,3 +53,4 @@ harness = false
[[bench]]
name = "canonicalize_bench"
harness = false
required-features = ["test-harness"]
206 changes: 124 additions & 82 deletions encodings/fastlanes/benches/canonicalize_bench.rs
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This creates patches

    let mut rng = StdRng::seed_from_u64(0);
    let values = (0..len)
        .map(|_| {
            if rng.gen_bool(0.99) {
                T::from(rng.gen_range(0..6)).vortex_expect("valid value")
                    + T::from(7).vortex_expect("sdf")
            } else {
                T::from(rng.gen_range(0..100)).vortex_expect("valid value")
                    * T::from(100).expect("valid value")
            }
        })
        .collect::<BufferMut<T>>()
        .into_array()
        .into_primitive()
        .vortex_unwrap();

    let bit = bitpack_encode(values, 12).vortex_unwrap();

Original file line number Diff line number Diff line change
@@ -1,116 +1,158 @@
use divan::Bencher;
use rand::prelude::StdRng;
use rand::{Rng, SeedableRng};
use rand::SeedableRng;
use vortex_array::array::ChunkedArray;
use vortex_array::builders::{ArrayBuilder, PrimitiveBuilder};
use vortex_array::{Array, IntoArray, IntoArrayVariant, IntoCanonical};
use vortex_buffer::BufferMut;
use vortex_dtype::NativePType;
use vortex_error::{VortexExpect, VortexUnwrap};
use vortex_fastlanes::bitpack_to_best_bit_width;
use vortex_array::{IntoArray, IntoCanonical};
use vortex_error::{VortexExpect as _, VortexUnwrap};
use vortex_fastlanes::test_harness::make_array;

fn main() {
divan::main();
}

fn make_array<T: NativePType>(len: usize) -> Array {
#[divan::bench(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mind extract these into a single shared const, see BENCH_ARGS

args = [
(100000, 1, 0.10),
(100000, 1, 0.01),
(100000, 1, 0.00),
(100000, 10, 0.10),
(100000, 10, 0.01),
(100000, 10, 0.00),
(100000, 100, 0.10),
(100000, 100, 0.01),
(100000, 100, 0.00),
(100000, 1000, 0.00),
// (1000000, 100, 0.00),
// (1000000, 1000, 0.00),
// (10000000, 100, 0.00),
]
)]
fn into_canonical_non_nullable(
bencher: Bencher,
(chunk_len, chunk_count, fraction_patched): (usize, usize, f64),
) {
let mut rng = StdRng::seed_from_u64(0);
let values = (0..len)
.map(|_| T::from(rng.gen_range(0..100)).vortex_expect("valid value"))
.collect::<BufferMut<T>>()
.into_array()
.into_primitive()
.vortex_unwrap();

bitpack_to_best_bit_width(values)
.vortex_unwrap()
.into_array()
}

#[divan::bench()]
fn test() {
let chunks = (0..10).map(|_| make_array::<i32>(100)).collect::<Vec<_>>();
let arr = make_array::<i32>(1);
let chunked = ChunkedArray::try_new(chunks, arr.dtype().clone())
.vortex_unwrap()
.into_array();
let chunks = (0..chunk_count)
.map(|_| {
make_array(&mut rng, chunk_len, fraction_patched, 0.0).vortex_expect("make_array works")
})
.collect::<Vec<_>>();
let chunked = ChunkedArray::from_iter(chunks).into_array();

let into_ca = chunked
.clone()
.into_canonical()
.vortex_unwrap()
.into_primitive()
.vortex_unwrap();
let mut primitive_builder =
PrimitiveBuilder::<i32>::with_capacity(arr.dtype().nullability(), 10 * 100);
chunked
.clone()
.canonicalize_into(&mut primitive_builder)
.vortex_unwrap();
let ca_into = primitive_builder.finish().vortex_unwrap();
bencher
.with_inputs(|| chunked.clone())
.bench_values(|chunked| chunked.into_canonical().vortex_unwrap());
}

assert_eq!(
into_ca.as_slice::<i32>(),
ca_into.into_primitive().vortex_unwrap().as_slice::<i32>()
);
#[divan::bench(
args = [
(100000, 1, 0.10),
(100000, 1, 0.01),
(100000, 1, 0.00),
(100000, 10, 0.10),
(100000, 10, 0.01),
(100000, 10, 0.00),
(100000, 100, 0.10),
(100000, 100, 0.01),
(100000, 100, 0.00),
(100000, 1000, 0.00),
// (1000000, 100, 0.00),
// (1000000, 1000, 0.00),
// (10000000, 100, 0.00),
]
)]
fn canonical_into_non_nullable(
bencher: Bencher,
(chunk_len, chunk_count, fraction_patched): (usize, usize, f64),
) {
let mut rng = StdRng::seed_from_u64(0);

let mut primitive_builder =
PrimitiveBuilder::<i32>::with_capacity(arr.dtype().nullability(), 10 * 100);
primitive_builder.extend_from_array(chunked).vortex_unwrap();
let ca_into = primitive_builder.finish().vortex_unwrap();
let chunks = (0..chunk_count)
.map(|_| {
make_array(&mut rng, chunk_len, fraction_patched, 0.0).vortex_expect("make_array works")
})
.collect::<Vec<_>>();
let chunked = ChunkedArray::from_iter(chunks).into_array();

assert_eq!(
into_ca.as_slice::<i32>(),
ca_into.into_primitive().vortex_unwrap().as_slice::<i32>()
);
bencher
.with_inputs(|| chunked.clone())
.bench_values(|chunked| {
let mut primitive_builder = PrimitiveBuilder::<i32>::with_capacity(
chunked.dtype().nullability(),
chunk_len * chunk_count + 1024,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot assume this constant since its not added by the builder or at least we need another bench without this which is the primary one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran the benchmark without the extra 1024 elements, and canonical_into ended up slower in all cases. When the builder can’t hold the full result, it has to resize and copy—just like into_canonical.

I can also address your comment about using a temporary buffer for the final chunk and include that in a benchmark.

Looking ahead, I see two possible approaches:

  1. Never use truncate: canonical_into implementations would only push as many elements as they have (len), without truncating.
  2. Provide extra capacity: canonical_into implementations would reserve “garbage” space (perhaps via a minimum_builder_capacity()) so child structures can always write directly to their final location.

I’m inclined toward the second option because data goes exactly where it belongs, although it does waste space for smaller arrays. For example, a 500-element array might need space for 1024 elements. Ultimately, this is a key design decision for how builders should work.

Maybe worth a quick sync or a wider discussion? What do you think?

);
chunked
.canonicalize_into(&mut primitive_builder)
.vortex_unwrap();
primitive_builder.finish().vortex_unwrap()
});
}

#[divan::bench(
types = [u32],
args = [
// (1000, 100),
// (100000, 100),
// (1000000, 100),
// (100000, 1000),
(100000, 3),
(100000, 1, 0.10),
(100000, 1, 0.00),
(100000, 10, 0.10),
(100000, 10, 0.00),
(100000, 100, 0.10),
(100000, 100, 0.00),
]
)]
fn into_canonical<T: NativePType>(bencher: Bencher, (arr_len, chunk_count): (usize, usize)) {
fn into_canonical_nullable(
bencher: Bencher,
(chunk_len, chunk_count, fraction_patched): (usize, usize, f64),
) {
let mut rng = StdRng::seed_from_u64(0);

let chunks = (0..chunk_count)
.map(|_| make_array::<T>(arr_len))
.map(|_| {
make_array(&mut rng, chunk_len, fraction_patched, 0.05)
.vortex_expect("make_array works")
})
.collect::<Vec<_>>();
let arr = make_array::<T>(1);
let chunked = ChunkedArray::try_new(chunks, arr.dtype().clone()).vortex_unwrap();
let chunked = ChunkedArray::from_iter(chunks).into_array();

bencher.bench(|| chunked.clone().into_canonical().vortex_unwrap().len());
bencher
.with_inputs(|| chunked.clone())
.bench_values(|chunked| chunked.into_canonical().vortex_unwrap());
}

#[divan::bench(
types = [u32],
args = [
// (1000, 100),
// (100000, 100),
// (1000000, 100),
// (100000, 1000),
(100000, 3),
(100000, 1, 0.10),
(100000, 1, 0.00),
(100000, 10, 0.10),
(100000, 10, 0.00),
(100000, 100, 0.10),
(100000, 100, 0.00),
]
)]
fn canonical_into<T: NativePType>(bencher: Bencher, (arr_len, chunk_count): (usize, usize)) {
fn canonical_into_nullable(
bencher: Bencher,
(chunk_len, chunk_count, fraction_patched): (usize, usize, f64),
) {
let mut rng = StdRng::seed_from_u64(0);

let chunks = (0..chunk_count)
.map(|_| make_array::<T>(arr_len))
.map(|_| {
make_array(&mut rng, chunk_len, fraction_patched, 0.05)
.vortex_expect("make_array works")
})
.collect::<Vec<_>>();
let arr = make_array::<T>(1);
let chunked = ChunkedArray::try_new(chunks, arr.dtype().clone())
.vortex_unwrap()
.into_array();
let chunked = ChunkedArray::from_iter(chunks).into_array();

bencher.bench(|| {
let mut primitive_builder =
PrimitiveBuilder::<T>::with_capacity(arr.dtype().nullability(), arr_len * chunk_count);
chunked
.clone()
.canonicalize_into(&mut primitive_builder)
.vortex_unwrap();
primitive_builder.finish().vortex_unwrap().len()
});
bencher
.with_inputs(|| chunked.clone())
.bench_values(|chunked| {
let mut primitive_builder = PrimitiveBuilder::<i32>::with_capacity(
chunked.dtype().nullability(),
chunk_len * chunk_count + 1024,
);
chunked
.canonicalize_into(&mut primitive_builder)
.vortex_unwrap();
primitive_builder.finish().vortex_unwrap()
});
}
Loading
Loading