Faster primitive arrays encoding into row format

[korowa]

Which issue does this PR close?

--

Rationale for this change

Performance improvements for encoding primitive arrays into row format

cargo bench

eduard@Aspire-A515-58M:~/projects/arrow-rs$ cargo bench --bench row_format -- "4096 (u64|i64|bool)\(.+\)$" --baseline master
   Compiling arrow-row v52.0.0 (/home/eduard/projects/arrow-rs/arrow-row)
   Compiling arrow v52.0.0 (/home/eduard/projects/arrow-rs/arrow)
    Finished `bench` profile [optimized] target(s) in 3.73s
     Running benches/row_format.rs (target/release/deps/row_format-fa578ffd3df1630f)
convert_columns 4096 u64(0)
                        time:   [8.0609 µs 8.0699 µs 8.0791 µs]
                        change: [-53.318% -53.191% -53.061%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
  2 (2.00%) high mild
  2 (2.00%) high severe

convert_columns_prepared 4096 u64(0)
                        time:   [7.7778 µs 7.7872 µs 7.7972 µs]
                        change: [-54.520% -54.429% -54.345%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe

convert_rows 4096 u64(0)
                        time:   [27.487 µs 27.495 µs 27.503 µs]
                        change: [-0.1673% -0.0151% +0.0911%] (p = 0.86 > 0.05)
                        No change in performance detected.
Found 7 outliers among 100 measurements (7.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
  3 (3.00%) high severe

convert_columns 4096 u64(0.3)
                        time:   [13.647 µs 13.663 µs 13.678 µs]
                        change: [-32.089% -31.925% -31.755%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) low mild

convert_columns_prepared 4096 u64(0.3)
                        time:   [13.807 µs 13.819 µs 13.831 µs]
                        change: [-34.109% -32.018% -30.597%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) low mild

convert_rows 4096 u64(0.3)
                        time:   [28.066 µs 28.079 µs 28.093 µs]
                        change: [-1.8794% +0.0417% +1.4081%] (p = 0.96 > 0.05)
                        No change in performance detected.
Found 4 outliers among 100 measurements (4.00%)
  3 (3.00%) high mild
  1 (1.00%) high severe

convert_columns 4096 i64(0)
                        time:   [8.0324 µs 8.0352 µs 8.0381 µs]
                        change: [-53.139% -53.071% -52.969%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
  3 (3.00%) high severe

convert_columns_prepared 4096 i64(0)
                        time:   [7.9678 µs 7.9719 µs 7.9761 µs]
                        change: [-55.284% -54.859% -54.449%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) high mild
  5 (5.00%) high severe

convert_rows 4096 i64(0)
                        time:   [27.541 µs 27.549 µs 27.556 µs]
                        change: [-0.0051% +0.1256% +0.2436%] (p = 0.05 < 0.05)
                        Change within noise threshold.
Found 5 outliers among 100 measurements (5.00%)
  1 (1.00%) low mild
  3 (3.00%) high mild
  1 (1.00%) high severe

convert_columns 4096 i64(0.3)
                        time:   [13.129 µs 13.131 µs 13.134 µs]
                        change: [-19.818% -19.751% -19.683%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) high mild
  2 (2.00%) high severe

convert_columns_prepared 4096 i64(0.3)
                        time:   [13.095 µs 13.101 µs 13.106 µs]
                        change: [-21.096% -20.958% -20.828%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) low mild
  2 (2.00%) high severe

convert_rows 4096 i64(0.3)
                        time:   [27.968 µs 27.977 µs 27.987 µs]
                        change: [+0.0320% +0.1985% +0.3580%] (p = 0.02 < 0.05)
                        Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) low mild
  2 (2.00%) high severe

convert_columns 4096 bool(0, 0.5)
                        time:   [7.8909 µs 7.8948 µs 7.8991 µs]
                        change: [-22.996% -22.855% -22.751%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  1 (1.00%) low mild
  6 (6.00%) high mild

convert_columns_prepared 4096 bool(0, 0.5)
                        time:   [7.9600 µs 7.9637 µs 7.9679 µs]
                        change: [-22.409% -22.205% -22.021%] (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

convert_rows 4096 bool(0, 0.5)
                        time:   [17.700 µs 17.711 µs 17.722 µs]
                        change: [-3.4067% -3.1262% -2.8614%] (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

convert_columns 4096 bool(0.3, 0.5)
                        time:   [10.999 µs 11.003 µs 11.009 µs]
                        change: [-8.1531% -8.0434% -7.9374%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  6 (6.00%) high mild
  2 (2.00%) high severe

convert_columns_prepared 4096 bool(0.3, 0.5)
                        time:   [11.021 µs 11.036 µs 11.051 µs]
                        change: [-6.6325% -6.2628% -5.9016%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  4 (4.00%) high mild
  2 (2.00%) high severe

convert_rows 4096 bool(0.3, 0.5)
                        time:   [17.712 µs 17.720 µs 17.728 µs]
                        change: [-2.0989% -1.8996% -1.7319%] (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

What changes are included in this PR?

• In some cases, skip() iterator may noticeably affect performance -- encoding primitive arrays into Arrow rows is one of them -- removing skip().zip() shows significant improvement over current master. (FWIW: +- same trick for fixed binary / string columns didn't show any meaningful / stable improvements.)
• Special casing non-nullable primitive arrays -- iterating over values allows not to perform null-checks, which also noticeably speeds things up.

Are there any user-facing changes?

No

[alamb]

Thank you @korowa -- I went through this code carefully and it makes sense to me

Thank you for the optimization

I think it would be good if @tustvold had a chance to review this one too before we merged it in.

[alamb]

I took the liberty of merging this PR up from main to pick up the fixes for the MSRV CI chekc

[tustvold]

Typically we use column.nulls().filter(|n| n.null_count() > 0) or similar for switching on nullability, this allows getting the benefit if there are no nulls regardless of schema, but happy for this to be done as a separate PR

[korowa]

After switching to this expression, I've found out that it's more beneficial to iterate over null, picking valid values from values -- now nullable columns conversions is +- close to non-nullable

[tustvold]

Perhaps LLVM is smart enough, but it might be interesting to see if pulling this out of the loop improves performance

[korowa]

I guess it'll be a follow up (need a bit more time with it) -- as for now after quick check, for non-nullable columns, encoded with descending order

    if opts.descending {
        // Flip bits to reverse order for encoded data skipping validity byte
        for offset_idx in 1..offsets.len() {
            let end_offset = offsets[offset_idx];
            let buf = &mut data[end_offset-T::ENCODED_LEN+1..end_offset];
            buf.iter_mut().for_each(|v| *v = !*v);
        }
    }

after "main" loop shows [9.7890 µs 9.9628 µs 10.196 µs] vs [12.391 µs 12.448 µs 12.545 µs], so there may be an improvement, but it's a bit unclear for nullable columns (don't have any solution with meaningful improvement yet).

[tustvold]

This looks good to me, I left some suggestions that could always be done as follows up.

I have not verified the benchmarks on a server, but whomever merges this should do this first

[alamb]

I am running benchmarks now on a server

[alamb]

Performance results (I measured about 20% performance improvement)

group                                                                                                                         encode-skip-iter                       master
-----                                                                                                                         ----------------                       ------

convert_rows 4096 bool(0, 0.5)                                                                                                1.20     28.6±0.07µs        ? ?/sec    1.00     23.8±0.08µs        ? ?/sec
convert_rows 4096 bool(0.3, 0.5)                                                                                              1.20     28.6±0.05µs        ? ?/sec    1.00     23.8±0.08µs        ? ?/sec
convert_rows 4096 i64(0)                                                                                                      1.21     48.8±0.06µs        ? ?/sec    1.00     40.3±0.08µs        ? ?/sec
convert_rows 4096 i64(0.3)                                                                                                    1.21     48.9±0.06µs        ? ?/sec    1.00     40.3±0.13µs        ? ?/sec
convert_rows 4096 u64(0)                                                                                                      1.20     48.0±0.07µs        ? ?/sec    1.00     39.9±0.08µs        ? ?/sec
convert_rows 4096 u64(0.3)                                                                                                    1.20     48.1±0.14µs        ? ?/sec    1.00     40.0±0.04µs        ? ?/sec

[alamb]

@korowa -- please let me know what you would like to do with this PR. We can merge it as is, or would you like to try some of @tustvold 's suggestions?

[alamb]

(we could try them as follow on PRs too)

[korowa]

@alamb @tustvold sorry for the late response.

Actually benchmark results are a bit unexpected (since encoding performance is the same for nullable and non-nullable columns), so I'd like to verify them, and also work on @tustvold 's comments.

Thank you.

[alamb]

To make running benchmarks easier, perhaps we could pull them out into their own PR that we could merge into master independently to this PR

[korowa]

@alamb @tustvold I suppose this version to be ready for merging -- could you, please, take a look when you have time?

And regarding benchmark results -- all the changes affect only convert_columns benchmarks, convert_rows ones should not be affected by this PR.

[alamb]

@alamb @tustvold I suppose this version to be ready for merging -- could you, please, take a look when you have time?

Thanks @korowa -- I am running the benchmarks again and I'll give this another look

[alamb]

🤔 I reran the benchmarks and it actually looks to me like this branch is slower than main (and looking at the results in #5858 (review) I think I misread them and they show this branch is slower too)

Here are the results:

group                                                                                                                         encode-skip-iter                       master
-----                                                                                                                         ----------------                       ------
convert_rows 4096 bool(0, 0.5)                                                                                                1.15     27.6±0.04µs        ? ?/sec    1.00     23.9±0.03µs        ? ?/sec
convert_rows 4096 bool(0.3, 0.5)                                                                                              1.15     27.7±0.04µs        ? ?/sec    1.00     24.0±0.29µs        ? ?/sec
convert_rows 4096 i64(0)                                                                                                      1.18     47.5±0.06µs        ? ?/sec    1.00     40.4±0.03µs        ? ?/sec
convert_rows 4096 i64(0.3)                                                                                                    1.18     47.7±0.18µs        ? ?/sec    1.00     40.6±0.46µs        ? ?/sec
convert_rows 4096 u64(0)                                                                                                      1.19     47.3±0.05µs        ? ?/sec    1.00     39.6±0.09µs        ? ?/sec
convert_rows 4096 u64(0.3)                                                                                                    1.19     47.3±0.06µs        ? ?/sec    1.00     39.7±0.13µs        ? ?/sec

Here is the script I used:

git checkout encode-skip-iter
cargo bench --all-features --bench row_format -- --save-baseline encode-skip-iter

git reset `git merge-base HEAD apache/master`
git restore arrow-row/src/lib.rs
git reset git restore arrow-row/src/fixed.rs
git status

cargo bench --all-features --bench row_format -- --save-baseline master

critcmp master encode-skip-iter

I am rerunning to be sure

eed to sort out performance benchmark results

[alamb]

I ran these numbers several times and they keep coming back slower. I may be doing something wrong

Results

Here are the results, which seem to clearly show this branch significantly slower than main:

++ critcmp master encode-skip-iter
group                                                                                                                         encode-skip-iter                       master
...
convert_rows 4096 bool(0, 0.5)                                                                                                1.16     27.6±0.03µs        ? ?/sec    1.00     23.8±0.05µs        ? ?/sec
convert_rows 4096 bool(0.3, 0.5)                                                                                              1.15     27.6±0.04µs        ? ?/sec    1.00     23.9±0.13µs        ? ?/sec
convert_rows 4096 i64(0)                                                                                                      1.18     47.6±0.06µs        ? ?/sec    1.00     40.3±0.05µs        ? ?/sec
convert_rows 4096 i64(0.3)                                                                                                    1.20     48.6±6.47µs        ? ?/sec    1.00     40.5±0.07µs        ? ?/sec
convert_rows 4096 string(10, 0)                                                                                               1.00     88.4±0.31µs        ? ?/sec    1.03     91.2±0.70µs        ? ?/sec
convert_rows 4096 string(100, 0)                                                                                              1.04    149.7±1.06µs        ? ?/sec    1.00    144.4±0.79µs        ? ?/sec
convert_rows 4096 string(100, 0.5)                                                                                            1.03   139.1±13.66µs        ? ?/sec    1.00    135.3±0.42µs        ? ?/sec
convert_rows 4096 string(20, 0.5), string(30, 0), string(100, 0), i64(0)                                                      1.00    384.9±3.21µs        ? ?/sec    1.01    387.0±3.45µs        ? ?/sec
convert_rows 4096 string(30, 0)                                                                                               1.00    102.0±0.19µs        ? ?/sec    1.01    103.0±0.21µs        ? ?/sec
convert_rows 4096 string_dictionary(10, 0)                                                                                    1.00    90.5±11.50µs        ? ?/sec    1.01     91.3±0.32µs        ? ?/sec
convert_rows 4096 string_dictionary(100, 0)                                                                                   1.03    150.5±1.14µs        ? ?/sec    1.00    145.5±0.69µs        ? ?/sec
convert_rows 4096 string_dictionary(100, 0.5)                                                                                 1.01    137.1±0.29µs        ? ?/sec    1.00    135.1±0.74µs        ? ?/sec
convert_rows 4096 string_dictionary(30, 0)                                                                                    1.00    100.6±0.21µs        ? ?/sec    1.02    103.0±0.39µs        ? ?/sec
convert_rows 4096 string_dictionary_low_cardinality(10, 0)                                                                    1.00     87.9±0.35µs        ? ?/sec    1.04     91.3±0.21µs        ? ?/sec
convert_rows 4096 string_dictionary_low_cardinality(100, 0)                                                                   1.03    149.0±0.68µs        ? ?/sec    1.00    144.4±0.52µs        ? ?/sec
convert_rows 4096 string_dictionary_low_cardinality(30, 0)                                                                    1.00    102.5±0.23µs        ? ?/sec    1.01    103.2±0.22µs        ? ?/sec
convert_rows 4096 u64(0)                                                                                                      1.19     47.3±0.17µs        ? ?/sec    1.00     39.6±0.07µs        ? ?/sec
convert_rows 4096 u64(0.3)                                                                                                    1.19     47.4±0.14µs        ? ?/sec    1.00     39.7±0.07µs        ? ?/sec

Test script

Here is my script

alamb@aal-dev:~/arrow-rs$ cat ./compare_encode_iter.sh
set -x -e

git fetch apache -p
git fetch korowa -p
git checkout encode-skip-iter
git reset --hard korowa/encode-skip-iter
cargo bench --all-features --bench row_format -- --save-baseline encode-skip-iter


git reset `git merge-base HEAD apache/master`
git restore arrow-row/src/lib.rs
git restore arrow-row/src/fixed.rs
git status

cargo bench --all-features --bench row_format -- --save-baseline master

critcmp master encode-skip-iter

Test Hardware

BTW I am running these on a c2-standard-8 VM from google cloud platform

cat /proc/cpuinfo 
...
processor       : 7
vendor_id       : GenuineIntel
cpu family      : 6
model           : 85
model name      : Intel(R) Xeon(R) CPU @ 3.10GHz
stepping        : 7
microcode       : 0xffffffff
cpu MHz         : 3100.310
cache size      : 25344 KB
physical id     : 0
siblings        : 8
core id         : 3
cpu cores       : 4
apicid          : 7
initial apicid  : 7
fpu             : yes
fpu_exception   : yes
cpuid level     : 13
wp              : yes
flags           : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology nonstop_tsc cpuid tsc_known_freq pni pclmulqdq ssse3 fma cx16 pcid sse\
4_1 sse4_2 x2apic movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single ssbd ibrs ibpb stibp ibrs_enhanced fsgsbase tsc_adjust bmi1 hle avx2 smep bmi2 erms invpcid rtm mpx avx512f avx512dq rdseed adx smap clflushopt clwb \
avx512cd avx512bw avx512vl xsaveopt xsavec xgetbv1 xsaves arat avx512_vnni md_clear arch_capabilities
bugs            : apic_c1e spectre_v1 spectre_v2 spec_store_bypass mds swapgs taa mmio_stale_data retbleed eibrs_pbrsb bhi
bogomips        : 6200.62
clflush size    : 64
cache_alignment : 64
address sizes   : 46 bits physical, 48 bits virtual
power management:

Benchmarks show regression

[korowa]

I ran these numbers several times and they keep coming back slower. I may be doing something wrong

@alamb, the script looks correct (it does what's expected), but still I'm not able to reproduce 20% diff for decoding.

I've temporarily (not for merging) added workflow to run row-format bench on github runner and check the results.

UPD: run results show that there are no serious regressions in convert rows benchmarks (I suppose that up to 4% timings in difference can be explained by GH runner usage / fluctuations).

To make running benchmarks easier, perhaps we could pull them out into their own PR

True, I guess it was not the greatest idea, which produced additional complexity, I'll add them separately and rebase this branch to check all (including new ones) benchmark results on this temp workflow.

[korowa]

Running the full set of benchmarks also doesn't show any regressions (except convert_columns 4096 string_dictionary_low_cardinality(30, 0) , which is fine in previous run + convert_columns_prepared 4096 string_dictionary_low_cardinality(30, 0) and other string dictionaries with low cardinality didn't show slowdown)

[alamb]

From my perspective this PR is ready to go once we remove the temporary CI job

Thank you for all the work @korowa

[alamb]

🚀

Thanks again @korowa