Faster utf8 validation

[Dandandan]

Which issue does this PR close?

Closes #6667

Rationale for this change

Improves performance for about 4-5% (on M1 Pro) on strings (plain encoding):

arrow_array_reader/StringArray/plain encoded, mandatory, no NULLs
                        time:   [740.81 µs 746.51 µs 752.11 µs]
                        change: [-5.8127% -5.2637% -4.6414%] (p = 0.00 < 0.05)
                        Performance has improved.
arrow_array_reader/StringArray/plain encoded, optional, no NULLs
                        time:   [743.62 µs 748.70 µs 754.14 µs]
                        change: [-4.2825% -3.6551% -3.0212%] (p = 0.00 < 0.05)
                        Performance has improved.
arrow_array_reader/StringArray/plain encoded, optional, half NULLs
                        time:   [633.43 µs 638.47 µs 643.71 µs]
                        change: [-5.1930% -4.5414% -3.8189%] (p = 0.00 < 0.05)

What changes are included in this PR?

Are there any user-facing changes?

[Dandandan]

We call compat from_utf8 again to get the same error.

[findepi]

the role of simdutf8::basic::from_utf8 and re-run with simdutf8::compat -- does this deserve a code comment?

(at least the .unwrap_err() safety deserves one)

[findepi]

same in offset_buffer.rs

[Dandandan]

Yeah I agree deserves some comments explaining why we rerun it in case of error.

[Dandandan]

If there is a positive sentiment about using simdutf8 for faster validation, I can do so.

[Dandandan]

It could be optional as well.

[findepi]

How mature is the library and its dependencies?
My random spike led me to https://github.com/rusticstuff/simdutf8/blob/main/src/implementation/aarch64/neon.rs#L16 and https://docs.rs/flexpect/latest/flexpect/ lacks documentation.
Should we help simdutf8 to bring it to arrow's maturity level?

[Dandandan]

It seems it is just some macro helper for clippy split off as crate / dependency. Doesn't seem too bad.

[tustvold]

I'm not sure that 5% really justifies an additional dependency, especially one that uses so much unsafe...

[Dandandan]

I'm not sure that 5% really justifies an additional dependency, especially one that uses so much unsafe...

Hm yeah wondering about that.

I think that 5% speed up for Parquet might be quite valuable though, given that it often translates in close to 5% faster query execution for queries where Parquet scan is a bottleneck (quite some DF benchmarks actually involving string data).

[Dandandan]

FWIW some other projects are using simdutf8 as well, like polars https://github.com/pola-rs/polars/blob/main/Cargo.toml#L77 and simd-json

[alamb]

I am not sure exactly the usecase here, but what about simply disabling utf8 validation for known good data?

• Proposal: Add unsafe option to disable UTF8 validation on parquet read #6701

[Dandandan]

I am not sure exactly the usecase here, but what about simply disabling utf8 validation for known good data?

• Proposal: Add unsafe option to disable UTF8 validation on parquet read #6701

The "use case" of this PR is just that utf8 validation takes time, this PR improves the performance.

I think having a option to disable it makes sense, but would be good to minimize the cost of validation as well.

[alamb]

So what shall we do with this PR? Make it an optional opt-in feature of the parquet crate that people can enable if they want more performance?

[doki23]

I believe this PR is solely for performance enhancement. Introducing an optional opt-in feature deserves a separate PR.