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

core::Engine: Fix memory leaks, misc #2

Merged
merged 2 commits into from Dec 30, 2013

Conversation

mckelvin
Copy link
Contributor

@mckelvin mckelvin commented Dec 6, 2013

this PR consists of 2 parts;

  1. fix compilation error while compiling using llvm(clang++ ) as reference to 'end' is ambiguous in llvm.
  2. fix memory leaks in core::Engine, the following code may be useful to prove that the memory leak do exists while reload DataFlow again and again:
import resource
from yaafelib import Engine, FeaturePlan

def main():
    fp = FeaturePlan()
    fp.addFeature("mfcc: MFCC blockSize=1024 stepSize=1024 CepsNbCoeffs=11")
    df = fp.getDataFlow()

    for i in range(10):
        for j in range(100):
            eng = Engine()
            eng.load(df)
        print i, resource.getrusage(resource.RUSAGE_SELF).ru_maxrss

if __name__ == '__main__':
    main()

the output is something like:

0 19640
1 22808
2 25712
3 28616
4 31784
5 34688
6 37592
7 40760
8 43664
9 46568

if no memory leaks happened, it should be something like:

0 19640
1 19640
2 19640
...

I've check and test the code. Please review it again!

BTW, I use the great tool valgrind to find the leak code.

@thomasfillon
Copy link
Member

@mckelvin : Thank you very much for this PR !

We will review it as soon as possible !

@mckelvin
Copy link
Contributor Author

Hi, @thomasfillon the PR has been pending for more than 3 weeks, I wonder if you have forgotten this. I've already merged this branch into my own repo's master branch. @seansay and I plan to add some more features into Yaafe to make it even better. Actually there are ready some heavy changes, I wonder if these changes are welcomed. ;)

thomasfillon added a commit that referenced this pull request Dec 30, 2013
core::Engine: Fix memory leaks, misc
@thomasfillon thomasfillon merged commit 6e473d5 into Yaafe:master Dec 30, 2013
@mckelvin mckelvin deleted the fix_memory_leaks branch December 30, 2013 08:27
@thomasfillon
Copy link
Member

OK the merge is done. Sorry for the delay and thank you again for this PR.
And by the way, the changes are very welcomed ;)

@mckelvin
Copy link
Contributor Author

It's not relevant with this pull, but forgive me, I don't where else to post.

https://github.com/Yaafe/Yaafe/blob/master/src_cpp/yaafe-python/yaafecore.cpp#L102 looks like a bug

@thomasfillon
Copy link
Member

Thank you very much. I will open an issue on that

@AM237
Copy link

AM237 commented Mar 13, 2014

Hi all, not sure if I'm missing something, but valgrind still reports ~5KB of leaked memory on a single (successful) run of yaafe-engine. Is this currently being tackled? Thanks!

@mckelvin
Copy link
Contributor Author

@AM237 #4 has fixed fix memleaks in: Smarc, MP3FileReader, freeOutputFormatDescription, but there should be some other memleaks including [here].(https://github.com/Yaafe/Yaafe/blob/master/src_cpp/yaafe-core/OutputFormat.cpp#L65 )

yomguy added a commit that referenced this pull request Jun 2, 2014
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

3 participants