bpo-30509: Optimize and clean up calling type slots.#1861
bpo-30509: Optimize and clean up calling type slots.#1861serhiy-storchaka wants to merge 7 commits intopython:mainfrom
Conversation
|
@serhiy-storchaka, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tim-one, @larryhastings and @benjaminp to be potential reviewers. |
There was a problem hiding this comment.
Why do you make this function public? (you removed static)
There was a problem hiding this comment.
lookup_maybe() was used only in _PyObject_LookupSpecial() and set_names(). The latter is not performance critical and can use _PyObject_LookupSpecial() instead of lookup_maybe(). Thus I just inlined lookup_maybe() in the single place where it is used, in _PyObject_LookupSpecial().
There was a problem hiding this comment.
Oh, I see. I didn't notice that _PyObject_LookupSpecial() already exist in the current code. I checked, it's called from various places in CPython :-) Your change makes sense ;-)
serhiy-storchaka
force-pushed
the
type-slot-calls
branch
from
2625c29 to
dac5ba7
Compare
| } | ||
|
|
||
| if (func == NULL) { | ||
| PyErr_Clear(); |
There was a problem hiding this comment.
I don't understand why PyErr_Clear() is needed with your change.
There was a problem hiding this comment.
Actually it is not needed.
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
While I am not 100% convinced that the change makes Python faster, the change itself now LGTM thanks to the first cleanup commit. I let you decide to merge it or not ;-) (my vote is +0)
|
@serhiy-storchaka were you interested in revisiting this to see if it should be merged or not? Thanks! |
|
This PR is stale because it has been open for 30 days with no activity. |
github-actions
Bot
removed
the
stale
|
This PR is stale because it has been open for 30 days with no activity. |
github-actions
Bot
removed
the
stale
|
This PR is stale because it has been open for 30 days with no activity. |
github-actions
Bot
removed
the
stale
|
This PR is stale because it has been open for 30 days with no activity. |
|
It would be interesting to rebase this PR on the main branch. |
github-actions
Bot
removed
the
stale
|
This PR is stale because it has been open for 30 days with no activity. |
10 participants
No description provided.