-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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-129149: Add fast paths to four more PyLong_From*
functions
#131211
base: main
Are you sure you want to change the base?
Conversation
PyLong_From* functions
PyLong_FromInt32()
, PyLong_FromInt64()
, PyLong_FromUInt32()
and PyLong_FromUInt64()
PyLong_From*
functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the tests also updated accordingly? (namely do they correctly test all paths?)
Pretty much the same as cpython/Modules/_testlimitedcapi/long.c Lines 772 to 781 in 1a8e574
and then using check_long_asint , where I've already added more values to cover more cases (especially negative small and medium-size integers): #129301 (comment)
|
Maybe add even more values? Definitely no performance bottleneck and might gain test coverage. E.g. one more positive and negative small integer? |
If we're already covering the small, medium and large integers, both in negative and positive ranges, it's fine. |
@vstinner are you fine with this? (sorry for the ping, though) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Use the macro
PYLONG_FROM_INT
to implementPyLong_FromInt32()
andPyLong_FromInt64()
, and the macroPYLONG_FROM_UINT
to implementPyLong_FromUInt32()
andPyLong_FromUInt64()
, because they have a fast path for small and medium-size integers.before:
after:
Benchmark done like in #129301 (comment).