Skip to content
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

Enable test_memory #1077

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Enable test_memory #1077

wants to merge 5 commits into from

Conversation

slozier
Copy link
Contributor

@slozier slozier commented Dec 13, 2020

The issue is that the PythonType._subtypes list (which is used for the __subclasses__ method) is growing every time a new subclass is created and not getting cleaned up as types get GCd. This is not actually a regression with ipy2, if you swap the class definitions with new-style classes then it also fails there.

I'm not super happy with the perf impact of this solution but I'm not sure what else to do (other than adding a finalizer to PythonType). @BCSharp any other ideas?

Resolves #508

@BCSharp
Copy link
Member

BCSharp commented Dec 14, 2020

Yeah, I am not a big fan of using finalizers for stuff like this. They come with their own performance cost. But in this case, I assume creating new types happens far more frequently than getting them garbage collected, so I guess that using finalizers could be justified.

Another idea: keep the current _subtypes list cleanup in AddSubType, but instead of doing it every new type creation, do it only every 16 or so. Or when the list is about to expand its capacity. Or a combination of both, since both have some unique advantages. Just doing this would reduce the performance impact by an order of magnitude, without any serious memory penalty. I suppose most types will have a short list of user-defined subtypes anyway, with the exception of object of course.

@BCSharp
Copy link
Member

BCSharp commented Jun 23, 2022

Another additional trigger for the cleanup could be gc.collect (unless it is already happening somehow).

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.

test_memory fails intermittently
2 participants