[RF] Problem of memory leak (?) with RooDataSet #8984
Comments
|
Hi @marcescalier, I have assigned this to Jonas; he will take a look as soon as he can. :-) Cheers. |
|
Thank you. Good luck to Jonas if he finds time for the investigation. |
guitargeek
changed the title
|
Hello |
|
Hi, I'm glad you asked! I tried to reproduce this issue myself a few weeks ago actually, but I couldn't see the memory increase. Which ROOT version are you using? If this is still a problem for you with the current ROOT version, I will continue the investigation. |
|
Thank you. Which version of root do you use without having the problem ? Could you check that in "EXAMPLE Of PROBLEM", you really have a decrease of memory consumption ? I tried again (same problem) just after creation of dataset toy that uses previous toy per category -on a french cluster of computer, with root 6.26.04 : same problem |
|
To summarize : the problem is not a memory increase, but the fact that when we delete the RooDataSet, so there is an intrinsic memory leak of roofit : the allocated memory is not given back to the system when we delete the RooDataset. |
|
Okay! Then it's very likely related to the memory pool, which will be disabled in ROOT 6.28 as of this PR: Can you please try if you still see you problem with the ROOT nighlty build? |
|
I tried to setup a nightly version of root, following your suggestion : but this gives : and then the program does not work |
|
Oh well, I don't know what's messed up with the environment. You tried on lxplus? For me it works: And in fact, the increase in memory is gone, right? By the way, the |
|
ok : I don't know why root does not work for me (yes, I use lxplus). just after creation of dataset toy that uses previous toy per category You see that after deleting the dataset gives exactly the same memory as after having created the dataset, so there is a problem, isn't it ?!? (thank you for the information about the RooArgSet * deletion) |
I'm not convinced by this yet because I don't completely understand the output at this point. Yes, you are right, memory doesn't decrease, but starting from the third toy it also doesn't increase when But anyway, I managed to convince myself by generating many more toys, and observing a steady memory increase over time. I did this with a simplified version of your reproducer: void Minimum2()
{
using namespace RooFit;
using namespace RooStats;
ProcInfo_t procinfo;
const float toMB = 1.f / 1024.f;
std::unique_ptr<TFile> f_ws{TFile::Open("WS-YY-resonant_500_For_Comb.root", "READ")};
auto *ws = f_ws->Get<RooWorkspace>("combWS");
auto *mc = static_cast<ModelConfig *>(ws->obj("ModelConfig"));
auto *pdf = static_cast<RooSimultaneous *>(mc->GetPdf());
for (std::size_t index_toy = 1; index_toy < 500; index_toy++) {
const RooArgSet *Observables = (RooArgSet *)mc->GetObservables();
std::vector<std::unique_ptr<RooDataSet>> toyDatas;
std::map<string, RooDataSet *> toyDataMap;
RooCategory channellist{"channellist", "channellist"};
// generate each category
for (auto const &item : pdf->indexCat()) {
channellist.defineType(item.first.c_str());
RooAbsPdf *pdftmp = pdf->getPdf(item.first.c_str());
RooArgSet obstmp;
pdftmp->getObservables(Observables, obstmp);
toyDatas.emplace_back(static_cast<RooDataSet *>(pdftmp->generate(obstmp, Extended())));
toyDataMap[item.first.c_str()] = toyDatas.back().get();
}
RooRealVar wt("wt", "wt", 1);
RooDataSet toyData{"toyData", "", {*Observables, wt}, Index(channellist), Import(toyDataMap), WeightVar(wt)};
if (index_toy % 10 == 0) {
gSystem->GetProcInfo(&procinfo);
std::cout << index_toy << ": " << procinfo.fMemResident * toMB << " " << procinfo.fMemVirtual * toMB
<< std::endl;
}
}
}The output is now: I will continue debugging this, starting from there. |
|
Thank you Jonas. just before generate We see that after generate, the memory has increased (which is normal), but afterwards, even when deleting the generated dataset, we no more get back to the level of memory memory that was before the generate, even though we delete the generated dataset. |
|
Yes yes I see the point for the first toy, but for the others I just didn't understand it. Anyhow I'm convinced that there is a problem, and just to record my progress for today, here is a standalone reproducer that doesn't depend on Marcs workspace: #include <RooGenContext.h>
#include <RooRealVar.h>
#include <RooExponential.h>
#include <RooUniform.h>
#include <RooMsgService.h>
#include <TFile.h>
#include <TSystem.h>
#include <iostream>
void repro()
{
using namespace RooFit;
ProcInfo_t procinfo;
const float toMB = 1.f / 1024.f;
RooRealVar x{"x", "x", 0.1, 5.1};
RooRealVar c{"c", "c", -1.8, -5, 5};
RooExponential expo{"expo", "expo", x, c};
RooAbsPdf *pdf = &expo;
// With the uniform distribution, there is no leak!
// RooUniform uni{"uni", "uni", x};
// RooAbsPdf * pdf = &uni;
std::size_t nToys = 5000;
for (std::size_t index_toy = 1; index_toy <= nToys; index_toy++) {
if (index_toy % 500 == 0) {
gSystem->GetProcInfo(&procinfo);
std::cout << index_toy << ": " << procinfo.fMemResident * toMB << " " << procinfo.fMemVirtual * toMB
<< std::endl;
}
if (index_toy == nToys) {
RooMsgService::instance().addStream(DEBUG, Topic(Generation));
}
RooGenContext{*pdf, x};
}
}The output: |
|
I also increased the priority, because having a leak in such a fundamental operation with RooFit is not so great... |
|
Thanks a lot Jonas for your investigation, and for realizing that there is really something. |
This fixes a memory leak related to the usage of `TRef` in `TFoamCell`
that can be reproduced with the following code:
```C++
class MyTFoamBinding : public TFoamIntegrand {
public:
double Density(Int_t ndim, double *) override { return 1.0; }
};
void reproTFoam()
{
ProcInfo_t procinfo;
const float toMB = 1.f / 1024.f;
std::size_t nToys = 5000;
TRandom3 rand{};
for (std::size_t index_toy = 1; index_toy <= nToys; index_toy++) {
if (index_toy % 500 == 0) {
gSystem->GetProcInfo(&procinfo);
std::cout << index_toy << ": " << procinfo.fMemResident * toMB
<< " " << procinfo.fMemVirtual * toMB << std::endl;
}
MyTFoamBinding binding{};
TFoam tfoam{"TFOAM"};
tfoam.SetkDim(1);
tfoam.SetRho(&binding);
tfoam.SetPseRan(&rand);
tfoam.SetnCells(30);
tfoam.SetnSampl(200);
tfoam.SetPseRan(&rand);
tfoam.SetChat(0);
tfoam.Initialize();
}
}
```
Before this commit, you will see a steady memory increase that is gone
after this commit.
This fixes the fundamental memory increase in RooFit toy studies,
reported for example in root-project#8984.
Ater the last commit that excludes the TFoam classes from IO, this
commit is not a problem to the funcitonality because TFoam can't do IO
anyway, and no persistent reference with `TRef` are needed.
This fixes a memory leak related to the usage of `TRef` in `TFoamCell`
that can be reproduced with the following code:
```C++
class MyTFoamBinding : public TFoamIntegrand {
public:
double Density(Int_t ndim, double *) override { return 1.0; }
};
void reproTFoam()
{
ProcInfo_t procinfo;
const float toMB = 1.f / 1024.f;
std::size_t nToys = 5000;
TRandom3 rand{};
for (std::size_t index_toy = 1; index_toy <= nToys; index_toy++) {
if (index_toy % 500 == 0) {
gSystem->GetProcInfo(&procinfo);
std::cout << index_toy << ": " << procinfo.fMemResident * toMB
<< " " << procinfo.fMemVirtual * toMB << std::endl;
}
MyTFoamBinding binding{};
TFoam tfoam{"TFOAM"};
tfoam.SetkDim(1);
tfoam.SetRho(&binding);
tfoam.SetPseRan(&rand);
tfoam.SetnCells(30);
tfoam.SetnSampl(200);
tfoam.SetPseRan(&rand);
tfoam.SetChat(0);
tfoam.Initialize();
}
}
```
Before this commit, you will see a steady memory increase that is gone
after this commit.
This fixes the fundamental memory increase in RooFit toy studies,
reported for example in root-project#8984.
Ater the last commit that excludes the TFoam classes from IO, this
commit is not a problem to the funcitonality because TFoam can't do IO
anyway, and no persistent reference with `TRef` are needed.
This fixes a memory leak related to the usage of `TRef` in `TFoamCell`
that can be reproduced with the following code:
```C++
class MyTFoamBinding : public TFoamIntegrand {
public:
double Density(Int_t ndim, double *) override { return 1.0; }
};
void reproTFoam()
{
ProcInfo_t procinfo;
const float toMB = 1.f / 1024.f;
std::size_t nToys = 5000;
TRandom3 rand{};
for (std::size_t index_toy = 1; index_toy <= nToys; index_toy++) {
if (index_toy % 500 == 0) {
gSystem->GetProcInfo(&procinfo);
std::cout << index_toy << ": " << procinfo.fMemResident * toMB
<< " " << procinfo.fMemVirtual * toMB << std::endl;
}
MyTFoamBinding binding{};
TFoam tfoam{"TFOAM"};
tfoam.SetkDim(1);
tfoam.SetRho(&binding);
tfoam.SetPseRan(&rand);
tfoam.SetnCells(30);
tfoam.SetnSampl(200);
tfoam.SetPseRan(&rand);
tfoam.SetChat(0);
tfoam.Initialize();
}
}
```
Before this commit, you will see a steady memory increase that is gone
after this commit.
This fixes the fundamental memory increase in RooFit toy studies,
reported for example in root-project#8984.
After the last commit that excludes the TFoam classes from IO, this
commit is not a problem to the funcitonality because TFoam can't do IO
anyway, and no persistent reference with `TRef` are needed.
This fixes a memory leak related to the usage of `TRef` in `TFoamCell`
that can be reproduced with the following code:
```C++
class MyTFoamBinding : public TFoamIntegrand {
public:
double Density(Int_t ndim, double *) override { return 1.0; }
};
void reproTFoam()
{
ProcInfo_t procinfo;
const float toMB = 1.f / 1024.f;
std::size_t nToys = 5000;
TRandom3 rand{};
for (std::size_t index_toy = 1; index_toy <= nToys; index_toy++) {
if (index_toy % 500 == 0) {
gSystem->GetProcInfo(&procinfo);
std::cout << index_toy << ": " << procinfo.fMemResident * toMB
<< " " << procinfo.fMemVirtual * toMB << std::endl;
}
MyTFoamBinding binding{};
TFoam tfoam{"TFOAM"};
tfoam.SetkDim(1);
tfoam.SetRho(&binding);
tfoam.SetPseRan(&rand);
tfoam.SetnCells(30);
tfoam.SetnSampl(200);
tfoam.SetPseRan(&rand);
tfoam.SetChat(0);
tfoam.Initialize();
}
}
```
Before this commit, you will see a steady memory increase that is gone
after this commit.
This fixes the fundamental memory increase in RooFit toy studies,
reported for example in root-project#8984.
This fixes a memory leak related to the usage of `TRef` in `TFoamCell`
that can be reproduced with the following code:
```C++
class MyTFoamBinding : public TFoamIntegrand {
public:
double Density(Int_t ndim, double *) override { return 1.0; }
};
void reproTFoam()
{
ProcInfo_t procinfo;
const float toMB = 1.f / 1024.f;
std::size_t nToys = 5000;
TRandom3 rand{};
for (std::size_t index_toy = 1; index_toy <= nToys; index_toy++) {
if (index_toy % 500 == 0) {
gSystem->GetProcInfo(&procinfo);
std::cout << index_toy << ": " << procinfo.fMemResident * toMB
<< " " << procinfo.fMemVirtual * toMB << std::endl;
}
MyTFoamBinding binding{};
TFoam tfoam{"TFOAM"};
tfoam.SetkDim(1);
tfoam.SetRho(&binding);
tfoam.SetPseRan(&rand);
tfoam.SetnCells(30);
tfoam.SetnSampl(200);
tfoam.SetPseRan(&rand);
tfoam.SetChat(0);
tfoam.Initialize();
}
}
```
Before this commit, you will see a steady memory increase that is gone
after this commit.
This fixes the fundamental memory increase in RooFit toy studies,
reported for example in root-project#8984.
This fixes a memory leak related to the usage of `TRef` in `TFoamCell`
that can be reproduced with the following code:
```C++
class MyTFoamBinding : public TFoamIntegrand {
public:
double Density(Int_t ndim, double *) override { return 1.0; }
};
void reproTFoam()
{
ProcInfo_t procinfo;
const float toMB = 1.f / 1024.f;
std::size_t nToys = 5000;
TRandom3 rand{};
for (std::size_t index_toy = 1; index_toy <= nToys; index_toy++) {
if (index_toy % 500 == 0) {
gSystem->GetProcInfo(&procinfo);
std::cout << index_toy << ": " << procinfo.fMemResident * toMB
<< " " << procinfo.fMemVirtual * toMB << std::endl;
}
MyTFoamBinding binding{};
TFoam tfoam{"TFOAM"};
tfoam.SetkDim(1);
tfoam.SetRho(&binding);
tfoam.SetPseRan(&rand);
tfoam.SetnCells(30);
tfoam.SetnSampl(200);
tfoam.SetPseRan(&rand);
tfoam.SetChat(0);
tfoam.Initialize();
}
}
```
Before this commit, you will see a steady memory increase that is gone
after this commit.
This fixes the fundamental memory increase in RooFit toy studies,
reported for example in root-project#8984.
This fixes a memory leak related to the usage of `TRef` in `TFoamCell`
that can be reproduced with the following code:
```C++
class MyTFoamBinding : public TFoamIntegrand {
public:
double Density(Int_t ndim, double *) override { return 1.0; }
};
void reproTFoam()
{
ProcInfo_t procinfo;
const float toMB = 1.f / 1024.f;
std::size_t nToys = 5000;
TRandom3 rand{};
for (std::size_t index_toy = 1; index_toy <= nToys; index_toy++) {
if (index_toy % 500 == 0) {
gSystem->GetProcInfo(&procinfo);
std::cout << index_toy << ": " << procinfo.fMemResident * toMB
<< " " << procinfo.fMemVirtual * toMB << std::endl;
}
MyTFoamBinding binding{};
TFoam tfoam{"TFOAM"};
tfoam.SetkDim(1);
tfoam.SetRho(&binding);
tfoam.SetPseRan(&rand);
tfoam.SetnCells(30);
tfoam.SetnSampl(200);
tfoam.SetPseRan(&rand);
tfoam.SetChat(0);
tfoam.Initialize();
}
}
```
Before this commit, you will see a steady memory increase that is gone
after this commit.
This fixes the fundamental memory increase in RooFit toy studies,
reported for example in root-project#8984.
This fixes a memory leak related to the usage of `TRef` in `TFoamCell`
that can be reproduced with the following code:
```C++
class MyTFoamBinding : public TFoamIntegrand {
public:
double Density(Int_t ndim, double *) override { return 1.0; }
};
void reproTFoam()
{
ProcInfo_t procinfo;
const float toMB = 1.f / 1024.f;
std::size_t nToys = 5000;
TRandom3 rand{};
for (std::size_t index_toy = 1; index_toy <= nToys; index_toy++) {
if (index_toy % 500 == 0) {
gSystem->GetProcInfo(&procinfo);
std::cout << index_toy << ": " << procinfo.fMemResident * toMB
<< " " << procinfo.fMemVirtual * toMB << std::endl;
}
MyTFoamBinding binding{};
TFoam tfoam{"TFOAM"};
tfoam.SetkDim(1);
tfoam.SetRho(&binding);
tfoam.SetPseRan(&rand);
tfoam.SetnCells(30);
tfoam.SetnSampl(200);
tfoam.SetPseRan(&rand);
tfoam.SetChat(0);
tfoam.Initialize();
}
}
```
Before this commit, you will see a steady memory increase that is gone
after this commit.
This fixes the fundamental memory increase in RooFit toy studies,
reported for example in #8984.
This fixes a memory leak related to the usage of `TRef` in `TFoamCell`
that can be reproduced with the following code:
```C++
class MyTFoamBinding : public TFoamIntegrand {
public:
double Density(Int_t ndim, double *) override { return 1.0; }
};
void reproTFoam()
{
ProcInfo_t procinfo;
const float toMB = 1.f / 1024.f;
std::size_t nToys = 5000;
TRandom3 rand{};
for (std::size_t index_toy = 1; index_toy <= nToys; index_toy++) {
if (index_toy % 500 == 0) {
gSystem->GetProcInfo(&procinfo);
std::cout << index_toy << ": " << procinfo.fMemResident * toMB
<< " " << procinfo.fMemVirtual * toMB << std::endl;
}
MyTFoamBinding binding{};
TFoam tfoam{"TFOAM"};
tfoam.SetkDim(1);
tfoam.SetRho(&binding);
tfoam.SetPseRan(&rand);
tfoam.SetnCells(30);
tfoam.SetnSampl(200);
tfoam.SetPseRan(&rand);
tfoam.SetChat(0);
tfoam.Initialize();
}
}
```
Before this commit, you will see a steady memory increase that is gone
after this commit.
This fixes the fundamental memory increase in RooFit toy studies,
reported for example in root-project#8984.
This fixes a memory leak related to the usage of `TRef` in `TFoamCell`
that can be reproduced with the following code:
```C++
class MyTFoamBinding : public TFoamIntegrand {
public:
double Density(Int_t ndim, double *) override { return 1.0; }
};
void reproTFoam()
{
ProcInfo_t procinfo;
const float toMB = 1.f / 1024.f;
std::size_t nToys = 5000;
TRandom3 rand{};
for (std::size_t index_toy = 1; index_toy <= nToys; index_toy++) {
if (index_toy % 500 == 0) {
gSystem->GetProcInfo(&procinfo);
std::cout << index_toy << ": " << procinfo.fMemResident * toMB
<< " " << procinfo.fMemVirtual * toMB << std::endl;
}
MyTFoamBinding binding{};
TFoam tfoam{"TFOAM"};
tfoam.SetkDim(1);
tfoam.SetRho(&binding);
tfoam.SetPseRan(&rand);
tfoam.SetnCells(30);
tfoam.SetnSampl(200);
tfoam.SetPseRan(&rand);
tfoam.SetChat(0);
tfoam.Initialize();
}
}
```
Before this commit, you will see a steady memory increase that is gone
after this commit.
This fixes the fundamental memory increase in RooFit toy studies,
reported for example in #8984.
|
Thank you @guitargeek . Would you know in which root version I can check that it works ? |
|
In the 6.28.00 release that will be released in a week or so. Sorry I didn't mean to close the issue, GitHub did that automatically! I'll leave this open until you confirm that the leak is gone. If you manage to source them correctly, you can also try it out with the ROOT nightly builds: |
|
Removing the 6.28 milestone because it should be already fixed in ROOT 6.28. This issue is only left open because the validation of the fix is still pending |
|
Thanks. I will test it in a few days. Apologizes for the delay. |
|
So unfortunately, I have no way to check with the nightly (apart if I would change/redevelop a new program). The reason is that the workspace used in my example includes RooTwoSidedCBShape, which is unknown to root, so one needs to switch on first a tool that allows to access to RooTwoSidedCBShape. Such tools are not available with recent root versions. With root 6.29.01, this gives : [escalier@lxplus745 /afs/cern.ch/work/e/escalier/public/ForOthers/ProblemMemory] root -b -q "Minimum.C+()" |tee log_6.29.01| Welcome to ROOT 6.29/01 https://root.cern |
|
This fixes a memory leak related to the usage of `TRef` in `TFoamCell`
that can be reproduced with the following code:
```C++
class MyTFoamBinding : public TFoamIntegrand {
public:
double Density(Int_t ndim, double *) override { return 1.0; }
};
void reproTFoam()
{
ProcInfo_t procinfo;
const float toMB = 1.f / 1024.f;
std::size_t nToys = 5000;
TRandom3 rand{};
for (std::size_t index_toy = 1; index_toy <= nToys; index_toy++) {
if (index_toy % 500 == 0) {
gSystem->GetProcInfo(&procinfo);
std::cout << index_toy << ": " << procinfo.fMemResident * toMB
<< " " << procinfo.fMemVirtual * toMB << std::endl;
}
MyTFoamBinding binding{};
TFoam tfoam{"TFOAM"};
tfoam.SetkDim(1);
tfoam.SetRho(&binding);
tfoam.SetPseRan(&rand);
tfoam.SetnCells(30);
tfoam.SetnSampl(200);
tfoam.SetPseRan(&rand);
tfoam.SetChat(0);
tfoam.Initialize();
}
}
```
Before this commit, you will see a steady memory increase that is gone
after this commit.
This fixes the fundamental memory increase in RooFit toy studies,
reported for example in root-project#8984.
|
Okay, then I close this issue now! Feel free to open a new one if you see problems again. |
|
Hi @guitargeek, It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise. Sincerely, |
Describe the bug
I open a workspace (RooWorkspace) that contains a pdf and various parameters. From these I do a loop on pseudo-experiments ("toys") that generate a RooDataSet.
For each toy in the loop, after generation, I delete the RooDataset.
But the memory, as reported by the class ProcInfo_t, never decreases.
It means that either ProcInfo_t is not actually showing the current memory information, or that is shows the "integrated memory used since the beginning",
or that RooDataSet would have a memory leak ?
or that I have a bug in my program ?
I provide a minimum reproducible example.
See for example : "EXAMPLE OF PROBLEM"
The minimum program is here :
/afs/cern.ch/work/e/escalier/public/ForOthers/ProblemMemory
Expected behavior
the expected behaviour would be that when I delete the RooDataSet, the memory would reduce.
To Reproduce
The README shows the quick steps for allowing to read the root file
Setup
I used root 6.18.00 to reproduce the problem on a french cluster of computer
On lxplus, I reproduce the problem, with root 6.20.02
Additional context
The text was updated successfully, but these errors were encountered: