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 in StFstClusterMaker #593

Merged
merged 43 commits into from
Oct 6, 2023
Merged

Fix memory leak in StFstClusterMaker #593

merged 43 commits into from
Oct 6, 2023

Conversation

techuan-huang
Copy link
Contributor

@techuan-huang techuan-huang commented Oct 1, 2023

Memory leaks were found by Gene in issue #590.
There are 3 places causing the leak in StFstClusterMaker/StFstScanRadiusClusterAlgo.cxx:

  1. Pointers assigned to std::vector in line 55 were not deleted before calling std::vector::pop_back() in line 69 and line 80.
  2. Object created in line 140 but only conditionally assigned to a std::vector in line 148.
  3. In line 203, std::vector::erase() was called without deleting the pointers.

After fixing these places, there is no memory leak from StFstClusterMaker reported by valgrind.

@techuan-huang techuan-huang changed the base branch from main to AgML_2_0 October 2, 2023 03:47
@techuan-huang techuan-huang changed the base branch from AgML_2_0 to main October 2, 2023 03:47
Copy link
Contributor

@genevb genevb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch has resolved the significant increase in reconstruction time per event over the course of processing files that I observed and documented when including FWD chain options. I'm very happy to see that!

@plexoos plexoos merged commit c1caf5e into star-bnl:main Oct 6, 2023
@techuan-huang techuan-huang deleted the FstMemLeak branch October 6, 2023 14:25
dkapukchyan pushed a commit to dkapukchyan/star-sw that referenced this pull request Mar 11, 2024
Memory leaks were found by Gene in issue star-bnl#590.
There are 3 places causing the leak in
`StFstClusterMaker/StFstScanRadiusClusterAlgo.cxx`:
1. Pointers assigned to std::vector in `line 55` were not deleted before
calling std::vector::pop_back() in `line 69` and `line 80`.
2. Object created in `line 140` but only conditionally assigned to a
std::vector in `line 148`.
3. In `line 203`, std::vector::erase() was called without deleting the
pointers.

After fixing these places, there is no memory leak from
StFstClusterMaker reported by valgrind.

---------

Co-authored-by: Dmitri Smirnov <[email protected]>
Co-authored-by: Te-Chuan Huang <[email protected]>
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.

4 participants