bpo-30509: Optimize and clean up calling type slots. by serhiy-storchaka · Pull Request #1861

bpo-30509: Optimize and clean up calling type slots. by serhiy-storchaka · Pull Request #1861 · python/cpython

Choose a reason for hiding this comment

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

Why do you make this function public? (you removed static)

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

		}

		if (func == NULL) {
		PyErr_Clear();

Choose a reason for hiding this comment

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

I don't understand why PyErr_Clear() is needed with your change.

Choose a reason for hiding this comment

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

Actually it is not needed.

Choose a reason for hiding this comment

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

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.

It would be interesting to rebase this PR on the main branch.

This PR is stale because it has been open for 30 days with no activity.