-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
BUG : fixes FontProperties memory leak #3274
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
Conversation
According to the data model (https://docs.python.org/2/reference/datamodel.html#object.__hash__) objects that define __hash__ need to define __eq__ or __cmp__. By default, all user defined classes have a __cmp__ which evaluates to False for all other objects. Previously we do not define either __cmp__ or __eq__ on FontProperties, This results in never finding the property cached in the dict, hence the growth in the number of FontProperties (and I assume the stuff in them). By adding __eq__ and __ne__ we complete the data model and adding FontProperties to dictionaries should work as expected. This was not a problem before, but in matplotlib#3077 the caching key was changed from hash(prop) -> prop. Closes matplotlib#3264
@neothemachine Can you check if this fixes the problem? |
I will, but did you check it yourself with objgraph? It's quite easy, just invoke the show_growth function every few hundred drawing calls (using my test script) and see if the counts are zero. Maybe this could even be added to the test suite, but then the method would have to be slightly changed to not actually output stuff but return the numbers. |
Hmm, you seem to have fixed this one (checked with objgraph) but memory keeps increasing and it feels a lot like the previous real memory leak is back again. I saw there was a cleanup commit tacaswell@7b39e78 Could this have reintroduced the leak? |
Seems to be something else:
|
@neothemachine I did not use objgraph, I only have so much bandwidth every night. Created a new issue for the freetype leak #3276. |
Cool. Thanks for getting to the bottom of that! |
BUG : fixes FontProperties memory leak
According to the data
model (https://docs.python.org/2/reference/datamodel.html#object.__hash__)
objects that define hash need to define eq or cmp. By
default, all user defined classes have a cmp which evaluates to
False for all other objects.
Previously we do not define either cmp or eq on FontProperties,
This results in never finding the property
cached in the dict, hence the growth in the number of
FontProperties (and I assume the stuff in them).
By adding eq and ne we complete the data model and adding
FontProperties to dictionaries should work as expected.
This was not a problem before, but in #3077 the caching key was changed
from hash(prop) -> prop.
Closes #3264