[Root7] A path to reducing global memory management.#18083
Draft
hageboeck wants to merge 12 commits intoroot-project:masterfrom
Draft
[Root7] A path to reducing global memory management.#18083hageboeck wants to merge 12 commits intoroot-project:masterfrom
hageboeck wants to merge 12 commits intoroot-project:masterfrom
Conversation
hageboeck
force-pushed
the
root7-memoryManagement
branch
from
89bd358 to
4d29c60
Compare
pcanal
reviewed
Mar 20, 2025
Test Results 22 files 22 suites 3d 7h 51m 54s ⏱️ Results for commit 0587de2. ♻️ This comment has been updated with latest results. |
silverweed
reviewed
Mar 21, 2025
hageboeck
force-pushed
the
root7-memoryManagement
branch
from
4d29c60 to
1224a4b
Compare
hageboeck
force-pushed
the
root7-memoryManagement
branch
from
b894456 to
167c0cc
Compare
hageboeck
force-pushed
the
root7-memoryManagement
branch
from
342a3cb to
0ca4f8a
Compare
hageboeck
force-pushed
the
root7-memoryManagement
branch
from
0ca4f8a to
68de9c4
Compare
1 task
hageboeck
force-pushed
the
root7-memoryManagement
branch
from
68de9c4 to
9c5ce87
Compare
hageboeck
force-pushed
the
root7-memoryManagement
branch
from
9c5ce87 to
b79032e
Compare
hageboeck
force-pushed
the
root7-memoryManagement
branch
2 times, most recently
from
74a6279 to
8d9511a
Compare
hageboeck
force-pushed
the
root7-memoryManagement
branch
2 times, most recently
from
0b52df2 to
c9f787b
Compare
hageboeck
force-pushed
the
root7-memoryManagement
branch
3 times, most recently
from
d354941 to
3878984
Compare
Replace uses of TH1::AddDirectory by TDirectory::TContext. It is thread-local and uses the RAII idiom, so state changes cannot inadvertently leak out of TMVA functions.
…ect. It makes sense when a different object with the same name is registered (the old object will leak), but if the object is identical, the replacement is like a no-op.
In order for TTree::Draw to work as documented, histograms must be registered to gDirectory. Therefore, make it independent of TH1::AddDirectory and DisableImplicitObjectOwnership. Histograms will unconditionally be registered.
- Don't rely on implicit registration to directories. - Make the test insensitive to any defaults by always adding the histogram to the file.
The ROOT 7 ownership settings don't affect TH1::AddDirectoryStatus, so objects read from a TFile associate themselves with that file. Auto registration "off" affects whether new objects are associated to gDirectory so the documentation needed to be updated. TH1::AddDirectoryStatus has the same effect as before (affecting both new objects and objects read from file), and RFile undoes or avoids any potential registration.
When the implicit ownership is switched to ROOT 7 mode, the corresponding notification needs to be masked from output diffs and/or the RootTestDriver checks.
- Add Enable/DisableObjectAutoRegistration() in ROOT::Experimental. - Add a function to test if registration is on or off. - Allow for setting defaults using - The environment variable ROOT_OBJECT_AUTO_REGISTRATION - The rootrc variable Root.ObjectAutoRegistration
…ph2D. In locations where TH1::AddDirectoryStatus is queried, also query ObjectAutoRegistrationEnabled(). This extends the ROOT 7 testing mode to TH1 and derived classes, TGraph2D and TEfficiency.
hageboeck
force-pushed
the
root7-memoryManagement
branch
from
3878984 to
0587de2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a longer-term effort to update ROOT to work both with implicit and explicit object ownership (i.e. whether THx and similar are owned by gDirectory). The branch started by switching off this ownership and fixing all failing tests. These fixes will be done in a way that the code works both with and without implicit ownership, and will be split off into other PRs to keep the review load manageable. Once that's done, the final names and implementations of the ownership-related functions will be decided here.
TODOs:
TH1::AddDirectory()as suggested in ReplaceTH1::AddDirectorybyTDirectory::TContext#21523 ?Note: No need to review until the following PRs are merged:
gDirectory, use TContext instead. #21287TH1::AddDirectorybyTDirectory::TContext#21523