Skip to content

Replace _pysqlite_long_from_int64() with PyLong_FromLongLong() #16882

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

Merged
merged 1 commit into from
Oct 23, 2019

Conversation

sir-sigurd
Copy link
Contributor

No description provided.

PyObject *
_pysqlite_long_from_int64(sqlite_int64 value)
{
# if SIZEOF_LONG_LONG < 8
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unneeded since long long is guaranteed to be at least 64-bit.

Comment on lines -115 to -119
# if SIZEOF_LONG < SIZEOF_LONG_LONG
if (value > LONG_MAX || value < LONG_MIN)
return PyLong_FromLongLong(value);
# endif
return PyLong_FromLong(Py_SAFE_DOWNCAST(value, sqlite_int64, long));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that using PyLong_FromLong() for small values on 32 bit platforms provides speed-up, but if it does that should be done in PyLong_FromLongLong().
Moreover AFAIK long is 32 bit on 64 bit Windows, so that condition is not compiled out.

Copy link
Contributor

@eamanu eamanu left a comment

Choose a reason for hiding this comment

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

LGTM but you should be open a issue on bpo to more discussion

@serhiy-storchaka serhiy-storchaka merged commit b6f5b9d into python:master Oct 23, 2019
@sir-sigurd sir-sigurd deleted the pysqlite_long_from_int64 branch October 23, 2019 08:16
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
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.

5 participants