Skip to content

gh-136599: Improve long_hash #136600

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

Merged
vstinner merged 20 commits into from
Sep 4, 2025
Merged

gh-136599: Improve long_hash #136600

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
146f5aa
Add test for long_hash
eendebakpt Jul 12, 2025
a162da2
Unroll first digit calculation in long_hash
eendebakpt Jul 12, 2025
4f9fc76
Unroll second digit calculation in long_hash
eendebakpt Jul 12, 2025
07bce4b
whitespace
eendebakpt Jul 12, 2025
32341de
add gh number
eendebakpt Jul 12, 2025
194fb7a
Merge branch 'long_hash' of github.com:eendebakpt/cpython into long_hash
eendebakpt Jul 12, 2025
a48860f
review comments
eendebakpt Jul 13, 2025
6d3754b
fix test on wasi
eendebakpt Jul 13, 2025
fec9fbe
add news entry
eendebakpt Jul 13, 2025
08d7ba9
review comments
eendebakpt Jul 25, 2025
14a90f1
Merge branch 'main' into long_hash
eendebakpt Aug 31, 2025
76c4f6a
Apply suggestions from code review
eendebakpt Sep 1, 2025
f720557
Update Objects/longobject.c
eendebakpt Sep 1, 2025
55e5bd9
Merge branch 'main' into long_hash
eendebakpt Sep 2, 2025
0aa56f0
Merge branch 'long_hash' of github.com:eendebakpt/cpython into long_hash
eendebakpt Sep 2, 2025
c1a3184
review comment
eendebakpt Sep 2, 2025
b9a487d
fix assert
eendebakpt Sep 3, 2025
b0fd0d8
Update Objects/longobject.c
eendebakpt Sep 3, 2025
c6e060d
Apply suggestions from code review
eendebakpt Sep 3, 2025
9b6e628
Update Objects/longobject.c
vstinner Sep 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions Lib/test/test_long.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,5 +1693,22 @@ class MyInt(int):
# GH-117195 -- This shouldn't crash
object.__sizeof__(1)

def test_hash(self):
Copy link
Copy Markdown
Member

@skirpichev skirpichev Sep 1, 2025

Choose a reason for hiding this comment

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

I think that part could be in a separate pr, merged first.

The docs describe algorithm in details:
https://docs.python.org/3/library/stdtypes.html#hashing-of-numeric-types

Maybe we could test it against pure-Python implementation, using also hypothesis?

# gh-136599
self.assertEqual(hash(-1), -2)
self.assertEqual(hash(0), 0)
self.assertEqual(hash(10), 10)

self.assertEqual(hash(sys.hash_info.modulus - 2), sys.hash_info.modulus - 2)
self.assertEqual(hash(sys.hash_info.modulus - 1), sys.hash_info.modulus - 1)
self.assertEqual(hash(sys.hash_info.modulus), 0)
self.assertEqual(hash(sys.hash_info.modulus + 1), 1)

self.assertEqual(hash(-sys.hash_info.modulus - 2), -2)
self.assertEqual(hash(-sys.hash_info.modulus - 1), -2)
self.assertEqual(hash(-sys.hash_info.modulus), 0)
self.assertEqual(hash(-sys.hash_info.modulus + 1), - (sys.hash_info.modulus - 1))
Copy link
Copy Markdown
Member

@skirpichev skirpichev Sep 1, 2025

Choose a reason for hiding this comment

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

Suggested change
self.assertEqual(hash(-sys.hash_info.modulus + 1), - (sys.hash_info.modulus - 1))
self.assertEqual(hash(-sys.hash_info.modulus + 1), -sys.hash_info.modulus + 1)



if __name__ == "__main__":
unittest.main()
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2025-07-13-21-21-17.gh-issue-136599.sLhm2O.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve performance of :class:`int` hash calculations.
15 changes: 14 additions & 1 deletion Objects/longobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3676,7 +3676,20 @@ long_hash(PyObject *obj)
}
i = _PyLong_DigitCount(v);
sign = _PyLong_NonCompactSign(v);
x = 0;

// unroll first two digits
#if ( PyHASH_BITS > PyLong_SHIFT )
Copy link
Copy Markdown
Member

@skirpichev skirpichev Sep 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

PyHASH_BITS can be either 31 or 61, while PyLong_SHIFT - either 15 or 30. So, this condition is always true.

BTW, it seems the 15-bit digit is untested by regular CI. Is there at least some buildbot with such settings? I'll open an issue.

Suggested change
#if ( PyHASH_BITS > PyLong_SHIFT )

Copy link
Copy Markdown
Contributor Author

@eendebakpt eendebakpt Sep 1, 2025

Choose a reason for hiding this comment

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

The case is indeed untested, but I added it because of a comment by Serhiy #136600 (comment)

Copy link
Copy Markdown
Member

@skirpichev skirpichev Sep 1, 2025

Choose a reason for hiding this comment

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

The case is indeed untested

FYI: #138336

I added it because of a comment by Serhiy

Then fine. Though, an assert might be option.

Copy link
Copy Markdown
Member

@vstinner vstinner Sep 2, 2025

Choose a reason for hiding this comment

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

You can replace the test with a build assertion: Py_BUILD_ASSERT(PyHASH_BITS > PyLong_SHIFT);

assert(i>=2);
Copy link
Copy Markdown
Member

@chris-eibl chris-eibl Sep 1, 2025

Choose a reason for hiding this comment

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

Maybe just use assert(i>=1) here and then again in the second #if below?

Otherwise LGTM.

--i;
x = v->long_value.ob_digit[i];
assert(x < PyHASH_MODULUS);
#endif
Comment thread
skirpichev marked this conversation as resolved.
Outdated
#if ( PyHASH_BITS > (2 * PyLong_SHIFT) )
Comment thread
eendebakpt marked this conversation as resolved.
Outdated
--i;
x = ((x << PyLong_SHIFT));
Comment thread
eendebakpt marked this conversation as resolved.
Outdated
x += v->long_value.ob_digit[i];
assert(x < PyHASH_MODULUS);
#endif
Comment thread
vstinner marked this conversation as resolved.
while (--i >= 0) {
/* Here x is a quantity in the range [0, _PyHASH_MODULUS); we
want to compute x * 2**PyLong_SHIFT + v->long_value.ob_digit[i] modulo
Expand Down
Loading