Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-124502: Add PyUnicode_Equal() function#124504

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
vstinner merged 6 commits intopython:mainfromvstinner:unicode_equal
Oct 7, 2024

Conversation

@vstinner
Copy link
Member

@vstinnervstinner commentedSep 25, 2024
edited by github-actionsbot
Loading

edgarrmondragon and colesbury reacted with thumbs up emoji
@vstinner
Copy link
MemberAuthor

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

Copy link
Member

@picnixzpicnixz left a comment

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).

Copy link
Member

@ZeroIntensityZeroIntensity left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM.

@rruuaanng
Copy link
Contributor

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

@vstinner
Copy link
MemberAuthor

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

ZeroIntensity reacted with thumbs up emoji

@vstinner
Copy link
MemberAuthor

vstinner commentedSep 26, 2024
edited
Loading

I wrote a microbenchmark comparingequal 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 publicPyUnicode_Equal() is1.2x faster thanPyUnicode_Compare() (-2.0 ns) and 1.3x faster thanPyUnicode_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
Copy link
MemberAuthor

vstinner commentedSep 30, 2024
edited
Loading

Microbenchmark on comparison ofinequal 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

@vstinnervstinnerenabled auto-merge (squash)October 7, 2024 21:07
@vstinner
Copy link
MemberAuthor

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

The C API Working Group approved the API.

@vstinnervstinner merged commita7f0727 intopython:mainOct 7, 2024
37 checks passed
@vstinnervstinner deleted the unicode_equal branchOctober 7, 2024 21:24
@vstinner
Copy link
MemberAuthor

Merged. Thanks for reviews@picnixz and@ZeroIntensity.

ZeroIntensity and picnixz reacted with hooray emoji

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

Reviewers

@picnixzpicnixzpicnixz approved these changes

@ZeroIntensityZeroIntensityZeroIntensity approved these changes

@encukouencukouAwaiting requested review from encukouencukou is a code owner

+1 more reviewer

@rruuaanngrruuaanngrruuaanng left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vstinner@rruuaanng@picnixz@ZeroIntensity

[8]ページ先頭

©2009-2025 Movatter.jp