Skip to content

GH-129149: Add fast paths to four more PyLong_From* functions #131211

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.

Sign up for GitHub

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

Jump to bottom
Merged
picnixz merged 4 commits into from
Mar 30, 2025
Merged

GH-129149: Add fast paths to four more PyLong_From* functions #131211

picnixz merged 4 commits into from
Mar 30, 2025

Conversation

@chris-eibl
Copy link
Member

@chris-eibl chris-eibl commented Mar 13, 2025 •
edited by bedevere-app bot
Loading

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

@bedevere-app bedevere-app bot mentioned this pull request Mar 13, 2025
Closed
@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
picnixz
picnixz reviewed Mar 13, 2025
View reviewed changes
picnixz
picnixz approved these changes Mar 13, 2025
View reviewed changes
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
Member Author

chris-eibl commented Mar 13, 2025

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

cpython/Modules/_testlimitedcapi/long.c

Lines 772 to 781 in 1a8e574

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
Member Author

chris-eibl 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?

@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
Member Author

chris-eibl commented Mar 22, 2025

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

vstinner
vstinner approved these changes Mar 24, 2025
View reviewed changes
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

@picnixz picnixz self-assigned this Mar 29, 2025
@picnixz picnixz merged commit a175d64 into python:main Mar 30, 2025
46 checks passed
@picnixz
Copy link
Member

picnixz commented Mar 30, 2025

Thank you!

@chris-eibl chris-eibl deleted the more_pylong_from branch March 30, 2025 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

@picnixz picnixz picnixz approved these changes

Assignees

@picnixz picnixz

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chris-eibl @picnixz @vstinner