Skip to content

Caching the tuple hash calculation speeds up some code significantly #131525

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
Open
mdboom opened this issue Mar 20, 2025 · 1 comment
Open

Caching the tuple hash calculation speeds up some code significantly #131525

mdboom opened this issue Mar 20, 2025 · 1 comment
Assignees
@mdboom
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage type-feature A feature request or enhancement

Comments

@mdboom
Copy link
Contributor

mdboom commented Mar 20, 2025 •
edited by bedevere-app bot
Loading

Proposal:

Back in 2013, it was determined that caching the result of tuple_hash did not have any significant speedup.

However, a lot has changed since then, and in a recent experiment to add a tuple hash cache back in, the mdp benchmark increased by 86%.

Admittedly, there was no measurable improvement on any other benchmark, but it also seems to have no downside, including for memory usage when measured with max_rss.

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

  • [ ]

Linked PRs
@mdboom mdboom added the performance Performance or resource usage label Mar 20, 2025
@mdboom mdboom self-assigned this Mar 20, 2025
mdboom added a commit to mdboom/cpython that referenced this issue Mar 20, 2025
@mdboom
pythongh-131525: Cache the result of tuple_hash
54e07d1
mdboom added a commit to mdboom/cpython that referenced this issue Mar 20, 2025
@mdboom
pythongh-131525: Cache the result of tuple_hash
b3c3977
@bedevere-app bedevere-app bot mentioned this issue Mar 20, 2025
Merged
@rhettinger
Copy link
Contributor

This would nicely simplify and speed-up the pure python implementation of lru_cache. The _HashedSeq class would no longer be needed, and a plain tuple would suffice.

@picnixz picnixz added type-feature A feature request or enhancement interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Mar 22, 2025
mdboom added a commit that referenced this issue Mar 27, 2025
@mdboom @picnixz @chris-eibl
gh-131525: Cache the result of tuple_hash (#131529)
8614f86 
* gh-131525: Cache the result of tuple_hash

* Fix debug builds

* Add blurb

* Fix formatting

* Pre-compute empty tuple singleton

* Mostly set the cache within tuple_alloc

* Fixes for TSAN

* Pre-compute empty tuple singleton

* Fix for 32-bit platforms

* Assert that op != NULL in _PyTuple_RESET_HASH_CACHE

* Use FT_ATOMIC_STORE_SSIZE_RELAXED macro

* Update Include/internal/pycore_tuple.h

Co-authored-by: Bénédikt Tran <[email protected]>

* Fix alignment

* atomic load

* Update Objects/tupleobject.c

Co-authored-by: Chris Eibl <[email protected]>

---------

Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Chris Eibl <[email protected]>
lgeiger added a commit to lgeiger/cpython that referenced this issue Mar 31, 2025
@lgeiger
pythongh-131525: Remove _HashedSeq wrapper from lru_cache
685f8fe
@lgeiger lgeiger mentioned this issue Mar 31, 2025
Merged
lgeiger added a commit to lgeiger/cpython that referenced this issue Mar 31, 2025
@lgeiger
pythongh-131525: Remove _HashedSeq wrapper from lru_cache
7c4dfe6
rhettinger pushed a commit that referenced this issue Mar 31, 2025
@lgeiger
gh-131525: Remove _HashedSeq wrapper from lru_cache (gh-131922)
0147be0
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 1, 2025
@mdboom @picnixz
@chris-eibl @diegorusso
pythongh-131525: Cache the result of tuple_hash (python#131529)
10e8e0c 
* pythongh-131525: Cache the result of tuple_hash

* Fix debug builds

* Add blurb

* Fix formatting

* Pre-compute empty tuple singleton

* Mostly set the cache within tuple_alloc

* Fixes for TSAN

* Pre-compute empty tuple singleton

* Fix for 32-bit platforms

* Assert that op != NULL in _PyTuple_RESET_HASH_CACHE

* Use FT_ATOMIC_STORE_SSIZE_RELAXED macro

* Update Include/internal/pycore_tuple.h

Co-authored-by: Bénédikt Tran <[email protected]>

* Fix alignment

* atomic load

* Update Objects/tupleobject.c

Co-authored-by: Chris Eibl <[email protected]>

---------

Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Chris Eibl <[email protected]>
seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
@mdboom @picnixz
@chris-eibl @seehwan80
pythongh-131525: Cache the result of tuple_hash (python#131529)
6dbfa42 
* pythongh-131525: Cache the result of tuple_hash

* Fix debug builds

* Add blurb

* Fix formatting

* Pre-compute empty tuple singleton

* Mostly set the cache within tuple_alloc

* Fixes for TSAN

* Pre-compute empty tuple singleton

* Fix for 32-bit platforms

* Assert that op != NULL in _PyTuple_RESET_HASH_CACHE

* Use FT_ATOMIC_STORE_SSIZE_RELAXED macro

* Update Include/internal/pycore_tuple.h

Co-authored-by: Bénédikt Tran <[email protected]>

* Fix alignment

* atomic load

* Update Objects/tupleobject.c

Co-authored-by: Chris Eibl <[email protected]>

---------

Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Chris Eibl <[email protected]>
seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
@lgeiger @seehwan80
pythongh-131525: Remove _HashedSeq wrapper from lru_cache (python…
78a7a91 
…gh-131922)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mdboom mdboom

Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mdboom @rhettinger @picnixz