Skip to content

gh-134043: use stackrefs for dict lookup in _PyObject_GetMethodStackRef#136412

Merged
kumaraditya303 merged 12 commits intopython:mainfrom
kumaraditya303:dict
Jul 28, 2025
Merged

gh-134043: use stackrefs for dict lookup in _PyObject_GetMethodStackRef#136412
kumaraditya303 merged 12 commits intopython:mainfrom
kumaraditya303:dict

Conversation

@kumaraditya303
Copy link
Contributor

@kumaraditya303 kumaraditya303 commented Jul 8, 2025

Changes _PyObject_GetMethodStackRef to use _PyDict_GetMethodStackRef to avoid reference counting on deferred objects.

@kumaraditya303 kumaraditya303 marked this pull request as ready for review July 8, 2025 12:05
@kumaraditya303 kumaraditya303 requested a review from colesbury July 8, 2025 12:05
Copy link
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

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

  1. We should add a scaling benchmark to ftscalingbench.py for the relevant access pattern
  2. We probably need to avoid Py_INCREF(dict) and that's going to require refactoring. You can safely avoid Py_INCREF(dict) in the same cases where _Py_dict_lookup_threadsafe_stackref is relevant: when the dictionary keys are DICT_KEYS_UNICODE.

@colesbury
Copy link
Contributor

This needs more refactoring - we can't load the keys twice like this because the condition might not hold.

@kumaraditya303 kumaraditya303 changed the title gh-134043: use _Py_dict_lookup_threadsafe_stackref for dict lookup in _PyObject_GetMethodStackRef gh-134043: use _Py_dict_lookup_unicode_threadsafe_stackref for dict lookup in _PyObject_GetMethodStackRef Jul 18, 2025
@kumaraditya303 kumaraditya303 changed the title gh-134043: use _Py_dict_lookup_unicode_threadsafe_stackref for dict lookup in _PyObject_GetMethodStackRef gh-134043: use stackrefs for dict lookup in _PyObject_GetMethodStackRef Jul 24, 2025
@kumaraditya303 kumaraditya303 requested a review from colesbury July 24, 2025 12:45
@kumaraditya303 kumaraditya303 merged commit cbe6ebe into python:main Jul 28, 2025
44 checks passed
@kumaraditya303 kumaraditya303 deleted the dict branch July 28, 2025 16:35
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
…dStackRef` (python#136412)

Co-authored-by: Sam Gross <colesbury@gmail.com>
@colesbury
Copy link
Contributor

I'm going to backport this to 3.14 because it will simplify backporting #145826, which uses some of the same functions and macros.

@colesbury colesbury added the needs backport to 3.14 bugs and security fixes label Mar 17, 2026
@miss-islington-app
Copy link

Thanks @kumaraditya303 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @kumaraditya303, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker cbe6ebe15b3b8db00d3ca82a408cfd62b6d93b7d 3.14

colesbury added a commit to colesbury/cpython that referenced this pull request Mar 17, 2026
…etMethodStackRef` (pythonGH-136412)

(cherry picked from commit cbe6ebe)

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
Co-authored-by: Sam Gross <colesbury@gmail.com>
colesbury added a commit to colesbury/cpython that referenced this pull request Mar 17, 2026
…etMethodStackRef` (pythonGH-136412)

(cherry picked from commit cbe6ebe)

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
Co-authored-by: Sam Gross <colesbury@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Mar 17, 2026

GH-146077 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Mar 17, 2026
colesbury added a commit that referenced this pull request Mar 17, 2026
…odStackRef` (GH-136412) (#146077)

(cherry picked from commit cbe6ebe)

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
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.

2 participants