You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Dandandan (via GitHub)" <gi...@apache.org> on 2024/10/31 19:22:55 UTC
[PR] Faster utf8 validation [arrow-rs]
Dandandan opened a new pull request, #6668:
URL: https://github.com/apache/arrow-rs/pull/6668
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes https://github.com/apache/arrow-rs/issues/6667
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2450763032
> 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).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster parquet utf8 validation using `simdjson` [arrow-rs]
Posted by "XiangpengHao (via GitHub)" <gi...@apache.org>.
XiangpengHao commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2568007955
This is my benchmark results with Clickbench non-partitioned and filter pushdown enabled. Benchmarked on x86 AMD 9900x. Scan-dominate queries can get 20% improvements.
```
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━┳━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query ┃ no-simd-utf8 ┃ simd-utf8 ┃ Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━╇━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0 │ 0.35ms │ 0.36ms │ no change │
│ QQuery 1 │ 50.50ms │ 50.15ms │ no change │
│ QQuery 2 │ 68.05ms │ 67.37ms │ no change │
│ QQuery 3 │ 89.46ms │ 88.81ms │ no change │
│ QQuery 4 │ 463.69ms │ 459.30ms │ no change │
│ QQuery 5 │ 527.10ms │ 481.84ms │ +1.09x faster │
│ QQuery 6 │ 50.25ms │ 50.92ms │ no change │
│ QQuery 7 │ 55.59ms │ 55.34ms │ no change │
│ QQuery 8 │ 562.75ms │ 561.54ms │ no change │
│ QQuery 9 │ 580.23ms │ 580.26ms │ no change │
│ QQuery 10 │ 159.95ms │ 155.25ms │ no change │
│ QQuery 11 │ 172.33ms │ 168.08ms │ no change │
│ QQuery 12 │ 730.38ms │ 653.11ms │ +1.12x faster │
│ QQuery 13 │ 1205.87ms │ 1100.43ms │ +1.10x faster │
│ QQuery 14 │ 739.05ms │ 666.65ms │ +1.11x faster │
│ QQuery 15 │ 550.73ms │ 552.73ms │ no change │
│ QQuery 16 │ 1188.05ms │ 1159.64ms │ no change │
│ QQuery 17 │ 1159.66ms │ 1146.29ms │ no change │
│ QQuery 18 │ 2641.59ms │ 2652.32ms │ no change │
│ QQuery 19 │ 81.54ms │ 90.09ms │ 1.10x slower │
│ QQuery 20 │ 610.90ms │ 589.75ms │ no change │
│ QQuery 21 │ 705.48ms │ 663.11ms │ +1.06x faster │
│ QQuery 22 │ 1659.48ms │ 1357.19ms │ +1.22x faster │
│ QQuery 23 │ 3534.78ms │ 3639.67ms │ no change │
│ QQuery 24 │ 533.81ms │ 475.18ms │ +1.12x faster │
│ QQuery 25 │ 470.34ms │ 373.89ms │ +1.26x faster │
│ QQuery 26 │ 576.26ms │ 476.26ms │ +1.21x faster │
│ QQuery 27 │ 1121.43ms │ 1056.52ms │ +1.06x faster │
│ QQuery 28 │ 4291.35ms │ 4288.26ms │ no change │
│ QQuery 29 │ 228.93ms │ 235.60ms │ no change │
│ QQuery 30 │ 589.98ms │ 541.70ms │ +1.09x faster │
│ QQuery 31 │ 716.59ms │ 702.51ms │ no change │
│ QQuery 32 │ 2593.06ms │ 2528.18ms │ no change │
│ QQuery 33 │ 2362.52ms │ 2336.75ms │ no change │
│ QQuery 34 │ 2360.44ms │ 2334.17ms │ no change │
│ QQuery 35 │ 705.78ms │ 696.32ms │ no change │
│ QQuery 36 │ 163.69ms │ 156.20ms │ no change │
│ QQuery 37 │ 129.14ms │ 97.25ms │ +1.33x faster │
│ QQuery 38 │ 88.72ms │ 90.76ms │ no change │
│ QQuery 39 │ 218.61ms │ 213.61ms │ no change │
│ QQuery 40 │ 71.73ms │ 73.40ms │ no change │
│ QQuery 41 │ 68.23ms │ 67.56ms │ no change │
│ QQuery 42 │ 69.22ms │ 68.76ms │ no change │
└──────────────┴──────────────┴───────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary ┃ ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (no-simd-utf8) │ 34947.62ms │
│ Total Time (simd-utf8) │ 33803.08ms │
│ Average Time (no-simd-utf8) │ 812.74ms │
│ Average Time (simd-utf8) │ 786.12ms │
│ Queries Faster │ 12 │
│ Queries Slower │ 1 │
│ Queries with No Change │ 30 │
└─────────────────────────────┴────────────┘
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster parquet utf8 validation using `simdjson` [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#discussion_r1907995431
##########
parquet/src/arrow/array_reader/byte_view_array.rs:
##########
@@ -679,9 +679,12 @@ impl ByteViewArrayDecoderDelta {
/// Check that `val` is a valid UTF-8 sequence
pub fn check_valid_utf8(val: &[u8]) -> Result<()> {
- match std::str::from_utf8(val) {
+ match simdutf8::basic::from_utf8(val) {
Ok(_) => Ok(()),
- Err(e) => Err(general_err!("encountered non UTF-8 data: {}", e)),
+ Err(_) => {
+ let e = simdutf8::compat::from_utf8(val).unwrap_err();
Review Comment:
Could we have our own `from_utf8` that wraps the `simdutf8` implementation? Then the weird basic/compat path would only need to be documented once (and make it easier to replace other `from_utf8` invocations that @alamb identified).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "findepi (via GitHub)" <gi...@apache.org>.
findepi commented on code in PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#discussion_r1826843346
##########
parquet/src/arrow/array_reader/byte_view_array.rs:
##########
@@ -679,9 +679,12 @@ impl ByteViewArrayDecoderDelta {
/// Check that `val` is a valid UTF-8 sequence
pub fn check_valid_utf8(val: &[u8]) -> Result<()> {
- match std::str::from_utf8(val) {
+ match simdutf8::basic::from_utf8(val) {
Ok(_) => Ok(()),
- Err(e) => Err(general_err!("encountered non UTF-8 data: {}", e)),
+ Err(_) => {
+ let e = simdutf8::compat::from_utf8(val).unwrap_err();
Review Comment:
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)
##########
parquet/src/arrow/array_reader/byte_view_array.rs:
##########
@@ -679,9 +679,12 @@ impl ByteViewArrayDecoderDelta {
/// Check that `val` is a valid UTF-8 sequence
pub fn check_valid_utf8(val: &[u8]) -> Result<()> {
- match std::str::from_utf8(val) {
+ match simdutf8::basic::from_utf8(val) {
Ok(_) => Ok(()),
- Err(e) => Err(general_err!("encountered non UTF-8 data: {}", e)),
+ Err(_) => {
+ let e = simdutf8::compat::from_utf8(val).unwrap_err();
Review Comment:
same in offset_buffer.rs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on code in PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#discussion_r1826852837
##########
parquet/src/arrow/array_reader/byte_view_array.rs:
##########
@@ -679,9 +679,12 @@ impl ByteViewArrayDecoderDelta {
/// Check that `val` is a valid UTF-8 sequence
pub fn check_valid_utf8(val: &[u8]) -> Result<()> {
- match std::str::from_utf8(val) {
+ match simdutf8::basic::from_utf8(val) {
Ok(_) => Ok(()),
- Err(e) => Err(general_err!("encountered non UTF-8 data: {}", e)),
+ Err(_) => {
+ let e = simdutf8::compat::from_utf8(val).unwrap_err();
Review Comment:
Yeah I agree deserves some comments explaining why we rerun it in case of error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2450740636
I'm not sure that 5% really justifies an additional dependency, especially one that uses so much unsafe...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2567090473
What I suggest we do with this PR is get some end to end performance numbers (aka run the DataFusion clickbench benchmark with a pinned arrow version with this change)
Assuming that looks promising I recommend creating a PR that has an optional feature (enabled by default) for using simdjson for utf8 validation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2463388912
I am not sure exactly the usecase here, but what about simply disabling utf8 validation for known good data?
- https://github.com/apache/arrow-rs/issues/6701
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2463791729
> 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](https://github.com/apache/arrow-rs/issues/6701)
The "use case" of this PR is just that utf8 validation takes time, this PR improves the performance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2451775521
FWIW some other projects are using `simdutf8` as well, like polars https://github.com/pola-rs/polars/blob/main/Cargo.toml#L77
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster parquet utf8 validation using `simdjson` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2590070274
Let's keep chipping away at performance 🚀 -- thank you all for your thoughts and help here
Here is the follow on PR to add validation as a feature flag:
- https://github.com/apache/arrow-rs/pull/6979
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "doki23 (via GitHub)" <gi...@apache.org>.
doki23 commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2560586578
I believe this PR is solely for performance enhancement. Introducing an optional opt-in feature deserves a separate PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster parquet utf8 validation using `simdutf8` [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2592204639
Totally missed the updates on this PR.
Thanks for validating the perf difference @XiangpengHao and for moving the PR forward @alamb
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster parquet utf8 validation using `simdjson` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#discussion_r1908033297
##########
parquet/src/arrow/array_reader/byte_view_array.rs:
##########
@@ -679,9 +679,12 @@ impl ByteViewArrayDecoderDelta {
/// Check that `val` is a valid UTF-8 sequence
pub fn check_valid_utf8(val: &[u8]) -> Result<()> {
- match std::str::from_utf8(val) {
+ match simdutf8::basic::from_utf8(val) {
Ok(_) => Ok(()),
- Err(e) => Err(general_err!("encountered non UTF-8 data: {}", e)),
+ Err(_) => {
+ let e = simdutf8::compat::from_utf8(val).unwrap_err();
Review Comment:
I'll make a PR that does this in the next few days if no one beats me to it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on code in PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#discussion_r1826853446
##########
parquet/Cargo.toml:
##########
@@ -69,6 +69,7 @@ paste = { version = "1.0" }
half = { version = "2.1", default-features = false, features = ["num-traits"] }
sysinfo = { version = "0.32.0", optional = true, default-features = false, features = ["system"] }
crc32fast = { version = "1.4.2", optional = true, default-features = false }
+simdutf8 = { version = "0.1.5"}
Review Comment:
It seems it is just some macro helper for clippy split off as crate / dependency. Doesn't seem too bad.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2549496341
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster parquet utf8 validation using `simdjson` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#discussion_r1914851263
##########
parquet/Cargo.toml:
##########
@@ -69,6 +69,7 @@ paste = { version = "1.0" }
half = { version = "2.1", default-features = false, features = ["num-traits"] }
sysinfo = { version = "0.32.0", optional = true, default-features = false, features = ["system"] }
crc32fast = { version = "1.4.2", optional = true, default-features = false }
+simdutf8 = { version = "0.1.5"}
Review Comment:
Update: given the number of downloads (28M) and use by widely popular libraries (e.g polars) in mt opinion the `simdutf8` crate is sufficiently mature for inclusion. More info in this [comment](https://github.com/apache/arrow-rs/pull/6668#pullrequestreview-2538452333)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "XiangpengHao (via GitHub)" <gi...@apache.org>.
XiangpengHao commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2567068505
> Improves performance for about 4-5% (on M1 Pro) on strings (plain encoding)
Coming from https://github.com/apache/arrow-rs/pull/6921#discussion_r1900411077, I have seen much larger performance (~15%) improvements using simdutf8 with StringViewArray + x86, especially when strings are long (>128 byte).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on code in PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#discussion_r1826854660
##########
parquet/src/arrow/array_reader/byte_view_array.rs:
##########
@@ -679,9 +679,12 @@ impl ByteViewArrayDecoderDelta {
/// Check that `val` is a valid UTF-8 sequence
pub fn check_valid_utf8(val: &[u8]) -> Result<()> {
- match std::str::from_utf8(val) {
+ match simdutf8::basic::from_utf8(val) {
Ok(_) => Ok(()),
- Err(e) => Err(general_err!("encountered non UTF-8 data: {}", e)),
+ Err(_) => {
+ let e = simdutf8::compat::from_utf8(val).unwrap_err();
Review Comment:
If there is a positive sentiment about using `simdutf8` for faster validation, I can do so.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on code in PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#discussion_r1825072260
##########
parquet/src/arrow/array_reader/byte_view_array.rs:
##########
@@ -679,9 +679,12 @@ impl ByteViewArrayDecoderDelta {
/// Check that `val` is a valid UTF-8 sequence
pub fn check_valid_utf8(val: &[u8]) -> Result<()> {
- match std::str::from_utf8(val) {
+ match simdutf8::basic::from_utf8(val) {
Ok(_) => Ok(()),
- Err(e) => Err(general_err!("encountered non UTF-8 data: {}", e)),
+ Err(_) => {
+ let e = simdutf8::compat::from_utf8(val).unwrap_err();
Review Comment:
We call `from_utf8` to get the same error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on code in PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#discussion_r1825072260
##########
parquet/src/arrow/array_reader/byte_view_array.rs:
##########
@@ -679,9 +679,12 @@ impl ByteViewArrayDecoderDelta {
/// Check that `val` is a valid UTF-8 sequence
pub fn check_valid_utf8(val: &[u8]) -> Result<()> {
- match std::str::from_utf8(val) {
+ match simdutf8::basic::from_utf8(val) {
Ok(_) => Ok(()),
- Err(e) => Err(general_err!("encountered non UTF-8 data: {}", e)),
+ Err(_) => {
+ let e = simdutf8::compat::from_utf8(val).unwrap_err();
Review Comment:
We call compat `from_utf8` again to get the same error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster parquet utf8 validation using `simdjson` [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2578891295
> @etseidl I wonder if you have any thoughts on this PR?
None that haven't already been voiced. It seems like a fairly low risk (especially if made optional) way to get a significant speed up in string handling.
+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on code in PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#discussion_r1825072735
##########
parquet/Cargo.toml:
##########
@@ -69,6 +69,7 @@ paste = { version = "1.0" }
half = { version = "2.1", default-features = false, features = ["num-traits"] }
sysinfo = { version = "0.32.0", optional = true, default-features = false, features = ["system"] }
crc32fast = { version = "1.4.2", optional = true, default-features = false }
+simdutf8 = { version = "0.1.5"}
Review Comment:
It could be optional as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster utf8 validation [arrow-rs]
Posted by "findepi (via GitHub)" <gi...@apache.org>.
findepi commented on code in PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#discussion_r1826842634
##########
parquet/Cargo.toml:
##########
@@ -69,6 +69,7 @@ paste = { version = "1.0" }
half = { version = "2.1", default-features = false, features = ["num-traits"] }
sysinfo = { version = "0.32.0", optional = true, default-features = false, features = ["system"] }
crc32fast = { version = "1.4.2", optional = true, default-features = false }
+simdutf8 = { version = "0.1.5"}
Review Comment:
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster parquet utf8 validation using `simdjson` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2589945053
I merged this branch up from main and intend to merge it when the CI passes.
I will also make a follow on PR to make this feature optional.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster parquet utf8 validation using `simdjson` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faster parquet utf8 validation using `simdutf8` [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6668:
URL: https://github.com/apache/arrow-rs/pull/6668#issuecomment-2592283385
Thanks for bearing with us. This is going to be awesome
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org