Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vectorize basic_string::find #5101

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AlexGuteniev
Copy link
Contributor

@AlexGuteniev AlexGuteniev commented Nov 19, 2024

Resolves #5036

⚠️ Need to decide whether this CRT replacing vectorization is needed ⚠️

🏁 Race against the runtime!

basic_string::find is already vectorized for 8 and 16 bit characters via calling memchr and wmemchr

memchr shows good results for smaller positions, but if the distance from the beginning to the result is long the memchr results are noticeably worse. There's strong indication that such a results are due to not using AVX in memchr.

wmemchr is not fast at all. It was promised to be fast in a new CRT in #4873 though.

For larger characters the vectorization is novel, there's no CRT implementation, but these characters are rare anyway.

⏱️ Benchmark results

Benchmark main this
bm<char, not_highly_aligned_allocator, Op::StringFind>/8021/3056 89.6 ns 49.3 ns
bm<char, not_highly_aligned_allocator, Op::StringFind>/63/62 11.3 ns 3.70 ns
bm<char, not_highly_aligned_allocator, Op::StringFind>/31/30 9.90 ns 8.70 ns
bm<char, not_highly_aligned_allocator, Op::StringFind>/15/14 4.94 ns 7.10 ns
bm<char, not_highly_aligned_allocator, Op::StringFind>/7/6 2.83 ns 3.49 ns
bm<char, highly_aligned_allocator, Op::StringFind>/8021/3056 78.0 ns 48.0 ns
bm<char, highly_aligned_allocator, Op::StringFind>/63/62 12.4 ns 4.11 ns
bm<char, highly_aligned_allocator, Op::StringFind>/31/30 10.2 ns 7.85 ns
bm<char, highly_aligned_allocator, Op::StringFind>/15/14 5.18 ns 7.42 ns
bm<char, highly_aligned_allocator, Op::StringFind>/7/6 2.76 ns 3.42 ns
bm<wchar_t, not_highly_aligned_allocator, Op::StringFind>/8021/3056 754 ns 85.3 ns
bm<wchar_t, not_highly_aligned_allocator, Op::StringFind>/63/62 31.1 ns 3.84 ns
bm<wchar_t, not_highly_aligned_allocator, Op::StringFind>/31/30 16.2 ns 2.92 ns
bm<wchar_t, not_highly_aligned_allocator, Op::StringFind>/15/14 4.65 ns 3.66 ns
bm<wchar_t, not_highly_aligned_allocator, Op::StringFind>/7/6 2.45 ns 2.93 ns
bm<char32_t, not_highly_aligned_allocator, Op::StringFind>/8021/3056 749 ns 159 ns
bm<char32_t, not_highly_aligned_allocator, Op::StringFind>/63/62 18.5 ns 5.25 ns
bm<char32_t, not_highly_aligned_allocator, Op::StringFind>/31/30 9.60 ns 3.55 ns
bm<char32_t, not_highly_aligned_allocator, Op::StringFind>/15/14 6.42 ns 2.91 ns
bm<char32_t, not_highly_aligned_allocator, Op::StringFind>/7/6 2.80 ns 2.95 ns

@AlexGuteniev AlexGuteniev requested a review from a team as a code owner November 19, 2024 12:54
@CaseyCarter CaseyCarter added the performance Must go faster label Nov 19, 2024
@StephanTLavavej StephanTLavavej self-assigned this Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster
Projects
Status: Initial Review
Development

Successfully merging this pull request may close these issues.

Use vector algorithms in basic_string::find and basic_string::rfind
3 participants