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

Added IndexRange to remove checks in hot loops #247

Merged
merged 2 commits into from
Aug 4, 2021
Merged

Conversation

jorgecarleitao
Copy link
Owner

@jorgecarleitao jorgecarleitao commented Aug 4, 2021

This improves performance of sort by removing checks from inside hot loops.

Backward incompatible changes

The trait Index is now available at arrow2::types::Index, instead of arrow2::array::Index

@jorgecarleitao jorgecarleitao added enhancement An improvement to an existing feature backwards-incompatible labels Aug 4, 2021
@jorgecarleitao jorgecarleitao mentioned this pull request Aug 4, 2021
@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #247 (5fbe923) into main (3d123b3) will increase coverage by 0.12%.
The diff coverage is 73.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #247      +/-   ##
==========================================
+ Coverage   76.69%   76.81%   +0.12%     
==========================================
  Files         229      230       +1     
  Lines       19687    19666      -21     
==========================================
+ Hits        15099    15107       +8     
+ Misses       4588     4559      -29     
Impacted Files Coverage Δ
src/array/mod.rs 52.25% <ø> (ø)
src/array/specification.rs 70.96% <ø> (+28.99%) ⬆️
src/buffer/mutable.rs 91.76% <ø> (+0.24%) ⬆️
src/compute/sort/boolean.rs 89.47% <ø> (ø)
src/compute/sort/lex_sort.rs 82.14% <ø> (ø)
src/compute/sort/mod.rs 53.55% <ø> (ø)
src/compute/sort/primitive/indices.rs 100.00% <ø> (ø)
src/compute/sort/utf8.rs 100.00% <ø> (ø)
src/compute/take/mod.rs 88.48% <ø> (ø)
src/types/mod.rs 52.00% <ø> (ø)
... and 4 more

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 3d123b3...5fbe923. Read the comment docs.

@sundy-li
Copy link
Collaborator

sundy-li commented Aug 4, 2021

LGTM, do we have benchmark results vs native ranges?

@jorgecarleitao jorgecarleitao changed the title Added IndexRange to improve performance in sort. Added IndexRange to remove checks in hot loops Aug 4, 2021
@jorgecarleitao
Copy link
Owner Author

Thanks for the review. This has no impact in performance. I think it makes it easier to read and rules out the from_usize bit. Thus, merging :)

@jorgecarleitao
Copy link
Owner Author

sort-limit 2^10 f32     time:   [12.611 us 12.639 us 12.666 us]                                 
                        change: [-0.3425% +0.2052% +0.9419%] (p = 0.61 > 0.05)
                        No change in performance detected.
Found 14 outliers among 100 measurements (14.00%)
  6 (6.00%) low severe
  4 (4.00%) low mild
  1 (1.00%) high mild
  3 (3.00%) high severe

sort-limit 2^12 f32     time:   [25.693 us 25.705 us 25.717 us]                                 
                        change: [-0.6963% -0.2300% +0.0666%] (p = 0.31 > 0.05)
                        No change in performance detected.
Found 13 outliers among 100 measurements (13.00%)
  4 (4.00%) low severe
  4 (4.00%) low mild
  4 (4.00%) high mild
  1 (1.00%) high severe

sort-limit 2^14 f32     time:   [78.742 us 78.811 us 78.871 us]                                
                        change: [-0.8476% -0.5057% -0.1062%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 16 outliers among 100 measurements (16.00%)
  4 (4.00%) low severe
  6 (6.00%) low mild
  4 (4.00%) high mild
  2 (2.00%) high severe

sort-limit 2^16 f32     time:   [448.16 us 449.77 us 452.01 us]                                
                        change: [-0.8242% -0.4075% +0.0675%] (p = 0.07 > 0.05)
                        No change in performance detected.
Found 10 outliers among 100 measurements (10.00%)
  2 (2.00%) low severe
  3 (3.00%) low mild
  1 (1.00%) high mild
  4 (4.00%) high severe

Benchmarking sort-limit 2^18 f32: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 8.7s, enable flat sampling, or reduce sample count to 50.
sort-limit 2^18 f32     time:   [1.7144 ms 1.7167 ms 1.7198 ms]                                 
                        change: [-0.6946% -0.4104% -0.1233%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 18 outliers among 100 measurements (18.00%)
  4 (4.00%) low severe
  7 (7.00%) low mild
  3 (3.00%) high mild
  4 (4.00%) high severe

sort-limit 2^20 f32     time:   [9.0083 ms 9.0169 ms 9.0250 ms]                                
                        change: [+0.1708% +0.3120% +0.4439%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 11 outliers among 100 measurements (11.00%)
  5 (5.00%) low severe
  2 (2.00%) low mild
  2 (2.00%) high mild
  2 (2.00%) high sever

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement An improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants