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 leak from HepPDT::HeavyIonUnknownID #25298
Conversation
HepPDT::HeavyIonUnknownID creates a new particle in the case the HepPDT does not know of that particle by default. However, nothing deletes that particle. This code wraps HeavyIonUnknownID and does a thread safe cache of the particle in order to delete it later.
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25298/7318 |
A new Pull Request was created by @Dr15Jones (Chris Jones) for master. It involves the following packages: SimGeneral/HepPDTESSource @cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
@slava77 suggestions on how to find out what the RECO differences are in the comparison? |
please test |
Pull request #25298 was updated. @cmsbuild, @civanch, @mdhildreth can you please check and sign again. |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
@Dr15Jones , may be I do not fully understand but my interpretation of this solution: an unknown particle is created for an event, has proper unique ptr, so deleted at the end. At the same time, there is no attempt to reuse unknown particles created in previous events. |
@civanch the particle is reused, right after the ParticleDataTable (PDT) called this class, the PDT caches the result. Since the PDT is caching, there is no need for the code I wrote to also cache. |
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
@Dr15Jones , looks like new version of HepPDT has fixed the memory leak issue ( cms-sw/cmsdist#6548 (comment) ) . Should we revert this change in order to integrate HepPDT 3.04.01? |
Revert #25298; memory leak fix for HepPDT::HeavyIonUnknownID
HepPDT::HeavyIonUnknownID creates a new particle in the case the HepPDT does not know of that particle by default. However, nothing deletes that particle. This code wraps HeavyIonUnknownID and does a thread safe cache of the particle in order to delete it later.