Skip to content

gh-124502: Add PyUnicode_Equal() function - #124504

Merged
vstinner merged 6 commits into
python:mainfrom
vstinner:unicode_equal
Oct 7, 2024
Merged

gh-124502: Add PyUnicode_Equal() function#124504
vstinner merged 6 commits into
python:mainfrom
vstinner:unicode_equal

Conversation

@vstinner

@vstinnervstinner commented Sep 25, 2024

Copy link
Copy Markdown
Member

Comment threadDoc/c-api/unicode.rst Outdated
Comment threadDoc/whatsnew/3.14.rst Outdated
Comment threadLib/test/test_capi/test_unicode.py
Comment threadObjects/unicodeobject.c
Comment threadDoc/c-api/unicode.rst Outdated
@vstinner

Copy link
Copy Markdown
MemberAuthor

@picnixz@rruuaanng@ZeroIntensity: I addressed your comments. Please review the updated PR.

@picnixzpicnixz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final nitpick but you can ignore it (I don't know whether it's that useful).

Comment threadObjects/unicodeobject.c Outdated

@ZeroIntensityZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! LGTM.

Comment threadLib/test/test_capi/test_unicode.py
@rruuaanng

Copy link
Copy Markdown
Contributor

Maybe you are right, But you can consider my suggestion.

@vstinner

Copy link
Copy Markdown
MemberAuthor

I created capi-workgroup/decisions#43 issue in the C API Working Group.

@vstinner

vstinner commented Sep 26, 2024

Copy link
Copy Markdown
MemberAuthor

I wrote a microbenchmark comparing equal strings of 10 characters:

Results on Fedora 40 with CPU isolation:

  • (private) _PyUnicode_EQ: Mean +- std dev: 9.71 ns +- 0.04 ns
  • (private) _PyUnicode_Equal: Mean +- std dev: 8.75 ns +- 0.01 ns
  • (public) PyUnicode_Equal: Mean +- std dev: 9.14 ns +- 0.02 ns
  • (public) PyUnicode_Compare: Mean +- std dev: 11.1 ns +- 0.1 ns
  • (public) PyUnicode_RichCompare: Mean +- std dev: 12.2 ns +- 0.0 ns

Proposed public PyUnicode_Equal() is 1.2x faster than PyUnicode_Compare() (-2.0 ns) and 1.3x faster than PyUnicode_RichCompare() (-3.1 ns).

Private _PyUnicode_EQ() and private _PyUnicode_Equal() are not exactly the same implementation, it seems like private _PyUnicode_Equal() is a little bit faster (1 ns!).

@vstinner

vstinner commented Sep 30, 2024

Copy link
Copy Markdown
MemberAuthor

Microbenchmark on comparison of inequal strings of 10 characters, only the last character is different.

  • (private) _PyUnicode_EQ: Mean +- std dev: 10.0 ns +- 0.0 ns
  • (private) _PyUnicode_Equal: Mean +- std dev: 9.18 ns +- 0.00 ns
  • (public) PyUnicode_Equal: Mean +- std dev: 10.1 ns +- 0.0 ns
  • (public) PyUnicode_Compare: Mean +- std dev: 10.9 ns +- 0.0 ns
  • (public) PyUnicode_RichCompare: Mean +- std dev: 13.1 ns +- 0.0 ns

@vstinner
vstinner enabled auto-merge (squash) October 7, 2024 21:07
@vstinner

Copy link
Copy Markdown
MemberAuthor

I created capi-workgroup/decisions#43 issue in the C API Working Group.

The C API Working Group approved the API.

@vstinner
vstinner merged commit a7f0727 into python:mainOct 7, 2024
@vstinner
vstinner deleted the unicode_equal branch October 7, 2024 21:24
@vstinner

Copy link
Copy Markdown
MemberAuthor

Merged. Thanks for reviews @picnixz and @ZeroIntensity.

efimov-mikhail pushed a commit to efimov-mikhail/cpython that referenced this pull request Oct 9, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@vstinner@rruuaanng@picnixz@ZeroIntensity