[opt](function)Optimize the performance of the pad function under UTF-8. #40162
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
TPC-H: Total hot run time: 38528 ms |
TPC-DS: Total hot run time: 192837 ms |
ClickBench: Total hot run time: 32.31 s |
Mryange
changed the title
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
TPC-H: Total hot run time: 38413 ms |
TPC-DS: Total hot run time: 187491 ms |
ClickBench: Total hot run time: 31.94 s |
|
run buildall |
1 similar comment
|
run buildall |
TPC-H: Total hot run time: 38523 ms |
TPC-DS: Total hot run time: 189125 ms |
ClickBench: Total hot run time: 32.36 s |
HappenLee
reviewed
Sep 2, 2024
| res_chars, res_offsets); | ||
| auto [real_len, skip_chars] = simd::VStringFunctions::skip_leading_utf8( | ||
| (const char*)str_data, (const char*)str_data + str_len, len); | ||
| if (len <= skip_chars) { |
There was a problem hiding this comment.
seems wired. skip_chars seems always <= len
HappenLee
reviewed
Sep 2, 2024
be/src/util/simd/vstring_function.h
Outdated
| @@ -187,6 +187,19 @@ class VStringFunctions { | |||
| return p; | |||
| } | |||
|
|
|||
| static inline std::pair<size_t, size_t> skip_leading_utf8(const char* begin, const char* end, | |||
There was a problem hiding this comment.
Add comment and change the func name. we should know what the functinon do. if just one place call the func, better be a lambda
HappenLee
reviewed
Sep 2, 2024
be/src/util/simd/vstring_function.h
Outdated
| @@ -187,6 +187,19 @@ class VStringFunctions { | |||
| return p; | |||
| } | |||
|
|
|||
| static inline std::pair<size_t, size_t> skip_leading_utf8(const char* begin, const char* end, | |||
There was a problem hiding this comment.
Add comment and change the func name. we should know what the functinon do. if just one place call the func, better be a lambda
|
run buildall |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
|
|
||
| template <bool str_const, bool len_const, bool pad_const> | ||
| void execute_utf8(const ColumnString::Offsets& strcol_offsets, |
There was a problem hiding this comment.
warning: function 'execute_utf8' exceeds recommended size/complexity thresholds [readability-function-size]
void execute_utf8(const ColumnString::Offsets& strcol_offsets,
^Additional context
be/src/vec/functions/function_string.h:1574: 82 lines including whitespace and comments (threshold 80)
void execute_utf8(const ColumnString::Offsets& strcol_offsets,
^
TPC-H: Total hot run time: 37769 ms |
TPC-DS: Total hot run time: 193079 ms |
ClickBench: Total hot run time: 31.85 s |
|
run p0 |
HappenLee
reviewed
Sep 3, 2024
| simd::VStringFunctions::iterate_utf8_with_limit_length( | ||
| (const char*)str_data, (const char*)str_data + str_len, len); | ||
| // If iterate_char_len is greater than len, it indicates that the string's length exceeds len, so truncation. | ||
| if (iterate_char_len >= len) { |
There was a problem hiding this comment.
not greater， only equal need to do truncate
|
run buildall |
|
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 38095 ms |
TPC-DS: Total hot run time: 192290 ms |
ClickBench: Total hot run time: 32.44 s |
github-actions
bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
Gabriel39
approved these changes
Sep 6, 2024
dataroaring
pushed a commit
that referenced
this pull request
Oct 9, 2024
…-8. (#40162) ## Proposed changes 1. Removed the calculation of str_index. 2. If pad is constant, calculate pad_index only once. 3. Do not insert res_chars inside the loop; instead, insert them all together after the loop completes. ``` mysql [test]>select count(lpad(Title, 100, "abc")) from hits_10m; +--------------------------------+ | count(lpad(Title, 100, 'abc')) | +--------------------------------+ | 10000000 | +--------------------------------+ 1 row in set (3.97 sec) mysql [test]>select count(lpad(Title, 100, "abc")) from hits_10m; +--------------------------------+ | count(lpad(Title, 100, 'abc')) | +--------------------------------+ | 10000000 | +--------------------------------+ 1 row in set (2.87 sec) ``` <!--Describe your changes.-->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes