Skip to content

Commit d674aa2

Browse files
authored
fix[vortex-array]: avoid unnecessary checks in check_listview_constant (#7423)
## Summary ListView sizes are checked on each element iteration. This can cause unecessary calls to arrays_value_equal if most but not all sizes are equal. This commit pulls up some fast checks we can do before executing the expensive arrays_value_equal loop. Concretely, only execute the loop if all sizes are equal *and* non-zero *and* offsets are not all equal, since the inverse is trivially constant. [Profile link here, we're seeing that this check is expensive](https://pprof.me/39a57fd47bacf99f6669aa6ba5beed31/?profile_filters=p%3Ahide_libc%3AHide%2520libc%2Cp%3Ahide_tokio_frames%3AHide%2520Tokio%2520Frames%2Cp%3Ahide_rust_futures%3AHide%2520Rust%2520Futures%2520Infrastructure%2Cp%3Ahide_rust_panic_backtrace%3AHide%2520Rust%2520Panic%2520Backtrace%2520Infrastructure) Signed-off-by: Alfonso Subiotto Marques <alfonso.subiotto@polarsignals.com>
1 parent d222c87 commit d674aa2

1 file changed

Lines changed: 16 additions & 7 deletions

File tree

  • vortex-array/src/aggregate_fn/fns/is_constant

vortex-array/src/aggregate_fn/fns/is_constant/list.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use vortex_error::VortexResult;
55

66
use super::arrays_value_equal;
7+
use super::is_constant;
78
use crate::ExecutionCtx;
89
use crate::arrays::ListViewArray;
910
use crate::arrays::listview::ListViewArrayExt;
@@ -20,16 +21,24 @@ pub(super) fn check_listview_constant(
2021
return Ok(true);
2122
}
2223

24+
if !is_constant(l.sizes(), ctx)? {
25+
// If sizes aren't all equal, can't be constant.
26+
return Ok(false);
27+
}
28+
2329
let first_size = l.size_at(0);
24-
let first_elements = l.list_elements_at(0)?;
30+
if first_size == 0 {
31+
return Ok(true);
32+
}
33+
34+
if is_constant(l.offsets(), ctx)? {
35+
// If all offsets are identical, every list references the same slice.
36+
return Ok(true);
37+
}
2538

39+
// Check each list individually, this can be expensive.
40+
let first_elements = l.list_elements_at(0)?;
2641
for i in 1..l.len() {
27-
if l.size_at(i) != first_size {
28-
return Ok(false);
29-
}
30-
if first_size == 0 {
31-
continue;
32-
}
3342
let current_elements = l.list_elements_at(i)?;
3443
if !arrays_value_equal(&first_elements, &current_elements, ctx)? {
3544
return Ok(false);

0 commit comments

Comments
 (0)