Disable unused event branches to prevent memory leak

[cmargalejo]

Investigation started from rest-for-physics/detectorlib#124, where @AlvaroEzq reported the leak from #542 was still present after the destructor fix.

When a REST file stores multiple event types (e.g. Raw → Signal → Hits → Track), only the last one is actually used. But all event branches in the tree were left enabled. Each time GetEntry() reads the tree, ROOT creates internal objects for the unused branches that never get freed. This causes memory to grow. The fix consists on, after opening the file, disabling all event branches except the one being used.

Fixes #542 (hopefully...)

[AlvaroEzq]

Fixes #542 (hopefully...)

Yes! I tested these changes and they avoid the memory leak of #542.

When a REST file stores multiple event types (e.g. Raw → Signal → Hits → Track), only the last one is actually used.

But I am concerned this is not true. I use multiple event types stored in the same REST file and all of them can be used. For instance, the REST_ViewEvents.C macro can be used to visualize the different event types of the same event if they are stored in the same REST file. I tried doing exactly this and now it crashes 😢 :.

I think we should try to preserve this feature of storing multiple event types for both functionality and retrocompatibility. If we found it is not possible, then discuss which way to go.

[cmargalejo]

I have pushed a fix so that switching between event types in REST_ViewEvents works again with Raw, Hits and Track events.

However, switching to a TRestDetectorSignalEvent still crashes... I think it's a separate bug in that class. I'll open an issue. Its in rest-for-physics/detectorlib#125 .

[cmargalejo]

@AlvaroEzq, @lobis, @hgmaluenda, @juanangp , small ping on this PR. We want to get a new stable REST release ready, and this is one of the PRs still waiting for review. Could you have a look when you get a chance?

[Vindaar]

(Note: Below is my current understanding, I may still be missing something)

This PR does not actually address what is called a memory leak in issue #542 (and/or in issue rest-for-physics/detectorlib#125).

The fundamental issue at play is that if we read a REST ROOT file with current master and latest libraries that contains a TRestDetectorSignal from the old detectorlib with vector<Float_t> instead of vector<Double_t>, reading the file crashes with an Out of Memory error for the following reason.

REST currently does not include StreamerInfo meta data for ROOT to keep track of the data types stored in a ROOT tree. As such, when we read such a ROOT file without this info, ROOT will simply match the class definitions with their data types to the binary file and attempt to parse them. When it encounters the fields that it expects to be vector<Double_t> but that are vector<Float_t> in actuality, it reads junk data, because the required size on disk for a vector<Double_t> is larger than for a vector<Float_t>. While the internal ROOT details are not clear, this results in ROOT reading a bogus size for the vector, which then attempts to allocate a gigantic vector in order to parse the data into. This simply fails.

What this PR does is to change how REST ROOT files are read by default. Instead of trying to parse the entire ROOT tree based on the class definition, we only read a subset of the tree. Importantly by default we do not read the TRestDetectorSignal data. But this is not actually a fix for the problem, but only a workaround by hiding the actual problem. When one reads the TRestDetectorSignal data manually, the same crash occurs.

However, this PR might still be a useful addition, as it improves the I/O performance. In most cases, I assume, we don't actually need to read all information every time. We can revisit this once the actual bug this was supposed to fix is handled as part of other PRs. The important nuance though is that this addition is a breaking change to the previous behavior and either needs to be documented or only become an opt-in solution one can manually activate for performance (plus we better add some regression tests etc).