You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/08/10 12:27:30 UTC
[GitHub] [arrow-rs] Marwes opened a new pull request, #2391: fix: Don't instantiate the scalar composition code quadratically for dictionaries
Marwes opened a new pull request, #2391:
URL: https://github.com/apache/arrow-rs/pull/2391
Instead, re-use the ones normal function. Reduces how much code `datafusion-physical-expr` generated significantly (since the functions are generic, and not instantiated in `arrow` itself, it only shows up downstream).
https://github.com/apache/arrow-datafusion
There is technically an extra indirect call now as the recursive call to `eq_dyn_scalar` etc coerces to a `dyn Array` again but that seems unlikely to matter.
## cargo llvm-lines -p datafusion-physical-expr
### Before
```
Lines Copies Function name
----- ------ -------------
2270242 (100%) 38377 (100%) (TOTAL)
245854 (10.8%) 5580 (14.5%) core::option::Option<T>::ok_or_else
58690 (2.6%) 10 (0.0%) arrow::compute::kernels::comparison::eq_dyn_scalar
58690 (2.6%) 10 (0.0%) arrow::compute::kernels::comparison::gt_dyn_scalar
58690 (2.6%) 10 (0.0%) arrow::compute::kernels::comparison::gt_eq_dyn_scalar
58690 (2.6%) 10 (0.0%) arrow::compute::kernels::comparison::lt_dyn_scalar
58690 (2.6%) 10 (0.0%) arrow::compute::kernels::comparison::lt_eq_dyn_scalar
58690 (2.6%) 10 (0.0%) arrow::compute::kernels::comparison::neq_dyn_scalar
55800 (2.5%) 900 (2.3%) arrow::compute::kernels::comparison::eq_dyn_scalar::{{closure}}
55800 (2.5%) 900 (2.3%) arrow::compute::kernels::comparison::gt_dyn_scalar::{{closure}}
55800 (2.5%) 900 (2.3%) arrow::compute::kernels::comparison::gt_eq_dyn_scalar::{{closure}}
55800 (2.5%) 900 (2.3%) arrow::compute::kernels::comparison::lt_dyn_scalar::{{closure}}
55800 (2.5%) 900 (2.3%) arrow::compute::kernels::comparison::lt_eq_dyn_scalar::{{closure}}
55800 (2.5%) 900 (2.3%) arrow::compute::kernels::comparison::neq_dyn_scalar::{{closure}}
44929 (2.0%) 900 (2.3%) core::option::Option<T>::map
40986 (1.8%) 162 (0.4%) <arrow::array::array_boolean::BooleanArray as core::iter::traits::collect::FromIterator<Ptr>>::from_iter
37528 (1.7%) 508 (1.3%) core::iter::traits::iterator::Iterator::fold
30595 (1.3%) 245 (0.6%) <alloc::vec::Vec<T> as alloc::vec::spec_from_iter_nested::SpecFromIterNested<T,I>>::from_iter
29272 (1.3%) 46 (0.1%) <core::iter::adapters::flatten::FlattenCompat<I,U> as core::iter::traits::iterator::Iterator>::size_hint
27815 (1.2%) 285 (0.7%) core::iter::traits::iterator::Iterator::try_fold
26014 (1.1%) 1 (0.0%) datafusion_physical_expr::expressions::binary::BinaryExpr::evaluate_array_scalar
25095 (1.1%) 441 (1.1%) core::iter::adapters::map::map_fold::{{closure}}
22849 (1.0%) 174 (0.5%) <core::iter::adapters::GenericShunt<I,R> as core::iter::traits::iterator::Iterator>::try_fold::{{closure}}
21888 (1.0%) 96 (0.3%) arrow::compute::kernels::comparison::compare_op_scalar
21464 (0.9%) 56 (0.1%) <arrow::array::array_string::GenericStringArray<OffsetSize> as core::iter::traits::collect::FromIterator<core::option::Option<Ptr>>>::from_iter
21461 (0.9%) 441 (1.1%) <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold
19918 (0.9%) 118 (0.3%) arrow::buffer::mutable::MutableBuffer::from_trusted_len_iter
16916 (0.7%) 246 (0.6%) <alloc::vec::Vec<T,A> as alloc::vec::spec_extend::SpecExtend<T,I>>::spec_extend
```
### After
```
Lines Copies Function name
----- ------ -------------
1475122 (100%) 28777 (100%) (TOTAL)
44929 (3.0%) 900 (3.1%) core::option::Option<T>::map
40986 (2.8%) 162 (0.6%) <arrow::array::array_boolean::BooleanArray as core::iter::traits::collect::FromIterator<Ptr>>::from_iter
37528 (2.5%) 508 (1.8%) core::iter::traits::iterator::Iterator::fold
34174 (2.3%) 780 (2.7%) core::option::Option<T>::ok_or_else
30595 (2.1%) 245 (0.9%) <alloc::vec::Vec<T> as alloc::vec::spec_from_iter_nested::SpecFromIterNested<T,I>>::from_iter
29272 (2.0%) 46 (0.2%) <core::iter::adapters::flatten::FlattenCompat<I,U> as core::iter::traits::iterator::Iterator>::size_hint
27815 (1.9%) 285 (1.0%) core::iter::traits::iterator::Iterator::try_fold
26014 (1.8%) 1 (0.0%) datafusion_physical_expr::expressions::binary::BinaryExpr::evaluate_array_scalar
25095 (1.7%) 441 (1.5%) core::iter::adapters::map::map_fold::{{closure}}
22849 (1.5%) 174 (0.6%) <core::iter::adapters::GenericShunt<I,R> as core::iter::traits::iterator::Iterator>::try_fold::{{closure}}
21888 (1.5%) 96 (0.3%) arrow::compute::kernels::comparison::compare_op_scalar
21464 (1.5%) 56 (0.2%) <arrow::array::array_string::GenericStringArray<OffsetSize> as core::iter::traits::collect::FromIterator<core::option::Option<Ptr>>>::from_iter
21461 (1.5%) 441 (1.5%) <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold
19918 (1.4%) 118 (0.4%) arrow::buffer::mutable::MutableBuffer::from_trusted_len_iter
16916 (1.1%) 246 (0.9%) <alloc::vec::Vec<T,A> as alloc::vec::spec_extend::SpecExtend<T,I>>::spec_extend
16146 (1.1%) 960 (3.3%) core::iter::adapters::map::Map<I,F>::new
15492 (1.1%) 427 (1.5%) core::iter::traits::iterator::Iterator::for_each
14921 (1.0%) 111 (0.4%) alloc::vec::Vec<T,A>::extend_desugared
14670 (1.0%) 126 (0.4%) core::iter::adapters::try_process
13918 (0.9%) 1 (0.0%) datafusion_physical_expr::expressions::binary::BinaryExpr::evaluate_scalar_array
13120 (0.9%) 64 (0.2%) <arrow::array::array_primitive::PrimitiveArray<T> as core::iter::traits::collect::FromIterator<Ptr>>::from_iter
12963 (0.9%) 52 (0.2%) <core::iter::adapters::flatten::FlattenCompat<I,U> as core::iter::traits::iterator::Iterator>::try_fold
12245 (0.8%) 180 (0.6%) <core::iter::adapters::enumerate::Enumerate<I> as core::iter::traits::iterator::Iterator>::fold::enumerate::{{closure}}
12201 (0.8%) 81 (0.3%) arrow::buffer::mutable::MutableBuffer::extend_from_iter
11826 (0.8%) 162 (0.6%) <arrow::array::array_boolean::BooleanArray as core::iter::traits::collect::FromIterator<Ptr>>::from_iter::{{closure}}
11536 (0.8%) 960 (3.3%) core::iter::traits::iterator::Iterator::map
11200 (0.8%) 32 (0.1%) alloc::raw_vec::RawVec<T,A>::grow_amortized
```
# Rationale for this change
Cuts down (release) compile times when these functions are used (in datafusion-physical-expr)
### Before
```
________________________________________________________
Executed in 147,60 secs fish external
usr time 323,32 secs 693,00 micros 323,32 secs
sys time 9,43 secs 187,00 micros 9,43 secs
```
### After
```
Executed in 133,27 secs fish external
usr time 312,95 secs 498,00 micros 312,95 secs
sys time 9,94 secs 134,00 micros 9,94 secs
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on a diff in pull request #2391: fix: Don't instantiate the scalar composition code quadratically for dictionaries
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2391:
URL: https://github.com/apache/arrow-rs/pull/2391#discussion_r943406426
##########
arrow/src/compute/kernels/comparison.rs:
##########
@@ -1186,7 +1163,7 @@ where
{
match left.data_type() {
DataType::Dictionary(key_type, _value_type) => {
- dyn_compare_scalar!(left, right, key_type, eq_scalar)
+ dyn_compare_scalar!(left, right, key_type, eq_dyn_scalar)
Review Comment:
❤️
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] ursabot commented on pull request #2391: fix: Don't instantiate the scalar composition code quadratically for dictionaries
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #2391:
URL: https://github.com/apache/arrow-rs/pull/2391#issuecomment-1211905720
Benchmark runs are scheduled for baseline = 6b1369e655154d5873b499859fe80fa50f505ca4 and contender = 6948373f147815753ac3d34bb3a73a8ba2079873. 6948373f147815753ac3d34bb3a73a8ba2079873 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/6c20829e543e4a3eae78c0590e65a80e...1815ad15d60349e5acdf6c88cba0b62b/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/5e528dc753304696a449280bcd0e8c39...0cb069ce7b014236b88a2b7784f4f592/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/abe6e78022a142338884170720e05e0e...78398989672d4770857b9a5a76e8f829/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5023ab1cb6084c39841d7e4c6b031a8e...c78aabb7e3b6446c93d35185e521ba27/)
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb merged pull request #2391: fix: Don't instantiate the scalar composition code quadratically for dictionaries
Posted by GitBox <gi...@apache.org>.
alamb merged PR #2391:
URL: https://github.com/apache/arrow-rs/pull/2391
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org