Skip to content

gh-117783: Immortalize objects that use deferred reference counting - #118112

Merged
colesbury merged 6 commits into
python:mainfrom
colesbury:gh-117783-immortalize
Apr 29, 2024
Merged

gh-117783: Immortalize objects that use deferred reference counting#118112
colesbury merged 6 commits into
python:mainfrom
colesbury:gh-117783-immortalize

Conversation

@colesbury

@colesburycolesbury commented Apr 19, 2024

Copy link
Copy Markdown
Contributor

Deferred reference counting is not fully implemented yet. As a temporary measure, we immortalize objects that would use deferred reference counting to avoid multi-threaded scaling bottlenecks.

This is only performed in the free-threaded build once the first non-main thread is started. Additionally, some tests, including refleak tests, suppress this behavior.

…ting
Deferred reference counting is not fully implemented yet. As a temporary
measure, we immortalize objects that would use deferred reference
counting to avoid multi-threaded scaling bottlenecks.
This is only performed in the free-threaded build once the first
non-main thread is started. Additionally, some tests, including refleak
tests, suppress this behavior.
@colesburycolesbury added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 19, 2024
@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @colesbury for commit 0cab82c 🤖

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

@bedevere-botbedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 19, 2024
@colesbury
colesbury marked this pull request as ready for review April 19, 2024 21:37
@colesbury
colesbury requested a review from DinoVApril 19, 2024 21:37
@rhettinger
rhettinger removed their request for review April 21, 2024 19:11
Comment threadInclude/internal/pycore_gc.h Outdated
Comment threadPython/pylifecycle.c Outdated
// run much later than typical leading to attribute errors due to
// partially cleared modules. To avoid this, we copy the module dict
// if it was immortalized.
PyObject *copy = PyDict_Copy(mod->md_dict);

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.

Do we need to clear the original dict here too? Otherwise it seems like the immortal dict will just hang around and keep everything alive anyway?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

My intention was to keep the fields alive, but maybe that's not the best way to address this.

Basically, test_concurrent_futures.test_interpreter_shutdown was failing because a weakref was cleared later than usual, after the module dictionary was partially cleared. I was concerned that the problem might not be limited to that test, but I don't really have evidence for that one way or the other.

I'll get rid of this swapping and make the weakref callback more robust.

@DinoVDinoV 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.

LGTM!

@colesbury
colesbury merged commit 7ccacb2 into python:mainApr 29, 2024
@colesbury
colesbury deleted the gh-117783-immortalize branch April 29, 2024 18:36
Sign up for freeto 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

@colesbury@bedevere-bot@DinoV