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

Improved performance of list iterator (- 10-20%) #441

Merged
merged 2 commits into from
Sep 25, 2021

Conversation

ritchie46
Copy link
Collaborator

This PR elides some of the bounds checks done while iteration list values. The slicing still does bound checks and IMO should be one of the invariants of the offsets (being in bounds). I want to propose a slice_unchecked on the Array trait. Let me know what you think.

This optimization might also work for FixedSizeList if we add a value_unchecked to the array.

Gnuplot not found, using plotters backend
list: iter_values 2^10  time:   [15.214 us 15.217 us 15.220 us]                                    
                        change: [-12.125% -12.063% -11.995%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
  1 (1.00%) low severe
  2 (2.00%) high mild
  8 (8.00%) high severe

list: iter 2^10         time:   [15.274 us 15.277 us 15.280 us]                             
                        change: [-12.382% -12.065% -11.801%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe

list: iter_values 2^12  time:   [60.861 us 60.869 us 60.878 us]                                   
                        change: [-11.760% -9.6523% -7.8631%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
  1 (1.00%) low severe
  7 (7.00%) high mild
  3 (3.00%) high severe

list: iter 2^12         time:   [61.928 us 61.995 us 62.076 us]                            
                        change: [-5.0382% -4.4427% -3.8211%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe

list: iter_values 2^14  time:   [244.38 us 244.41 us 244.44 us]                                   
                        change: [-8.0274% -7.9989% -7.9730%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  1 (1.00%) low severe
  1 (1.00%) high mild
  5 (5.00%) high severe

list: iter 2^14         time:   [245.20 us 245.23 us 245.26 us]                            
                        change: [-21.663% -18.963% -16.142%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
  1 (1.00%) high mild
  3 (3.00%) high severe

Benchmarking list: iter_values 2^16: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 5.4s, enable flat sampling, or reduce sample count to 60.
list: iter_values 2^16  time:   [1.0023 ms 1.0034 ms 1.0051 ms]                                    
                        change: [-3.7442% -3.3126% -2.7332%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 14 outliers among 100 measurements (14.00%)
  5 (5.00%) high mild
  9 (9.00%) high severe

Benchmarking list: iter 2^16: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 5.2s, enable flat sampling, or reduce sample count to 60.
list: iter 2^16         time:   [1.0243 ms 1.0254 ms 1.0266 ms]                             
                        change: [-0.3685% -0.1351% +0.1820%] (p = 0.35 > 0.05)
                        No change in performance detected.
Found 19 outliers among 100 measurements (19.00%)
  8 (8.00%) low mild
  8 (8.00%) high mild
  3 (3.00%) high severe

list: iter_values 2^18  time:   [3.9003 ms 3.9005 ms 3.9008 ms]                                    
                        change: [-11.058% -10.852% -10.638%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  1 (1.00%) low mild
  2 (2.00%) high mild
  4 (4.00%) high severe

list: iter 2^18         time:   [3.9241 ms 3.9244 ms 3.9248 ms]                             
                        change: [-6.2696% -6.1584% -6.0561%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
  1 (1.00%) high mild
  3 (3.00%) high severe

list: iter_values 2^20  time:   [15.604 ms 15.606 ms 15.607 ms]                                   
                        change: [-7.4029% -7.2619% -7.1323%] (p = 0.00 < 0.05)
                        Performance has improved.

list: iter 2^20         time:   [15.923 ms 16.046 ms 16.204 ms]                            
                        change: [-4.1817% -3.4015% -2.4924%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)

@codecov
Copy link

codecov bot commented Sep 23, 2021

Codecov Report

Merging #441 (1a1425d) into main (688e979) will decrease coverage by 0.14%.
The diff coverage is 78.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #441      +/-   ##
==========================================
- Coverage   80.77%   80.63%   -0.15%     
==========================================
  Files         372      372              
  Lines       22637    22726      +89     
==========================================
+ Hits        18286    18324      +38     
- Misses       4351     4402      +51     
Impacted Files Coverage Δ
src/array/mod.rs 55.55% <ø> (-3.42%) ⬇️
src/array/null.rs 35.71% <0.00%> (-2.75%) ⬇️
src/array/union/mod.rs 74.31% <0.00%> (-6.69%) ⬇️
src/array/fixed_size_binary/mod.rs 45.33% <70.00%> (+2.04%) ⬆️
src/array/list/iterator.rs 62.50% <75.00%> (ø)
src/array/fixed_size_list/mod.rs 51.42% <76.92%> (+3.97%) ⬆️
src/array/list/mod.rs 69.76% <83.33%> (-2.03%) ⬇️
src/array/struct_.rs 34.06% <88.88%> (-3.29%) ⬇️
src/array/binary/mod.rs 77.77% <90.00%> (-1.91%) ⬇️
src/array/boolean/mod.rs 67.34% <90.00%> (+3.93%) ⬆️
... and 11 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 688e979...1a1425d. Read the comment docs.

@jorgecarleitao
Copy link
Owner

Thanks!

Yeap, unchecked slicing makes sense 👍

Also adds a slice unchecked and dispatches
slice method to the unchecked. This also
reduces a double assert to a single assert
in safe code. As the bounds were checked
at the array buffer and at the validity.
@ritchie46
Copy link
Collaborator Author

@jorgecarleitao I also added the unchecked_slice here. This reduces the bounds checking in safe code from two checks to a single one.

Now the bounds are asserted array::slice and the implementation is dispatched to the unchecked function.
Before the bounds were asserted in buffer::slice and in validity::slice.

Copy link
Owner

@jorgecarleitao jorgecarleitao left a comment

Choose a reason for hiding this comment

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

Went through it carefully and all checks great. Left some nits to apply, but it is good to go.

Thanks a lot, @ritchie46 ! 💯

src/array/dictionary/mod.rs Outdated Show resolved Hide resolved
src/array/dictionary/mod.rs Show resolved Hide resolved
src/buffer/immutable.rs Outdated Show resolved Hide resolved
src/bitmap/immutable.rs Outdated Show resolved Hide resolved
src/buffer/immutable.rs Outdated Show resolved Hide resolved
src/array/fixed_size_list/mod.rs Outdated Show resolved Hide resolved
src/array/fixed_size_binary/mod.rs Outdated Show resolved Hide resolved
src/array/fixed_size_binary/mod.rs Outdated Show resolved Hide resolved
src/array/boolean/mod.rs Outdated Show resolved Hide resolved
src/array/binary/mod.rs Outdated Show resolved Hide resolved
@jorgecarleitao jorgecarleitao merged commit fa11752 into jorgecarleitao:main Sep 25, 2021
@jorgecarleitao jorgecarleitao changed the title elide partial bounds check in list iterator Improved performance of list iterator (-10%) Sep 25, 2021
@jorgecarleitao jorgecarleitao added the enhancement An improvement to an existing feature label Sep 25, 2021
@jorgecarleitao jorgecarleitao changed the title Improved performance of list iterator (-10%) Improved performance of list iterator (- 10-20%) Sep 25, 2021
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