Looking at various posts in the forum I see that it is recommended to do delete f but that’s in C++ . I am not sure how to do the equivalent in python. May be @etejedor can help here.
Hi @wiso, thank you, I can reproduce the same behaviour, also in C++ with the equivalent code:
for (int i = 0; i < 10; i++) {
for (auto &s : { "result_56159.471__seed_206498109__globals_obs_NONE.root", "result_56159.271__seed_1484912357__globals_obs_NONE.root" }) {
TFile f(s);
auto r = f.Get("fitResult"); // this is a RooFitResult
}
}
An explicit delete works in the C++ case, will check how to free the object from Python.
If yes, then easiest is to (temporarily) assign the ownership of r to Python, or set the _creates flag of Get if there are many more calls to it that all need deletion:
It seems that TFile.Get does not have that attribute:
>>> ROOT.TFile.Get._creates
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: 'builtin_function_or_method' object has no attribute '_creates'
GetObjectChecked does have _creates, but assigning it to True does not make a difference (the memory is still not freed):
TFile.GetObjectChecked._creates = True
for i in range(10):
for fn in glob('result*.root'):
f = ROOT.TFile.Open(fn)
r = f.GetObjectChecked("fitResult", "RooFitResult") # r is not freed
Ah, right, b/c Get is pythonized with a custom function, which doesn’t carry _creates. Can still replace it with a wrapper that uses ROOT.SetOwnership() before returning.
But if the setting of _creates for the return of GetObjectChecked() does not do the trick, then I don’t see how the delete can make the leak go away on the C++ side: if Python owns the object, then it’ll be explicitly deleted when the refcount becomes zero.
Aside, there is currently no way to check ownership in PyROOT (it’s the __python_owns__ property in cppyy master), but is trivial to add (see SetOwnership in RootModule.cxx).
Hello @etejedor, thank you it seems to work. Now I am able to open all the 400 files, but at the end the memory is aroud 1.5Gb (only opening file and reading the RooFitResult), with a linear increase. So maybe there is another smaller leak.
By the way: why you don’t simply make ROOT.SetOwnership(r, True) for all the object when using python? I think that many user will like that, to me it is much more natural.
B/c for many objects, it’s C++ that owns them, so that would lead to double deletes… Instead, a function that hands over ownership should be more clearly defined, e.g. by returning a unique_ptr or simply by the name (CreateXYZ, or CloneXYZ). Same goes for taking ownership of function arguments, but except for outliers like RooFit and graphics, ROOT is pretty consistent there: non-const pointers take ownership, rest doesn’t.