Skip to content

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

Merged
merged 1 commit into from
Jul 18, 2014

Conversation

tacaswell
Copy link
Member

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

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
@tacaswell tacaswell added this to the v1.4.0 milestone Jul 18, 2014
@tacaswell
Copy link
Member Author

@neothemachine Can you check if this fixes the problem?

@letmaik
Copy link
Contributor

letmaik commented Jul 18, 2014

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.

@letmaik
Copy link
Contributor

letmaik commented Jul 18, 2014

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?

@letmaik
Copy link
Contributor

letmaik commented Jul 18, 2014

Seems to be something else:

==19165== 188,736 (187,648 direct, 1,088 indirect) bytes in 5,864 blocks are definitely lost in loss record 2,253 of 2,307
==19165==    at 0x4C27AAA: malloc (vg_replace_malloc.c:291)
==19165==    by 0x1565342B: FT2Font::make_open_args(_object*, FT_Open_Args_*) (ft2font.cpp:2191)
==19165==    by 0x15667125: FT2Font::FT2Font(Py::PythonClassInstance*, Py::Tuple&, Py::Dict&) (ft2font.cpp:869)
==19165==    by 0x1566BC2C: Py::PythonClass<FT2Font>::extension_object_init(_object*, _object*, _object*) (ExtensionType.hxx:273)
==19165==    by 0x4ED9857: type_call (typeobject.c:745)
==19165==    by 0x4E769A2: PyObject_Call (abstract.c:2529)
==19165==    by 0x4F254CC: PyEval_EvalFrameEx (ceval.c:4239)
==19165==    by 0x4F27B7D: PyEval_EvalCodeEx (ceval.c:3253)
==19165==    by 0x4F25D29: PyEval_EvalFrameEx (ceval.c:4117)
==19165==    by 0x4F27B7D: PyEval_EvalCodeEx (ceval.c:3253)
==19165==    by 0x4F25D29: PyEval_EvalFrameEx (ceval.c:4117)
==19165==    by 0x4F27B7D: PyEval_EvalCodeEx (ceval.c:3253)

@tacaswell
Copy link
Member Author

@neothemachine I did not use objgraph, I only have so much bandwidth every night.

Created a new issue for the freetype leak #3276.

@letmaik letmaik mentioned this pull request Jul 18, 2014
@mdboom
Copy link
Member

mdboom commented Jul 18, 2014

Cool. Thanks for getting to the bottom of that!

mdboom added a commit that referenced this pull request Jul 18, 2014
BUG : fixes FontProperties memory leak
@mdboom mdboom merged commit 7d4ac9d into matplotlib:v1.4.x Jul 18, 2014
@tacaswell tacaswell deleted the fix_fontprop_leak branch July 18, 2014 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants