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

gh-120176: Reduce number of compares in index range checks #120181

Closed
wants to merge 1 commit into from

Conversation

lgeiger
Copy link
Contributor

@lgeiger lgeiger commented Jun 6, 2024

This PR is a followup to #9784 (discussed in #72583 and #67741) and extracts the helper function into internal/pycore_abstract.h and uses it where applicable to reduce the number of comparisons in index range checks.

See https://godbolt.org/z/jM8oexT1d for an assembly comparison.

I feel like this change doesn't reduce readability but let me know if such micro optimisation does not justify the code churn.

Copy link

cpython-cla-bot bot commented Jun 6, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Jun 6, 2024

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 skip news label instead.

@methane
Copy link
Member

methane commented Jun 8, 2024

No news entry is required because this change is not visible to users.
I assume performance gain is not large enough to have news entry.

Copy link
Member

@methane methane left a comment

Choose a reason for hiding this comment

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

LGTM.
I will wait a week before merging this PR to ensure that other committers have the opportunity to review it.

@eendebakpt
Copy link
Contributor

@lgeiger @methane This pr is nearly identical to #99762. There are some more review comments there

@serhiy-storchaka
Copy link
Member

This idea was already discussed and rejected. See #72583.

@lgeiger
Copy link
Contributor Author

lgeiger commented Jun 8, 2024

Ah sorry, didn't see #99762

Sorry about that!

@lgeiger lgeiger deleted the faster-index-checks branch June 8, 2024 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants