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 leak #167

Merged
merged 1 commit into from Feb 19, 2020
Merged

Fix memory leak #167

merged 1 commit into from Feb 19, 2020

Conversation

@rcurtin
Copy link
Member

rcurtin commented Feb 17, 2020

While using valgrind I noticed that the instDecayPolicy and instUpdatePolicy used by SGD actually aren't properly deallocated. So this adds destructors to any optimizer that uses an Any internally, and that destructor calls Clear() to correctly free any allocated memory.

There are a few PRs now that have been merged since the last release, so I think we can do another release after this is merged.

@birm

This comment has been minimized.

Copy link
Member

birm commented Feb 17, 2020

I wonder what it would take to add the jenkins memory checker from mlpack to ensmallen too.

@zoq

This comment has been minimized.

Copy link
Member

zoq commented Feb 18, 2020

I wonder what it would take to add the jenkins memory checker from mlpack to ensmallen too.

Should be simple, maybe we could even run against the complete test suite instead of single test, as we do for mlpack.

@zoq
zoq approved these changes Feb 18, 2020
Copy link
Member

zoq left a comment

Looks good to me.

@birm
birm approved these changes Feb 18, 2020
@rcurtin rcurtin merged commit 4cf6625 into mlpack:master Feb 19, 2020
4 checks passed
4 checks passed
Static Code Analysis Checks Build finished.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
mlpack master build test Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.