FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-145685: per-type method cache implementation by kumaraditya303 · Pull Request #150160 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-145685: per-type method cache implementation - #150160

Merged
kumaraditya303 merged 32 commits into
python:mainfrom
kumaraditya303:mrocache
Jul 21, 2026
ConversationCommits (32)ChecksFiles changed
Merged

gh-145685: per-type method cache implementation#150160
kumaraditya303 merged 32 commits into
python:mainfrom
kumaraditya303:mrocache

Conversation

kumaraditya303 commented May 20, 2026 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

bedevere-app Bot mentioned this pull request May 20, 2026
Closed
kumaraditya303 marked this pull request as ready for review May 21, 2026 17:03
kumaraditya303 requested a review from colesbury May 22, 2026 11:54
kumaraditya303 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 27, 2026

bedevere-bot commented May 27, 2026

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @kumaraditya303 for commit 728021c 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F150160%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 27, 2026
colesbury reviewed Jun 15, 2026
View reviewed changes

colesbury left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

A few more comments below (mostly found by Claude). Also:

  • Have you run single-threaded benchmarks to ensure that we haven't introduced any regressions with this?
  • Does the type_lookup benchmark scale as well as the other scaling benchmarks? In other words, should we consider this a partial or full fix for the type cache scaling issues?

Comment thread Objects/typeobject.c
Comment thread Objects/typeobject.c Outdated
Comment thread Python/typecache.c
Comment thread Include/internal/pycore_typecache.h Outdated

read-the-docs-community Bot commented Jun 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #33682891 | 📁 Comparing 42a6ae8 against main (8646385)

  🔍 Preview build  

155 files changed · + 3 added · ± 151 modified · - 1 deleted

+ Added

± Modified

- Deleted

dpdani reviewed Jun 25, 2026
View reviewed changes
Comment thread Python/typecache.c

kumaraditya303 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Have you run single-threaded benchmarks to ensure that we haven't introduced any regressions with this?

Benchmark results show that it is 1% slower on gil enabled build https://github.com/facebookexperimental/free-threading-benchmarking/blob/main/results/bm-20260616-3.16.0a0-4fbec93/bm-20260616-vultr-x86_64-kumaraditya303-mrocache-3.16.0a0-4fbec93-vs-base.md

I think it it is within the error of benchmark machines and not significant.

markshannon commented Jun 25, 2026

Copy link
Copy Markdown
Member

I'm very wary of dismissing 1% slower as "within the noise" without some other evidence that there is no slowdown for a change like this.

It also looks like you've missed several of the stats.
How do the stats look for cache hits, misses and collisions?

How does this change memory use?

colesbury commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Results from running locally on my machine (via Claude, GIL-enabled, clang-20, optimized, lto=thin) look good: https://gist.github.com/colesbury/1e5c1eb4ccc1e626696b568d18d8bbc4. Cache misses drop substantially, but the overall effect is small (possibly within the noise margin) because:

  • the MRO cache doesn't matter as much now that we rely heavily on specialization.
  • the MRO hit rate was already high on these benchmarks

A few more suggestions:

  • We should drop the type_cache_collisions stat because it's not longer tracked and doesn't make sense in the same way. Previously a collision in the MRO cache would overwrite the previous cache entry; now we do linear probing.
  • We should track the number of times we resize/clear cache a type's cache

kumaraditya303 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

We should track the number of times we resize/clear cache a type's cache

I've added stats for resizes and invalidations and removed collisions.

Merge branch 'main' of https://github.com/python/cpython into mrocache
2f6ccde 
# Conflicts:
#	Lib/test/test_free_threading/test_type.py

kumaraditya303 commented Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@markshannon Do you have more comments/concerns on this?

dpdani commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

I would still like to see the top comment "Lock-free per type method cache implementation" rephrased since it's not a lock free implementation.

kumaraditya303 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

I would still like to see the top comment "Lock-free per type method cache implementation" rephrased since it's not a lock free implementation.

How would you rephrase it, do you have a suggestion?

dpdani commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

How about just "Per-type method cache implementation"?

kumaraditya303 merged commit daf09e1 into python:main Jul 21, 2026
71 of 72 checks passed
kumaraditya303 deleted the mrocache branch July 21, 2026 08:41
fedonman mentioned this pull request Aug 7, 2026
Open
matthiasgoergens pushed a commit to matthiasgoergens/cpython that referenced this pull request Sep 27, 2026
perf-notes: first CI timing results (controls, prototypes, disputes) …
0cc0e89 
…and Ir result for python#150160 [skip ci]

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016goMvuYFRBF79g8A7LkRrG
matthiasgoergens pushed a commit to matthiasgoergens/cpython that referenced this pull request Sep 27, 2026
perf-notes: python#150160 per-function Ir diagnosis [skip ci]
220159f
matthiasgoergens pushed a commit to matthiasgoergens/cpython that referenced this pull request Sep 27, 2026
perf-notes: first CI timing results (controls, prototypes, disputes) …
a4b0582 
…and Ir result for python#150160 [skip ci]
matthiasgoergens pushed a commit to matthiasgoergens/cpython that referenced this pull request Sep 27, 2026
perf-notes: python#150160 per-function Ir diagnosis [skip ci]
d2a03f3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

colesbury colesbury left review comments
zooba zooba left review comments
ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
ZeroIntensity Awaiting requested review from ZeroIntensity ZeroIntensity is a code owner
FFY00 Awaiting requested review from FFY00 FFY00 is a code owner
markshannon Awaiting requested review from markshannon markshannon is a code owner
erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner
AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner
emmatyping Awaiting requested review from emmatyping emmatyping is a code owner
itamaro Awaiting requested review from itamaro itamaro is a code owner
+1 more reviewer
dpdani dpdani left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL