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-129149: Add fast paths to four more PyLong_From* functions #131211

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

chris-eibl
Copy link
Contributor

@chris-eibl chris-eibl commented Mar 13, 2025

Use the macro PYLONG_FROM_INT to implement PyLong_FromInt32() and PyLong_FromInt64(), and the macro PYLONG_FROM_UINT to implement PyLong_FromUInt32() and PyLong_FromUInt64(), because they have a fast path for small and medium-size integers.

before:

PyLong_FromInt32: some small ints
          -5:   228.22 ms
           0:   218.91 ms
         256:   214.52 ms
PyLong_FromInt32: some medium ints
 -1073741823:   313.45 ms
        1000:   196.13 ms
  1073741822:   305.81 ms
PyLong_FromInt32: some bigger ints
 -1073741824:   305.89 ms
  1073741824:   283.47 ms

after:

PyLong_FromInt32: some small ints
          -5:    21.09 ms
           0:    20.80 ms
         256:    20.27 ms
PyLong_FromInt32: some medium ints
 -1073741823:   100.03 ms
        1000:   101.13 ms
  1073741822:    98.36 ms
PyLong_FromInt32: some bigger ints
 -1073741824:   204.82 ms
  1073741824:   205.22 ms

Benchmark done like in #129301 (comment).

@chris-eibl chris-eibl changed the title GH-129149: Add fast path for small and medium-size integers in PyLong_FromInt32(), PyLong_FromInt64(), PyLong_FromUInt32() and PyLong_FromUInt64() GH-129149: Add fast paths to four more PyLong_From* functions Mar 13, 2025
@picnixz picnixz self-requested a review March 13, 2025 20:40
Copy link
Member

@picnixz picnixz left a 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?)

@chris-eibl
Copy link
Contributor Author

Pretty much the same as test_long_as_ssize_t we've discussed in #129301: via round tripping e.g. for in PyLong_FromUInt32

static PyObject *
pylong_asuint32(PyObject *module, PyObject *arg)
{
NULLABLE(arg);
uint32_t value;
if (PyLong_AsUInt32(arg, &value) < 0) {
return NULL;
}
return PyLong_FromUInt32(value);
}

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)

@chris-eibl
Copy link
Contributor Author

Maybe add even more values? Definitely no performance bottleneck and might gain test coverage. E.g. one more positive and negative small integer?

@picnixz
Copy link
Member

picnixz commented Mar 13, 2025

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.

@chris-eibl
Copy link
Contributor Author

@vstinner are you fine with this? (sorry for the ping, though)

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants