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

gh-116946: fully implement GC protocol for `_thread._localdummy` by picnixz · Pull Request #138329 · python/cpython · GitHub

/ cpython Public

gh-116946: fully implement GC protocol for _thread._localdummy - #138329

Closed
picnixz wants to merge 1 commit into
python:mainfrom
picnixz:fix/gc/threading-heap-types-116946
Closed

gh-116946: fully implement GC protocol for _thread._localdummy#138329
picnixz wants to merge 1 commit into
python:mainfrom
picnixz:fix/gc/threading-heap-types-116946

Conversation

picnixz commented Sep 1, 2025
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

Did I correctly understand the fact that weakreflist should NOT be visited in the traverse function because the instance is not owning strong references to its elements here?

cc @kumaraditya303

Copy link
Copy Markdown
Contributor

Did I correctly understand the fact that weakreflist should NOT be visited in the traverse function because the instance is not owning strong references to its elements here?

Yes, only strong references needs to be traversed.

Also, why does _localdummy needs to be a gc type? It is an empty object so cannot be part of cycle, no?

picnixz commented Sep 1, 2025

Copy link
Copy Markdown
Member Author

Also, why does _localdummy needs to be a gc type? It is an empty object so cannot be part of cycle, no?

Even if it's "empty", the fact it's a heap type implies that it becomes a kind of "container" object as it has a reference to its type. At least, that's how the issue described the bug (and as the docs say). See also #87138 (comment).

Copy link
Copy Markdown
Contributor

Even if it's "empty", the fact it's a heap type implies that it becomes a kind of "container" object as it has a reference to its type. At least, that's how the issue described the bug (and as the docs say). See also

I agree, but the refcycle is only possible when the type is mutable then the cycle can be type -> some object -> instance -> type, if the type is immutable like this case then this isn't possible.

picnixz commented Sep 1, 2025

Copy link
Copy Markdown
Member Author

Hum. So there are a number of PRs I opened that are actually not necessary. Because if I'm following you on this one, then some commits were also not necessary in the past (some of them added GC support for empty immutable types).

Copy link
Copy Markdown
Contributor

Hum. So there are a number of PRs I opened that are actually not necessary. Because if I'm following you on this one, then some commits were also not necessary in the past (some of them added GC support for empty immutable types).

Yes, some of them in the past where done in cases where it wasn't needed but we should not do more of it, doing it increases pressure on GC which will slow down performance.

picnixz commented Sep 1, 2025

Copy link
Copy Markdown
Member Author

Ok, I will revert the PRs that touch immutable empty types. Sorry for the noise.

picnixz closed this Sep 1, 2025
picnixz deleted the fix/gc/threading-heap-types-116946 branch September 1, 2025 12:18
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
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


Back | FazBrowse Home | New Git URL