Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Speedup scalar boolean operations #546

Merged
merged 8 commits into from
Oct 22, 2021
Merged

Speedup scalar boolean operations #546

merged 8 commits into from
Oct 22, 2021

Conversation

Dandandan
Copy link
Collaborator

@Dandandan Dandandan commented Oct 22, 2021 •
edited
Loading

By simplifying some code (20-25%) faster.

@Dandandan Dandandan changed the title Speedup scalar bool Speedup scalar boolean operations Oct 22, 2021
Dandandan
Dandandan commented Oct 22, 2021
View reviewed changes
benches/comparison_kernels.rs
@@ -40,7 +40,7 @@ fn add_benchmark(c: &mut Criterion) {
b.iter(|| bench_op(&arr_a, &arr_b, Operator::Eq))
});
c.bench_function(&format!("bool scalar 2^{}", log2_size), |b| {
b.iter(|| bench_op_scalar(&arr_a, &BooleanScalar::from(Some(true)), Operator::Eq))
b.iter(|| bench_op_scalar(&arr_a, &BooleanScalar::from(Some(false)), Operator::Eq))
Copy link
Collaborator Author

@Dandandan Dandandan Oct 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Otherwise the benchmark measures lhs.clone() which doesn't increase with size - and is way faster (>20000x)

@Dandandan
Copy link
Collaborator Author

@jorgecarleitao any reason the code works at u8 at a time instead of u64? I guess the latter might be quite a bit faster for these kind of operations?

@codecov
Copy link

codecov bot commented Oct 22, 2021 •
edited
Loading

Codecov Report

Merging #546 (1535901) into main (bd8f02b) will increase coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #546      +/-   ##
==========================================
+ Coverage   81.25%   81.32%   +0.06%     
==========================================
  Files         380      380              
  Lines       23376    23414      +38     
==========================================
+ Hits        18995    19042      +47     
+ Misses       4381     4372       -9
Impacted Files Coverage Δ
src/compute/comparison/boolean.rs 81.60% <100.00%> (+11.49%) ⬆️
src/bitmap/utils/slice_iterator.rs 92.53% <0.00%> (-1.50%) ⬇️
src/io/parquet/write/mod.rs 65.64% <0.00%> (ø)
src/io/ipc/write/common.rs 76.28% <0.00%> (+0.64%) ⬆️
src/compute/arithmetics/time.rs 46.93% <0.00%> (+2.04%) ⬆️
src/array/specification.rs 83.33% <0.00%> (+2.08%) ⬆️
tests/it/array/utf8/mod.rs 100.00% <0.00%> (+2.35%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd8f02b...1535901. Read the comment docs.

@jorgecarleitao jorgecarleitao merged commit b424e90 into jorgecarleitao:main Oct 22, 2021
@jorgecarleitao jorgecarleitao added the enhancement An improvement to an existing feature label Oct 22, 2021
@jorgecarleitao
Copy link
Owner

No idea why the u8. Maybe I tried and it was not faster? Do not remember :X

@jorgecarleitao
Copy link
Owner

Thanks a lot, @Dandandan !

@Dandandan
Copy link
Collaborator Author

No idea why the u8. Maybe I tried and it was not faster? Do not remember :X

Ok, I still wonder - maybe I run some tests and compare with the same operations in arrow. I'm assuming operating on 64 bits at a time would be faster.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement An improvement to an existing feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Dandandan @jorgecarleitao