Looping on many files, memory leak (RooFitResult)

This seems to be similar to ROOT-9275, but not sure how to solve in my case

for fn in glob('result*.root'):
   f = ROOT.TFile.Open(fn)
   r = f.Get("fitResult")  # this is a RooFitResult

after ~400 files my job gets killed since it is using several Gb of memory

I tried with f.Close() and f.Clear(), nothing changed

Here a coupline of files as example:
result_56159.471__seed_206498109__globals_obs_NONE.root (366.1 KB)
result_56159.271__seed_1484912357__globals_obs_NONE.root (366.2 KB)

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 @wlso,

Can you share one or more of the files you are using so that I can try to reproduce? Is it enough with that code to see the behaviour you report?

I have attached a couple of files (my program get killed after the ~400th file) in the original post

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.

Explicit delete of r? I.e. the RooFitResult?

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:

oldcreates = TFile.Get._creates
TFile.Get._creates = True
# loop here
TFile.Get._creates = oldcreates

What that does, is assign ownership to Python of any object returned from Get.

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).

As suggested by @wlav, setting the ownership via ROOT.SetOwnership() prevents the leak. @wiso this is is how your code would look like:

for fn in glob('result*.root'):
    f = ROOT.TFile.Open(fn)
    r = f.Get("fitResult")
    ROOT.SetOwnership(r, True) # this makes Python delete r
1 Like

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.

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.