Skip to content

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Sep 1, 2025

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

@kumaraditya303
Copy link
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
Copy link
Member Author

picnixz commented Sep 1, 2025

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).

@kumaraditya303
Copy link
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
Copy link
Member Author

picnixz commented Sep 1, 2025

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).

@kumaraditya303
Copy link
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
Copy link
Member Author

picnixz commented Sep 1, 2025

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

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