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

Fix memory leaks found by memleak_hawaii3.py #5359

Merged
merged 1 commit into from Oct 30, 2015

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Oct 30, 2015

Sorry I couldn't sort this one out before 1.5.0.

@WeatherGod
Copy link
Member

The changes here make sense. Are there other places where convert_to_string was (mis)-used?

@mdboom
Copy link
Member Author

mdboom commented Oct 30, 2015

No -- convert_to_string is only called from one place: Py_convert_to_string.

@pelson
Copy link
Member

pelson commented Oct 30, 2015

Do you want to target the v1.5.x branch?

@mdboom mdboom added this to the next point release (1.5.0) milestone Oct 30, 2015
@mdboom
Copy link
Member Author

mdboom commented Oct 30, 2015

Might as well target 1.5.x

EDIT: But we can just merge to master and backport after.

@mdboom mdboom modified the milestones: next point release (1.5.0), Next bugfix release (1.5.1) Oct 30, 2015
WeatherGod added a commit that referenced this pull request Oct 30, 2015
Fix memory leaks found by memleak_hawaii3.py
@WeatherGod WeatherGod merged commit 2e880f2 into matplotlib:master Oct 30, 2015
WeatherGod added a commit that referenced this pull request Oct 30, 2015
Fix memory leaks found by memleak_hawaii3.py
@WeatherGod
Copy link
Member

backported to v1.5.x as a450f99

@efiring
Copy link
Member

efiring commented Oct 30, 2015

@mdboom Thanks very much for tracking this down. Out of curiosity: do you have a simple explanation for why the leak was so much larger on Python 3 than 2?

@mdboom
Copy link
Member Author

mdboom commented Oct 30, 2015

Unfortunately, no. But I was seeing a great deal of variation in the result from the memleak_hawaii3.py script -- I think it was actually calculating the average increase incorrectly, so it may well be that the numbers were just bogus. (It was calculating the difference between the memory usage in the middle and the end and dividing by the steps. That only makes sense if the range is monotonically increasing. If it's jagged -- which it is -- you can get wildly incorrect numbers. See #5360).

@mdboom mdboom deleted the memleak-fixes branch November 10, 2015 02:46
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.

None yet

5 participants