gh-153563 feat: add unicode_memeq() to compare either directly or SSE2 vector#153564
gh-153563 feat: add unicode_memeq() to compare either directly or SSE2 vector#153564rajat315315 wants to merge 2 commits into
Conversation
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
There was a problem hiding this comment.
I don't think that benchmarks are correct.
The issue states that it speeds things up for strings up to 32 chars.
But:
- 4 letter strings have a regression
- Only some platforms like
x86_64are supported, other platforms can also have performance regressions - Longer strings are not measured at all (32 chars and up)
- You only measure strings that have even length with
2 * nsize, odd strings are not measured at all. What happens there?
| * unicode_eq() is called when the hash of two unicode objects is equal. | ||
| */ | ||
| #if defined(__x86_64__) || defined(_M_X64) || defined(__i386__) || defined(_M_IX86) | ||
| # include <emmintrin.h> |
There was a problem hiding this comment.
Maybe we can instead check the support in configure?
| case 4: | ||
| return *(const uint32_t *)u1 == *(const uint32_t *)u2; | ||
| case 5: | ||
| case 6: |
There was a problem hiding this comment.
Consider adding /* fallthrough */ comment?
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
I will close the PR, let's please start with the benchmarks and design in the issue first. If you want to contribute: please, start with the discussion, prove that the optimization is worth it first, propose a design that will look correct and maintainable, then go to the implementation. Otherwise, we would have to take other actions to stop wasting maintainers time. Let's not do this. |
|
I agree @sobolevn .I am ready to contribute to healthy discussions before coming up with a solution. |
Linked Issue
Fixes #153563
Proposed Changes
Objects/stringlib/eq.hto implement a static inline memory comparisonunicode_memeqthat optimizes checks for string lengths up to 32 bytes.memcmpfunction call insideunicode_eqfor these short string sizes.__x86_64__, etc.) to guarantee portability on ARM, WebAssembly, and older x86 machines.Verification & Testing
Rebuilt CPython on Linux x86_64 with GCC 15 and ran the standard regression test suite. All tests passed successfully:
Benchmarks (Mean Latency of String Equality Comparison)
Measured Python-level comparison times using different pointers (recreated string objects) with identical hashes to isolate the
unicode_eqpath:For keys of length 2, 8, and 12 characters (which represent the vast majority of Python identifiers), this optimization provides a 16% to 35% speedup at the Python runtime level, directly accelerating attribute lookups and dictionary throughput.
unicode_eqfor short string comparisons by bypassingmemcmpoverhead #153563