Fixed memory leak in pydirac #10

Merged
merged 2 commits into from Nov 1, 2012

Conversation

Projects
None yet
3 participants
@psobot
Contributor

psobot commented Oct 31, 2012

I think I've found a pretty substantial memory leak in pydirac - a temporary array being alloc'd doesn't get Py_DECREF'd, which causes Python to hold onto the reference forever, essentially leaking the entire temporary buffer. (As pydirac is usually used on very large arrays... this leads to many-dozens-of-megabytes being lost every call.)

Memory usage without this patch:
Increasing forever!

...and with the patch:
Constant!


Test program to generate the above Instruments graphs:

import audio
import dirac

a = audio.LocalAudioFile('../wubmachine/tests/mario.mp3')
for _ in xrange(0, 1000):
    dirac.timeScale(a[0:300000].data, 1.4, 44100, 0)
Fixed memory leak in dirac
Temporary view of `insound` wasn't being DECREF'd, so it hung around
and never got cleaned up by the Python GC.
@bwhitman

This comment has been minimized.

Show comment
Hide comment
@bwhitman

bwhitman Oct 31, 2012

Contributor

Those graphs don't lie. Amazing. I'll check this out in a bit. Thanks for the catch.

On Oct 31, 2012, at 5:06 PM, Peter Sobot notifications@github.com wrote:

I think I've found a pretty substantial memory leak in pydirac - a temporary array being alloc'd doesn't get Py_DECREF'd, which causes Python to hold onto the reference forever, essentially leaking the entire temporary buffer. (As pydirac is usually used on very large arrays... this leads to many-dozens-of-megabytes being lost every call.)

Memory usage without this patch:

...and with the patch:

Test program to generate the above Instruments graphs:

import audio
import dirac

a = audio.LocalAudioFile('../wubmachine/tests/mario.mp3')
for _ in xrange(0, 1000):
dirac.timeScale(a[0:300000].data, 1.4, 44100, 0)
You can merge this Pull Request by running:

git pull https://github.com/psobot/remix patch-2
Or view, comment on, or merge it at:

#10

Commit Summary

Fixed memory leak in dirac
File Changes

M external/pydirac225/diracmodule.cpp (3)
Patch Links

https://github.com/echonest/remix/pull/10.patch
https://github.com/echonest/remix/pull/10.diff

Reply to this email directly or view it on GitHub.

Contributor

bwhitman commented Oct 31, 2012

Those graphs don't lie. Amazing. I'll check this out in a bit. Thanks for the catch.

On Oct 31, 2012, at 5:06 PM, Peter Sobot notifications@github.com wrote:

I think I've found a pretty substantial memory leak in pydirac - a temporary array being alloc'd doesn't get Py_DECREF'd, which causes Python to hold onto the reference forever, essentially leaking the entire temporary buffer. (As pydirac is usually used on very large arrays... this leads to many-dozens-of-megabytes being lost every call.)

Memory usage without this patch:

...and with the patch:

Test program to generate the above Instruments graphs:

import audio
import dirac

a = audio.LocalAudioFile('../wubmachine/tests/mario.mp3')
for _ in xrange(0, 1000):
dirac.timeScale(a[0:300000].data, 1.4, 44100, 0)
You can merge this Pull Request by running:

git pull https://github.com/psobot/remix patch-2
Or view, comment on, or merge it at:

#10

Commit Summary

Fixed memory leak in dirac
File Changes

M external/pydirac225/diracmodule.cpp (3)
Patch Links

https://github.com/echonest/remix/pull/10.patch
https://github.com/echonest/remix/pull/10.diff

Reply to this email directly or view it on GitHub.

@psobot

This comment has been minimized.

Show comment
Hide comment
@psobot

psobot Nov 1, 2012

Contributor

Found a leak in echonest.action.Crossfade too:

Pre-patch:
Old

...and post-patch:
New


Test case to reproduce the above graphs:

import echonest.audio
import echonest.action

a = echonest.audio.LocalAudioFile('../wubmachine/tests/mario.mp3')
for _ in xrange(0, 100000):
    echonest.action.Crossfade([a, a], [5, 10], 5).render()
Contributor

psobot commented Nov 1, 2012

Found a leak in echonest.action.Crossfade too:

Pre-patch:
Old

...and post-patch:
New


Test case to reproduce the above graphs:

import echonest.audio
import echonest.action

a = echonest.audio.LocalAudioFile('../wubmachine/tests/mario.mp3')
for _ in xrange(0, 100000):
    echonest.action.Crossfade([a, a], [5, 10], 5).render()
@tjehan

This comment has been minimized.

Show comment
Hide comment
@tjehan

tjehan Nov 1, 2012

Member

Awesome peter! These would be mine... oops!

Member

tjehan commented Nov 1, 2012

Awesome peter! These would be mine... oops!

bwhitman pushed a commit that referenced this pull request Nov 1, 2012

Brian Whitman
Merge pull request #10 from psobot/patch-2
Fixed memory leak in pydirac & crossfade

@bwhitman bwhitman merged commit 31c3739 into echonest:master Nov 1, 2012

@bwhitman

This comment has been minimized.

Show comment
Hide comment
@bwhitman

bwhitman Nov 1, 2012

Contributor

thanks peter!!

Contributor

bwhitman commented Nov 1, 2012

thanks peter!!

srubin pushed a commit to srubin/remix that referenced this pull request Dec 5, 2014

Brian Whitman
Merge pull request #10 from psobot/patch-2
Fixed memory leak in pydirac & crossfade

srubin pushed a commit to srubin/remix that referenced this pull request Dec 5, 2014

Brian Whitman
Merge pull request #10 from psobot/patch-2
Fixed memory leak in pydirac & crossfade
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment