You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "Mryange (via GitHub)" <gi...@apache.org> on 2026/05/28 04:09:51 UTC
[PR] [opt](function) optimize repeat and trim string functions [doris]
Mryange opened a new pull request, #63784:
URL: https://github.com/apache/doris/pull/63784
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
This PR optimizes two hot BE string function families that spend significant time on repeated buffer growth and high-overhead character lookup.
1. `repeat`
- Replace the old per-row append loop with a shared two-pass execution path for both vector repeat counts and constant repeat counts.
- Precompute `res_offsets` and total output size before writing results.
- Write directly into `ColumnString::Chars` with `StringOP::fast_repeat()`.
2. `trim_in` / `ltrim_in` / `rtrim_in`
- Reuse SIMD-assisted ASCII symbol search for small trim sets.
- Add reverse symbol search helpers for right trim.
- Use a fixed-size UTF-8 small-set lookup path for common cases while preserving fallback paths for larger trim strings.
3. Coverage and observability
- Add a larger `repeat('a', 256)` correctness case.
- Add `find_symbols` unit coverage for empty ranges, embedded NUL bytes, boundary lengths, and cross-check cases.
- Add old/new microbenchmarks for both repeat and trim paths.
### Benchmark results
Local Release benchmark results on this machine:
#### repeat
| Case | Old | New | Speedup |
| --- | ---: | ---: | ---: |
| RepeatVector 4096/16/8 | 86184 ns | 42878 ns | 2.01x |
| RepeatVector 4096/16/64 | 411330 ns | 146498 ns | 2.81x |
| RepeatVector 1024/128/16 | 84985 ns | 39620 ns | 2.15x |
| RepeatVector 4096/0/64 | 222362 ns | 7382 ns | 30.12x |
| RepeatConst 4096/16/8 | 127848 ns | 59495 ns | 2.15x |
| RepeatConst 4096/16/64 | 764912 ns | 205294 ns | 3.73x |
| RepeatConst 1024/128/16 | 158499 ns | 78090 ns | 2.03x |
| RepeatConst 4096/0/64 | 395137 ns | 7107 ns | 55.60x |
#### trim
| Case | Old | New | Speedup |
| --- | ---: | ---: | ---: |
| ASCII, 4 trim chars, 65536 rows | 2398.526 us | 997.839 us | 2.40x |
| ASCII, 8 trim chars, 65536 rows | 2452.269 us | 872.865 us | 2.81x |
| ASCII, no match, 65536 rows | 318.894 us | 309.079 us | 1.03x |
| UTF-8, 2 trim chars, 65536 rows | 7832.364 us | 6799.569 us | 1.15x |
Note:
The medium and large trim cases reproduced the expected improvement direction on this machine. One small-input case (`BM_TrimInAscii/1024`) was slightly slower at 0.81x, while the 4096-row and 65536-row cases remained clearly faster.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](function) optimize repeat and trim string functions [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #63784:
URL: https://github.com/apache/doris/pull/63784#issuecomment-4560781564
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](function) optimize repeat and trim string functions [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63784:
URL: https://github.com/apache/doris/pull/63784#issuecomment-4560764897
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR).
Please clearly describe your PR:
1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](function) optimize repeat and trim string functions [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #63784:
URL: https://github.com/apache/doris/pull/63784#discussion_r3315327145
##########
be/src/exprs/function/url/find_symbols.h:
##########
@@ -330,7 +330,68 @@ inline const char* find_first_symbols_sse42(const char* const begin, const char*
return return_mode == ReturnMode::End ? end : nullptr;
}
-/// NOTE No SSE 4.2 implementation for find_last_symbols_or_null. Not worth to do.
+template <bool positive, ReturnMode return_mode>
+inline const char* find_last_symbols_sse2(const char* const begin, const char* const end,
+ const char* symbols, size_t num_chars) {
+ if (begin >= end) return return_mode == ReturnMode::End ? end : nullptr;
+
+ const char* pos = end;
+
+#if defined(__SSE2__)
+ const auto needles = mm_is_in_prepare(symbols, num_chars);
+ for (; pos - 16 >= begin; pos -= 16) {
+ __m128i bytes = _mm_loadu_si128(reinterpret_cast<const __m128i*>(pos - 16));
+
+ __m128i eq = mm_is_in_execute(bytes, needles, num_chars);
+
+ uint16_t bit_mask = maybe_negate<positive>(uint16_t(_mm_movemask_epi8(eq)));
+ if (bit_mask) return pos - 1 - (__builtin_clz(bit_mask) - 16);
+ }
+#endif
+
+ --pos;
+ for (; pos >= begin; --pos)
+ if (maybe_negate<positive>(is_in(*pos, symbols, num_chars))) return pos;
Review Comment:
The scalar tail has undefined behavior when every remaining byte matches the trim set. For example, `find_last_not_symbols_or_null(s.data(), s.data() + 4, SearchSymbols(" "))` on four spaces reaches `pos == begin`, the loop body does not return, and the `--pos` in the loop expression decrements before the start of the array; the next `pos >= begin` comparison is also outside the valid range. This is the runtime SSE2 path used by the new ASCII rtrim optimization. Please use the same `for (const char* p = pos; p != begin;) { --p; ... }` pattern used in the SSE4.2 tail or otherwise avoid forming `begin - 1`.
##########
be/src/exprs/function/function_string_concat.h:
##########
@@ -608,57 +607,70 @@ class FunctionStringRepeat : public IFunction {
argument_ptr[0]->get_name(), argument_ptr[1]->get_name());
}
- Status vector_vector(const ColumnString::Chars& data, const ColumnString::Offsets& offsets,
- const ColumnInt32::Container& repeats, ColumnString::Chars& res_data,
- ColumnString::Offsets& res_offsets,
+ Status vector_vector(const ColumnString& source_column, const ColumnInt32::Container& repeats,
+ ColumnString::Chars& res_data, ColumnString::Offsets& res_offsets,
ColumnUInt8::Container& null_map) const {
- size_t input_row_size = offsets.size();
-
- fmt::memory_buffer buffer;
- res_offsets.resize(input_row_size);
- null_map.resize_fill(input_row_size, 0);
- for (ssize_t i = 0; i < input_row_size; ++i) {
- buffer.clear();
- const char* raw_str = reinterpret_cast<const char*>(&data[offsets[i - 1]]);
- size_t size = offsets[i] - offsets[i - 1];
- int repeat = repeats[i];
- if (repeat <= 0) {
- StringOP::push_empty_string(i, res_data, res_offsets);
- } else {
- ColumnString::check_chars_length(repeat * size + res_data.size(), 0);
- for (int j = 0; j < repeat; ++j) {
- buffer.append(raw_str, raw_str + size);
- }
- StringOP::push_value_string(std::string_view(buffer.data(), buffer.size()), i,
- res_data, res_offsets);
- }
- }
- return Status::OK();
+ return _repeat_execute(
+ source_column, [&repeats](size_t index) { return repeats[index]; }, res_data,
+ res_offsets, null_map);
}
// TODO: 1. use pmr::vector<char> replace fmt_buffer may speed up the code
// 2. abstract the `vector_vector` and `vector_const`
// 3. rethink we should use `DEFAULT_MAX_STRING_SIZE` to bigger here
- void vector_const(const ColumnString::Chars& data, const ColumnString::Offsets& offsets,
- int repeat, ColumnString::Chars& res_data, ColumnString::Offsets& res_offsets,
- ColumnUInt8::Container& null_map) const {
- size_t input_row_size = offsets.size();
+ Status vector_const(const ColumnString& source_column, int repeat,
+ ColumnString::Chars& res_data, ColumnString::Offsets& res_offsets,
+ ColumnUInt8::Container& null_map) const {
+ return _repeat_execute(
+ source_column, [repeat](size_t) { return repeat; }, res_data, res_offsets,
+ null_map);
+ }
+
+private:
+ template <typename RepeatGetter>
+ Status _repeat_execute(const ColumnString& source_column, RepeatGetter&& repeat_getter,
+ ColumnString::Chars& res_data, ColumnString::Offsets& res_offsets,
+ ColumnUInt8::Container& null_map) const {
+ const auto input_row_size = source_column.size();
- fmt::memory_buffer buffer;
res_offsets.resize(input_row_size);
null_map.resize_fill(input_row_size, 0);
- for (ssize_t i = 0; i < input_row_size; ++i) {
- buffer.clear();
- const char* raw_str = reinterpret_cast<const char*>(&data[offsets[i - 1]]);
- size_t size = offsets[i] - offsets[i - 1];
- ColumnString::check_chars_length(repeat * size + res_data.size(), 0);
- for (int j = 0; j < repeat; ++j) {
- buffer.append(raw_str, raw_str + size);
+ uint32_t total_size = 0;
+ for (size_t i = 0; i < input_row_size; ++i) {
+ const int repeat = repeat_getter(i);
+ if (repeat <= 0) {
+ res_offsets[i] = total_size;
+ continue;
}
- StringOP::push_value_string(std::string_view(buffer.data(), buffer.size()), i, res_data,
- res_offsets);
+
+ const auto str_ref = source_column.get_data_at(i);
+ const uint32_t size = str_ref.size;
+ total_size += size * repeat;
+ res_offsets[i] = total_size;
Review Comment:
This pre-size pass can overflow before `ColumnString::check_chars_length()` sees the real output size. Both `total_size` and `size` are `uint32_t`, and `size * repeat` is evaluated in 32-bit unsigned arithmetic, so a large but valid input such as one row with `size=65536` and `repeat=65536` wraps to zero. The later `res_data.resize(total_size)` then allocates too little and the second pass writes the full repeated payload through `fast_repeat`, causing memory corruption instead of raising `STRING_OVERFLOW_IN_VEC_ENGINE`. Please compute in `size_t`/checked arithmetic and call `check_chars_length()` before narrowing into `ColumnString::Offset`.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](function) optimize repeat and trim string functions [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #63784:
URL: https://github.com/apache/doris/pull/63784#issuecomment-4560848974
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](function) optimize repeat and trim string functions [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #63784:
URL: https://github.com/apache/doris/pull/63784#discussion_r3315370093
##########
be/src/exprs/function/url/find_symbols.h:
##########
@@ -330,7 +330,69 @@ inline const char* find_first_symbols_sse42(const char* const begin, const char*
return return_mode == ReturnMode::End ? end : nullptr;
}
-/// NOTE No SSE 4.2 implementation for find_last_symbols_or_null. Not worth to do.
+template <bool positive, ReturnMode return_mode>
+inline const char* find_last_symbols_sse2(const char* const begin, const char* const end,
+ const char* symbols, size_t num_chars) {
+ if (begin >= end) return return_mode == ReturnMode::End ? end : nullptr;
+
+ const char* pos = end;
+
+#if defined(__SSE2__)
+ const auto needles = mm_is_in_prepare(symbols, num_chars);
+ for (; pos - 16 >= begin; pos -= 16) {
+ __m128i bytes = _mm_loadu_si128(reinterpret_cast<const __m128i*>(pos - 16));
Review Comment:
This loop forms `pos - 16` before proving that there are 16 bytes left in the range. For any runtime `SearchSymbols` call on a string shorter than 16 bytes (for example `trim_in('abc', ' ')` through the new ASCII rtrim path), `pos` is `end` and `pos - 16` is outside the array before the comparison is evaluated, which is undefined behavior even if the loop body is skipped. This is distinct from the scalar-tail issue already raised: the UB happens in the SIMD loop condition before reaching the tail. Please use the same safe form as the SSE4.2 helper, e.g. `static_cast<size_t>(pos - begin) >= 16`, before computing `pos - 16`.
```suggestion
for (; static_cast<size_t>(pos - begin) >= 16; pos -= 16) {
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](function) optimize repeat and trim string functions [doris]
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange closed pull request #63784: [opt](function) optimize repeat and trim string functions
URL: https://github.com/apache/doris/pull/63784
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org