Skip to content

Commit ca37564

Browse files
JakeDernThomsonTandrewrelmas
authored
perf: Optimizing sorting by id/parent id (open-telemetry#2121)
# Change Summary This PR optimizes some id sorting logic that we have for split using techniques for two column sorts that we also use elsewhere in the transport optimize code. | Scenario | Baseline (RowConverter) | Optimized | Speedup | |----------------------------------------|-------------------------|-----------|---------| | u16 id only (random) | 7.23 us | 7.27 us | ~same | | u16 id only (sorted) | 1.20 us | 71 ns | 17x | | u16 pid + u16 id (native) | 25.4 us | 7.33 us | 3.5x | | u32 id + Dict<u8,u32> pid | 28.7 us | 7.89 us | 3.6x | | u32 id + Dict<u16,u32> pid | 28.3 us | 7.93 us | 3.6x | | u32 id (25% null) + Dict<u8,u32> pid | 30.4 us | 10.6 us | 2.9x | ## What issue does this PR close? * Closes open-telemetry#2122 ## How are these changes tested? New unit testing suite comparing results with the old and also indirectly covered by all the split tests. ## Are there any user-facing changes? No --------- Co-authored-by: Tom Tan <lilotom@gmail.com> Co-authored-by: Drew Relmas <drewrelmas@gmail.com>
1 parent 67bf269 commit ca37564

4 files changed

Lines changed: 528 additions & 23 deletions

File tree

rust/otap-dataflow/crates/pdata/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,7 @@ bench = []
5050
[[bench]]
5151
name = "concatenate"
5252
harness = false
53+
54+
[[bench]]
55+
name = "sort_by_parent_then_id"
56+
harness = false
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
//! Benchmarks for `sort_by_parent_then_id`.
5+
6+
use std::sync::Arc;
7+
8+
use arrow::array::{
9+
ArrayRef, DictionaryArray, PrimitiveArray, RecordBatch, UInt16Array, UInt32Array,
10+
};
11+
use arrow::datatypes::{DataType, Field, Schema, UInt8Type, UInt16Type};
12+
use criterion::{BenchmarkId, Criterion, criterion_group, criterion_main};
13+
use rand::rngs::StdRng;
14+
use rand::{RngExt, SeedableRng};
15+
16+
use otap_df_pdata::otap::transform::util::sort_by_parent_then_id;
17+
18+
const NUM_ROWS: usize = 1_000;
19+
const SEED: u64 = 42;
20+
21+
criterion_group!(benches, bench_sort);
22+
criterion_main!(benches);
23+
24+
fn bench_sort(c: &mut Criterion) {
25+
let mut group = c.benchmark_group("sort_by_parent_then_id");
26+
let mut rng = StdRng::seed_from_u64(SEED);
27+
28+
// 1) u16 id only - random order
29+
{
30+
let id_col = Arc::new(random_u16_array(&mut rng, NUM_ROWS)) as ArrayRef;
31+
let batch = make_batch(vec![("id", DataType::UInt16, id_col)]);
32+
let _ = group.bench_with_input(
33+
BenchmarkId::new("u16_id_only", "random"),
34+
&batch,
35+
|b, batch| {
36+
b.iter(|| sort_by_parent_then_id(batch.clone()).expect("sort failed"));
37+
},
38+
);
39+
}
40+
41+
// 2) u16 id only - already sorted
42+
{
43+
let id_col = Arc::new(sorted_u16_array(NUM_ROWS)) as ArrayRef;
44+
let batch = make_batch(vec![("id", DataType::UInt16, id_col)]);
45+
let _ = group.bench_with_input(
46+
BenchmarkId::new("u16_id_only", "sorted"),
47+
&batch,
48+
|b, batch| {
49+
b.iter(|| sort_by_parent_then_id(batch.clone()).expect("sort failed"));
50+
},
51+
);
52+
}
53+
54+
// 3) u16 parent_id + u16 id - random (native)
55+
{
56+
let pid = Arc::new(random_u16_array(&mut rng, NUM_ROWS)) as ArrayRef;
57+
let id_col = Arc::new(random_u16_array(&mut rng, NUM_ROWS)) as ArrayRef;
58+
let batch = make_batch(vec![
59+
("parent_id", DataType::UInt16, pid),
60+
("id", DataType::UInt16, id_col),
61+
]);
62+
let _ = group.bench_with_input(
63+
BenchmarkId::new("u16_pid_u16_id_native", "random"),
64+
&batch,
65+
|b, batch| {
66+
b.iter(|| sort_by_parent_then_id(batch.clone()).expect("sort failed"));
67+
},
68+
);
69+
}
70+
71+
// 4) u32 id + Dict<UInt8, UInt32> parent_id
72+
{
73+
let pid = random_dict_u8_u32(&mut rng, NUM_ROWS, 50);
74+
let id_col = Arc::new(random_u32_array(&mut rng, NUM_ROWS)) as ArrayRef;
75+
let dict_dt = DataType::Dictionary(Box::new(DataType::UInt8), Box::new(DataType::UInt32));
76+
let batch = make_batch(vec![
77+
("parent_id", dict_dt, pid),
78+
("id", DataType::UInt32, id_col),
79+
]);
80+
let _ = group.bench_with_input(
81+
BenchmarkId::new("u32_id_dict_u8_u32_pid", "random"),
82+
&batch,
83+
|b, batch| {
84+
b.iter(|| sort_by_parent_then_id(batch.clone()).expect("sort failed"));
85+
},
86+
);
87+
}
88+
89+
// 5) u32 id + Dict<UInt16, UInt32> parent_id
90+
{
91+
let pid = random_dict_u16_u32(&mut rng, NUM_ROWS, 200);
92+
let id_col = Arc::new(random_u32_array(&mut rng, NUM_ROWS)) as ArrayRef;
93+
let dict_dt = DataType::Dictionary(Box::new(DataType::UInt16), Box::new(DataType::UInt32));
94+
let batch = make_batch(vec![
95+
("parent_id", dict_dt, pid),
96+
("id", DataType::UInt32, id_col),
97+
]);
98+
let _ = group.bench_with_input(
99+
BenchmarkId::new("u32_id_dict_u16_u32_pid", "random"),
100+
&batch,
101+
|b, batch| {
102+
b.iter(|| sort_by_parent_then_id(batch.clone()).expect("sort failed"));
103+
},
104+
);
105+
}
106+
107+
// 6) u32 id (25% null) + Dict<UInt8, UInt32> parent_id
108+
{
109+
let pid = random_dict_u8_u32(&mut rng, NUM_ROWS, 50);
110+
let id_col = Arc::new(random_nullable_u32_array(&mut rng, NUM_ROWS)) as ArrayRef;
111+
let dict_dt = DataType::Dictionary(Box::new(DataType::UInt8), Box::new(DataType::UInt32));
112+
let batch = make_batch(vec![
113+
("parent_id", dict_dt, pid),
114+
("id", DataType::UInt32, id_col),
115+
]);
116+
let _ = group.bench_with_input(
117+
BenchmarkId::new("u32_id_null25_dict_u8_u32_pid", "random"),
118+
&batch,
119+
|b, batch| {
120+
b.iter(|| sort_by_parent_then_id(batch.clone()).expect("sort failed"));
121+
},
122+
);
123+
}
124+
125+
group.finish();
126+
}
127+
128+
fn make_batch(fields: Vec<(&str, DataType, ArrayRef)>) -> RecordBatch {
129+
let schema = Arc::new(Schema::new(
130+
fields
131+
.iter()
132+
.map(|(name, dt, _)| Field::new(*name, dt.clone(), true))
133+
.collect::<Vec<_>>(),
134+
));
135+
let columns: Vec<ArrayRef> = fields.into_iter().map(|(_, _, arr)| arr).collect();
136+
RecordBatch::try_new(schema, columns).expect("valid record batch")
137+
}
138+
139+
fn random_u16_array(rng: &mut StdRng, n: usize) -> UInt16Array {
140+
PrimitiveArray::from_iter_values((0..n).map(|_| rng.random::<u16>()))
141+
}
142+
143+
fn sorted_u16_array(n: usize) -> UInt16Array {
144+
PrimitiveArray::from_iter_values((0..n).map(|i| i as u16))
145+
}
146+
147+
fn random_u32_array(rng: &mut StdRng, n: usize) -> UInt32Array {
148+
PrimitiveArray::from_iter_values((0..n).map(|_| rng.random::<u32>()))
149+
}
150+
151+
/// Build a Dict<UInt8, UInt32> array with `n` rows, `n_values` distinct values.
152+
fn random_dict_u8_u32(rng: &mut StdRng, n: usize, n_values: usize) -> ArrayRef {
153+
let values: Vec<u32> = (0..n_values).map(|_| rng.random::<u32>()).collect();
154+
let keys: Vec<u8> = (0..n)
155+
.map(|_| rng.random_range(0..n_values as u8))
156+
.collect();
157+
let keys_arr = PrimitiveArray::<UInt8Type>::from_iter_values(keys);
158+
let values_arr = Arc::new(UInt32Array::from(values));
159+
Arc::new(DictionaryArray::new(keys_arr, values_arr))
160+
}
161+
162+
/// Build a Dict<UInt16, UInt32> array with `n` rows, `n_values` distinct values.
163+
fn random_dict_u16_u32(rng: &mut StdRng, n: usize, n_values: usize) -> ArrayRef {
164+
let values: Vec<u32> = (0..n_values).map(|_| rng.random::<u32>()).collect();
165+
let keys: Vec<u16> = (0..n)
166+
.map(|_| rng.random_range(0..n_values as u16))
167+
.collect();
168+
let keys_arr = PrimitiveArray::<UInt16Type>::from_iter_values(keys);
169+
let values_arr = Arc::new(UInt32Array::from(values));
170+
Arc::new(DictionaryArray::new(keys_arr, values_arr))
171+
}
172+
173+
/// Build a UInt32 array with `n` rows where ~25% are null.
174+
fn random_nullable_u32_array(rng: &mut StdRng, n: usize) -> UInt32Array {
175+
UInt32Array::from(
176+
(0..n)
177+
.map(|_| {
178+
if rng.random_range(0u8..4) == 0 {
179+
None
180+
} else {
181+
Some(rng.random::<u32>())
182+
}
183+
})
184+
.collect::<Vec<_>>(),
185+
)
186+
}

rust/otap-dataflow/crates/pdata/src/error.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ pub enum Error {
112112
)]
113113
UnsupportedParentIdType { actual: DataType },
114114

115+
#[error("parent_id column must not contain nulls")]
116+
NullParentId,
117+
115118
#[error("Unsupported payload type, got: {}", actual)]
116119
UnsupportedPayloadType { actual: i32 },
117120

0 commit comments

Comments
 (0)