Form fix ttree write threading#476
Conversation
the right TFile. Still contains cout statements for instrumentation to demonstrate that this solves the problem even when weird things happen to gDirectory.
branch solves a problem with file I/O.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #476 +/- ##
==========================================
- Coverage 85.51% 85.43% -0.09%
==========================================
Files 144 144
Lines 3591 3591
Branches 615 615
==========================================
- Hits 3071 3068 -3
- Misses 314 315 +1
- Partials 206 208 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| { | ||
| if (m_tree != nullptr) { | ||
| m_tree->Write(); | ||
| m_tree->GetDirectory()->WriteTObject(m_tree); |
There was a problem hiding this comment.
| m_tree->GetDirectory()->WriteTObject(m_tree); | |
| m_tree->Write(); |
It should already be doing what (and a few more thing) the PR does. (On the other the SetDirectory is probably necessary to ensure that the TTree ends up in the right file).
| m_tree = new TTree(name().c_str(), name().c_str()); | ||
| m_tree->SetDirectory(m_tfile.get()); |
There was a problem hiding this comment.
The fix is unlikely to solve the underlying problem.
If things are set properly (i.e. Thread Safety is enabled), the 2 codes have the same effective behavior.
TContext set gDirectory to m_tfile. If Thread Safety is enabled gDirectory is thread local and thus can not be influence by other threads.
If the new code is indeed more robust than the older code, it would (most likely) mean that Thread Safety is not enabled ... and if it not enabled all of ROOT/Core/Meta is not thread safe and will fail in many circumstances.
If Thread Safely is already enabled, then we have a deeper mystery to investigate.
There was a problem hiding this comment.
I think that must be it. Thread safety is probably not enabled. FORM doesn't do it to my knowledge, and I think we're the only component using ROOT. Thank you!
There was a problem hiding this comment.
Note that there is no harm in enabling it multiple time, so indeed FORM must enabled it explicitly.
5 participants
Fixes sporadic failure to write TTree to a file when using multiple threads. I failed to fix this the first time because the problem did not trigger consistently. We think there might be a data race involved.