Skip to content

Commit 265ca2d

Browse files
committed
performance[vortex-array]: don't call is_valid to count bytes in varbinview
I noticed that the better blocks compressor uses count_referenced_bytes which calls is_valid on each view and results in an expensive scalar_at call. This was 30% of our system-wide CPU usage over 12 hours (see PR for attached screenshot). This commit iterates over the nullability mask directly to avoid these expensive calls. Signed-off-by: Alfonso Subiotto Marques <[email protected]>
1 parent 265f29a commit 265ca2d

File tree

1 file changed

+67
-15
lines changed

1 file changed

+67
-15
lines changed

vortex-array/src/arrays/varbinview/compact.rs

Lines changed: 67 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -60,23 +60,51 @@ impl VarBinViewArray {
6060
fn count_referenced_bytes(&self) -> u64 {
6161
match self.validity() {
6262
Validity::AllInvalid => 0u64,
63-
Validity::NonNullable | Validity::AllValid | Validity::Array(_) => self
63+
Validity::NonNullable | Validity::AllValid => self
6464
.views()
6565
.iter()
66-
.enumerate()
67-
.map(|(idx, &view)| {
68-
if !self.is_valid(idx) || view.is_inlined() {
66+
.map(|&view| {
67+
if view.is_inlined() {
6968
0u64
7069
} else {
7170
view.len() as u64
7271
}
7372
})
7473
.sum(),
74+
Validity::Array(_) => {
75+
let mask = self.validity().to_mask(self.len());
76+
match mask.bit_buffer() {
77+
vortex_mask::AllOr::All => self
78+
.views()
79+
.iter()
80+
.map(|&view| {
81+
if view.is_inlined() {
82+
0u64
83+
} else {
84+
view.len() as u64
85+
}
86+
})
87+
.sum(),
88+
vortex_mask::AllOr::None => 0u64,
89+
vortex_mask::AllOr::Some(bit_buffer) => self
90+
.views()
91+
.iter()
92+
.zip(bit_buffer.iter())
93+
.map(|(&view, is_valid)| {
94+
if !is_valid || view.is_inlined() {
95+
0u64
96+
} else {
97+
view.len() as u64
98+
}
99+
})
100+
.sum(),
101+
}
102+
}
75103
}
76104
}
77105

78106
pub(crate) fn buffer_utilizations(&self) -> Vec<BufferUtilization> {
79-
let mut utilizations = self
107+
let mut utilizations: Vec<BufferUtilization> = self
80108
.buffers()
81109
.iter()
82110
.map(|buf| {
@@ -85,17 +113,41 @@ impl VarBinViewArray {
85113
})
86114
.collect();
87115

88-
if matches!(self.validity(), Validity::AllInvalid) {
89-
return utilizations;
90-
}
91-
92-
for (idx, &view) in self.views().iter().enumerate() {
93-
if !self.is_valid(idx) || view.is_inlined() {
94-
continue;
116+
match self.validity() {
117+
Validity::AllInvalid => {}
118+
Validity::NonNullable | Validity::AllValid => {
119+
for &view in self.views().iter() {
120+
if view.is_inlined() {
121+
continue;
122+
}
123+
let view = view.as_view();
124+
utilizations[view.buffer_index as usize].add(view.offset, view.size);
125+
}
126+
}
127+
Validity::Array(_) => {
128+
let mask = self.validity().to_mask(self.len());
129+
match mask.bit_buffer() {
130+
vortex_mask::AllOr::All => {
131+
for &view in self.views().iter() {
132+
if view.is_inlined() {
133+
continue;
134+
}
135+
let view = view.as_view();
136+
utilizations[view.buffer_index as usize].add(view.offset, view.size);
137+
}
138+
}
139+
vortex_mask::AllOr::None => {}
140+
vortex_mask::AllOr::Some(bit_buffer) => {
141+
for (&view, is_valid) in self.views().iter().zip(bit_buffer.iter()) {
142+
if !is_valid || view.is_inlined() {
143+
continue;
144+
}
145+
let view = view.as_view();
146+
utilizations[view.buffer_index as usize].add(view.offset, view.size);
147+
}
148+
}
149+
}
95150
}
96-
let view = view.as_view();
97-
98-
utilizations[view.buffer_index as usize].add(view.offset, view.size)
99151
}
100152

101153
utilizations

0 commit comments

Comments
 (0)