Skip to content

gh-145685: Improve scaling of type attribute lookups#145774

Open
colesbury wants to merge 2 commits intopython:mainfrom
colesbury:gh-145685-lock-free-type-lookup
Open

gh-145685: Improve scaling of type attribute lookups#145774
colesbury wants to merge 2 commits intopython:mainfrom
colesbury:gh-145685-lock-free-type-lookup

Conversation

@colesbury
Copy link
Contributor

@colesbury colesbury commented Mar 10, 2026

Avoid locking in the PyType_Lookup cache-miss path if the type's tp_version_tag is already valid.

Avoid locking in the PyType_Lookup cache-miss path if the type's
tp_version_tag is already valid.
@colesbury colesbury added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 10, 2026
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @colesbury for commit 68c122c 🤖

Results will be shown at:

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

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 10, 2026
@colesbury colesbury marked this pull request as ready for review March 10, 2026 19:25
@colesbury colesbury requested a review from markshannon as a code owner March 10, 2026 19:25
@colesbury colesbury requested a review from mpage March 10, 2026 19:25
Copy link
Contributor

@mpage mpage left a comment

Choose a reason for hiding this comment

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

Would it be worth adding a test case to the scaling benchmark?

END_TYPE_LOCK();
}
else {
res = find_name_in_mro(type, name, out);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to hold the type lock around this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The operations in find_name_in_mro are individually thread-safe, although this allows for mutations to types to be interleaved with the MRO traversal. I think that's acceptable. There were already edge cases where that could happen due to ensure_shared_on_read() calls suspending the critical section during dictionary lookups in find_name_in_mro.

Importantly, if any type in the MRO chain is modified during lookup, the assigned version tag will be stale so any future cache lookups won't use it.

@colesbury
Copy link
Contributor Author

Would it be worth adding a test case to the scaling benchmark?

This improves the scaling behavior, but it's not enough to scale linearly so I don't want to add a benchmark to ftscalingbench.py yet.

Here's the benchmark I've occasionally used locally: https://gist.github.com/colesbury/7d43e7dde89c1cb5574b5a2c73cc52b1

Some of the remaining issues:

  1. The code keeps getting re-specialized and the specialization code itself causes reference count contention. This seems fixable in follow up PRs

  2. Seq-lock contention in the MRO cache. This is trickier.

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.

3 participants