You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/10/15 13:36:11 UTC
[GitHub] [arrow] pitrou opened a new pull request #8470: ARROW-10313: [C++] Faster UTF8 validation for small strings
pitrou opened a new pull request #8470:
URL: https://github.com/apache/arrow/pull/8470
This improves CSV string conversion by about 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8470: ARROW-10313: [C++] Faster UTF8 validation for small strings
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8470:
URL: https://github.com/apache/arrow/pull/8470#issuecomment-709453602
Another possibility would be to compute and store ASCIIness of values while parsing CSV (ideally this should not cost anything CPU-wise). Then we can reuse that information to skip UTF8 validation for most values.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #8470: ARROW-10313: [C++] Faster UTF8 validation for small strings
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #8470:
URL: https://github.com/apache/arrow/pull/8470#discussion_r505776329
##########
File path: cpp/src/arrow/util/utf8.h
##########
@@ -98,8 +100,9 @@ inline bool ValidateUTF8(const uint8_t* data, int64_t size) {
// XXX This is doing an unaligned access. Contemporary architectures
// (x86-64, AArch64, PPC64) support it natively and often have good
// performance nevertheless.
- memcpy(&mask, data, 8);
- if (ARROW_PREDICT_TRUE((mask & high_bits_64) == 0)) {
+ uint64_t mask64;
+ memcpy(&mask64, data, 8);
Review comment:
```suggestion
uint64_t mask64 = SafeLoadAs<uint64_t>(data);
```
##########
File path: cpp/src/arrow/util/utf8.h
##########
@@ -154,13 +157,50 @@ inline bool ValidateUTF8(const uint8_t* data, int64_t size) {
return false;
}
- // Validate string tail one byte at a time
+ // Check if string tail is full ASCII (common case, fast)
+ if (size >= 4) {
+ uint32_t tail_mask = SafeLoadAs<uint32_t>(data + size - 4);
+ uint32_t head_mask = SafeLoadAs<uint32_t>(data);
+ if (ARROW_PREDICT_TRUE(((head_mask | tail_mask) & high_bits_32) == 0)) {
+ return true;
+ }
+ } else if (size >= 2) {
+ uint16_t tail_mask = SafeLoadAs<uint32_t>(data + size - 2);
+ uint16_t head_mask = SafeLoadAs<uint32_t>(data);
Review comment:
```suggestion
uint16_t tail_mask = SafeLoadAs<uint16_t>(data + size - 2);
uint16_t head_mask = SafeLoadAs<uint16_t>(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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou edited a comment on pull request #8470: ARROW-10313: [C++] Faster UTF8 validation for small strings
Posted by GitBox <gi...@apache.org>.
pitrou edited a comment on pull request #8470:
URL: https://github.com/apache/arrow/pull/8470#issuecomment-709453602
Another possibility would be to compute and store ASCIIness of values while parsing CSV (ideally this should not cost anything CPU-wise). Then we can reuse that information to skip UTF8 validation for most values.
Edit: actually, a quick attempt shows a significant decrease in CSV parsing speed. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8470: ARROW-10313: [C++] Faster UTF8 validation for small strings
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8470:
URL: https://github.com/apache/arrow/pull/8470#issuecomment-709336174
https://issues.apache.org/jira/browse/ARROW-10313
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou closed pull request #8470: ARROW-10313: [C++] Faster UTF8 validation for small strings
Posted by GitBox <gi...@apache.org>.
pitrou closed pull request #8470:
URL: https://github.com/apache/arrow/pull/8470
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #8470: ARROW-10313: [C++] Faster UTF8 validation for small strings
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #8470:
URL: https://github.com/apache/arrow/pull/8470#discussion_r505708835
##########
File path: cpp/src/arrow/util/utf8.h
##########
@@ -154,13 +156,52 @@ inline bool ValidateUTF8(const uint8_t* data, int64_t size) {
return false;
}
- // Validate string tail one byte at a time
+ // Check if string tail is full ASCII (common case, fast)
+ if (size >= 4) {
+ uint32_t mask1, mask2;
+ memcpy(&mask2, data + size - 4, 4);
+ memcpy(&mask1, data, 4);
+ if (ARROW_PREDICT_TRUE(((mask1 | mask2) & high_bits_32) == 0)) {
+ return true;
+ }
Review comment:
```suggestion
uint32_t head_mask = internal::SafeLoadAs<uint32_t>(data);
uint32_t tail_mask = internal::SafeLoadAs<uint32_t>(data + size - 4);
if (ARROW_PREDICT_TRUE(((head_mask | tail_mask) & high_bits_32) == 0)) {
return true;
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8470: ARROW-10313: [C++] Faster UTF8 validation for small strings
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8470:
URL: https://github.com/apache/arrow/pull/8470#issuecomment-709341899
This also improves the ARROW-10308 benchmark by about 9%.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org