Skip to content

GH-127705: Use _PyStackRefs in the default build. #127875

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.

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
merged 75 commits into from
Mar 10, 2025

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Dec 12, 2024

We already have _PyStackRefs in the default build, but they are just PyObject * pointers cast to a pointer-sized int.

This PR adds tags according to the tagging scheme in #127705.

markshannon and others added 2 commits March 3, 2025 15:12
…e-127705.Qad2hx.rst

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
@mpage mpage requested a review from colesbury March 3, 2025 17:47
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

This mostly seems okay. FYI, I didn't do a deep dive into the code to verifying correctness, like I normally do. I did read through all the files except for Include/internal/pycore_stackref.h. The changes make sense and I can see how they simplify things.

@mpage
Copy link
Contributor

mpage commented Mar 6, 2025

@markshannon - Have you measured the effect of the mortality optimizations independently from the changes to use macros? The tag bits are a precious commodity and the mortality optimizations require two bits.

@markshannon
Copy link
Member Author

markshannon commented Mar 7, 2025

@mpage. I changed the PR to use a single bit, using _Py_IsImmortal() in a few places where we need to distinguish between tagged pointers to mortal and immortal objects.
It has surprisingly little effect, the number differ per platform but the net effect is still about 0.

  • Linux (x86-64) 1.0% slower
  • MacOS 0.1% faster
  • Windows 1.1% faster

Full results

@@ -1093,6 +1104,7 @@ dummy_func(
inst(RETURN_VALUE, (retval -- res)) {
assert(frame->owner != FRAME_OWNED_BY_INTERPRETER);
_PyStackRef temp = retval;
assert(PyStackRef_IsHeapSafe(temp));
Copy link
Member

Choose a reason for hiding this comment

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

Why the assert here? This blocks stackrefs from passing through to the host frame, which blocks a lot of unboxed and deferred refcounting optimizations.

Copy link
Member Author

@markshannon markshannon Mar 10, 2025

Choose a reason for hiding this comment

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

It blocks a lot of unsafe operations. Passing references towards the caller is unsafe as it is possible that the only immediate reference count (stored in ob_refcnt) for an object is stored in the callee, and will be deleted during the return.

@markshannon markshannon merged commit 2bef8ea into python:main Mar 10, 2025
71 checks passed
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot s390x RHEL9 Refleaks 3.x (tier-3) has failed when building commit 2bef8ea.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1589/builds/1084) and take a look at the build logs.
  4. Check if the failure is related to this commit (2bef8ea) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1589/builds/1084

Failed tests:

  • test_import

Test leaking resources:

  • test_xml_etree_c: references
  • test_ast: references
  • test___all__: references
  • test_bdb: references
  • test_pydoc: references
  • test_import: memory blocks
  • test_pyclbr: references
  • test_zipfile: references
  • test_idle: references
  • test_datetime: references
  • test_import: references

Summary of the results of the build (if available):

==

Click to see traceback logs
TracebackTests.test_exec_failure) ... ok


TracebackTests.test_syntax_error) ... ok


TracebackTests.test_broken_from) ... ok


TracebackTests.test_broken_submodule) ... ok


TracebackTests.test_broken_parent) ... ok


TracebackTests.test_unencodable_filename) ... ok


TracebackTests.test_exec_failure_nested) ... ok


TracebackTests.test_import_bug) ... ok


TracebackTests.test_broken_parent_from) ... ok


TracebackTests.test_nonexistent_module) ... ok


TracebackTests.test_nonexistent_module_nested) ... ok

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot s390x RHEL8 Refleaks 3.x (tier-3) has failed when building commit 2bef8ea.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/75/builds/2284) and take a look at the build logs.
  4. Check if the failure is related to this commit (2bef8ea) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/75/builds/2284

Failed tests:

  • test_import

Test leaking resources:

  • test___all__: references
  • test_timeit: references
  • test_import: memory blocks
  • test_zipfile: references
  • test_idle: references
  • test_import: references

Summary of the results of the build (if available):

==

Click to see traceback logs
TracebackTests.test_exec_failure) ... ok


TracebackTests.test_syntax_error) ... ok


TracebackTests.test_broken_from) ... ok


TracebackTests.test_broken_submodule) ... ok


TracebackTests.test_broken_parent) ... ok


TracebackTests.test_unencodable_filename) ... ok


TracebackTests.test_syntax_error) ... o
k


TracebackTests.test_exec_failure_nested) ... ok


TracebackTests.test_import_bug) ... ok


TracebackTests.test_broken_parent_from) ... ok


TracebackTests.test_nonexistent_module) ... ok


TracebackTests.test_nonexistent_module_nested) ... ok

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot aarch64 RHEL8 Refleaks 3.x (tier-2) has failed when building commit 2bef8ea.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/551/builds/887) and take a look at the build logs.
  4. Check if the failure is related to this commit (2bef8ea) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/551/builds/887

Failed tests:

  • test_marshal
  • test_idle
  • test_import
  • test_datetime
  • test___all__
  • test_zipfile
  • test_pyclbr
  • test.test_pydoc.test_pydoc

Test leaking resources:

  • test___all__: references
  • test_pydoc: references
  • test_import: memory blocks
  • test_pyclbr: references
  • test_idle: references
  • test_zipfile: references
  • test_datetime: references
  • test_import: references
  • test_marshal: references

Summary of the results of the build (if available):

==

Click to see traceback logs
TracebackTests.test_exec_failure) ... ok


TracebackTests.test_syntax_error) ... ok


TracebackTests.test_broken_from) ... ok


TracebackTests.test_broken_submodule) ... ok


TracebackTests.test_broken_parent) ... ok


TracebackTests.test_unencodable_filename) ... ok


TracebackTests.test_exec_failure_nested) ... ok


TracebackTests.test_import_bug) ... ok


TracebackTests.test_broken_parent_from) ... ok


TracebackTests.test_nonexistent_module) ... ok


TracebackTests.test_nonexistent_module_nested) ... ok

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 RHEL8 Refleaks 3.x (tier-1) has failed when building commit 2bef8ea.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/259/builds/2292) and take a look at the build logs.
  4. Check if the failure is related to this commit (2bef8ea) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/259/builds/2292

Failed tests:

  • test_import

Test leaking resources:

  • test_xml_etree_c: references
  • test_inspect: references
  • test_ast: references
  • test_capi: references
  • test_import: memory blocks
  • test_pyclbr: references
  • test_idle: references
  • test_zipfile: references
  • test_import: references

Summary of the results of the build (if available):

==

Click to see traceback logs
TracebackTests.test_exec_failure) ... ok


TracebackTests.test_syntax_error) ... ok


TracebackTests.test_broken_from) ... ok


TracebackTests.test_broken_submodule) ... ok


TracebackTests.test_broken_parent) ... ok


TracebackTests.test_unencodable_filename) ... ok


TracebackTests.test_exec_failure_nested) ... ok


TracebackTests.test_import_bug) ... ok


TracebackTests.test_broken_parent_from) ... ok


TracebackTests.test_nonexistent_module) ... ok


TracebackTests.test_nonexistent_module_nested) ... ok

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Fedora Stable Refleaks 3.x (tier-1) has failed when building commit 2bef8ea.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/320/builds/2091) and take a look at the build logs.
  4. Check if the failure is related to this commit (2bef8ea) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/320/builds/2091

Failed tests:

  • test_idle
  • test_bdb
  • test_import
  • test_runpy
  • test_datetime
  • test___all__
  • test_capi
  • test_ast
  • test_decorators
  • test_zipfile
  • test_pyclbr

Test leaking resources:

  • test_runpy: memory blocks
  • test_ast: references
  • test___all__: references
  • test_runpy: references
  • test_capi: references
  • test_bdb: references
  • test_import: memory blocks
  • test_pyclbr: references
  • test_zipfile: references
  • test_idle: references
  • test_decorators: references
  • test_datetime: references
  • test_import: references

Summary of the results of the build (if available):

==

Click to see traceback logs
TracebackTests.test_exec_failure) ... ok


TracebackTests.test_syntax_error) ... ok


TracebackTests.test_broken_from) ... ok


TracebackTests.test_broken_submodule) ... ok


TracebackTests.test_broken_parent) ... ok


TracebackTests.test_unencodable_filename) ... ok


TracebackTests.test_exec_failure_nested) ... ok


TracebackTests.test_import_bug) ... ok


TracebackTests.test_broken_parent_from) ... ok


TracebackTests.test_nonexistent_module) ... ok


TracebackTests.test_nonexistent_module_nested) ... ok

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 FreeBSD Refleaks 3.x (tier-3) has failed when building commit 2bef8ea.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1613/builds/926) and take a look at the build logs.
  4. Check if the failure is related to this commit (2bef8ea) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1613/builds/926

Failed tests:

  • test_import

Test leaking resources:

  • test_timeit: references
  • test_import: memory blocks
  • test_idle: references
  • test_zipfile: references
  • test_import: references

Summary of the results of the build (if available):

==

Click to see traceback logs
TracebackTests.test_exec_failure) ... ok


TracebackTests.test_syntax_error) ... ok


TracebackTests.test_broken_from) ... ok


TracebackTests.test_broken_submodule) ... ok


TracebackTests.test_broken_parent) ... ok


TracebackTests.test_unencodable_filename) ... ok


TracebackTests.test_exec_failure_nested) ... ok


TracebackTests.test_import_bug) ... ok


TracebackTests.test_broken_parent_from) ... ok


TracebackTests.test_nonexistent_module) ... ok


TracebackTests.test_nonexistent_module_nested) ... ok

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE RHEL8 Refleaks 3.x (tier-2) has failed when building commit 2bef8ea.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/384/builds/1779) and take a look at the build logs.
  4. Check if the failure is related to this commit (2bef8ea) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/384/builds/1779

Failed tests:

  • test_ast
  • test_zipfile
  • test_idle
  • test_xml_etree_c
  • test_bdb
  • test_import
  • test_datetime

Test leaking resources:

  • test_xml_etree_c: references
  • test_ast: references
  • test_bdb: references
  • test_import: memory blocks
  • test_idle: references
  • test_zipfile: references
  • test_datetime: references
  • test_import: references

Summary of the results of the build (if available):

==

Click to see traceback logs
TracebackTests.test_exec_failure) ... ok


TracebackTests.test_syntax_error) ... ok


TracebackTests.test_broken_from) ... ok


TracebackTests.test_broken_submodule) ... ok


TracebackTests.test_broken_parent) ... ok


TracebackTests.test_unencodable_filename) ... ok


TracebackTests.test_exec_failure_nested) ... ok


TracebackTests.test_import_bug) ... ok


TracebackTests.test_broken_parent_from) ... ok


TracebackTests.test_nonexistent_module) ... ok


TracebackTests.test_nonexistent_module_nested) ... ok

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot PPC64LE Fedora Stable Refleaks 3.x (tier-2) has failed when building commit 2bef8ea.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/280/builds/1711) and take a look at the build logs.
  4. Check if the failure is related to this commit (2bef8ea) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/280/builds/1711

Failed tests:

  • test_idle
  • test_bdb
  • test_import
  • test_datetime
  • test_pyrepl
  • test_ast
  • test_zipfile
  • test_pyclbr
  • test.test_pydoc.test_pydoc

Test leaking resources:

  • test_ast: references
  • test_pydoc: memory blocks
  • test_bdb: references
  • test_pydoc: references
  • test_import: memory blocks
  • test_pyclbr: references
  • test_zipfile: references
  • test_idle: references
  • test_datetime: references
  • test_import: references
  • test_pyrepl: references

Summary of the results of the build (if available):

==

Click to see traceback logs
TracebackTests.test_exec_failure) ... ok


TracebackTests.test_syntax_error) ... ok


TracebackTests.test_broken_from) ... ok


TracebackTests.test_broken_submodule) ... ok


TracebackTests.test_broken_parent) ... ok


TracebackTests.test_unencodable_filename) ... ok


TracebackTests.test_exec_failure_nested) ... ok


TracebackTests.test_import_bug) ... ok


TracebackTests.test_broken_parent_from) ... ok


TracebackTests.test_nonexistent_module) ... ok


TracebackTests.test_nonexistent_module_nested) ... ok

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Windows11 Refleaks 3.x (tier-1) has failed when building commit 2bef8ea.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/920/builds/1438) and take a look at the build logs.
  4. Check if the failure is related to this commit (2bef8ea) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/920/builds/1438

Failed tests:

  • test_import
  • test_zipfile
  • test_multiprocessing_main_handling
  • test_idle

Test leaking resources:

  • test_import: references
  • test_import: memory blocks
  • test_zipfile: references
  • test_idle: references
  • test_multiprocessing_main_handling: memory blocks
  • test_multiprocessing_main_handling: references

Summary of the results of the build (if available):

==

Click to see traceback logs
TracebackTests.test_exec_failure) ... ok


TracebackTests.test_syntax_error) ... ok


TracebackTests.test_broken_from) ... ok


TracebackTests.test_broken_submodule) ... ok


TracebackTests.test_broken_parent) ... ok


TracebackTests.test_unencodable_filename) ... ok


TracebackTests.test_exec_failure_nested) ... ok


TracebackTests.test_import_bug) ... ok


TracebackTests.test_broken_parent_from) ... ok


TracebackTests.test_nonexistent_module) ... ok


TracebackTests.test_nonexistent_module_nested) ... ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants