Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow splitting of TTime objects #12499

Merged
merged 1 commit into from
Apr 19, 2023
Merged

Conversation

pcanal
Copy link
Member

@pcanal pcanal commented Mar 17, 2023

This fixes #12498.

TTime objects were not (anymore) split because TTime is still using the automatically generated but streamer-function based I/O. (The splitting stopped when TTree was upgraded to respect custom streamers).

@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac12/noimt, mac11/cxx14, windows10/cxx14
How to customize builds

@github-actions
Copy link

Test Results

       2 files         2 suites   15h 34m 9s ⏱️
2 416 tests 2 391 ✔️ 2 💤 23
4 834 runs  4 790 ✔️ 8 💤 36

For more details on these failures, see this check.

Results for commit 42d25a9.

Copy link
Member

@Axel-Naumann Axel-Naumann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really forward (and backward) compatible?

Is this class the only case in ROOT?

@pcanal
Copy link
Member Author

pcanal commented Mar 17, 2023

Is this really forward (and backward) compatible?

For this class the non-split layout is exactly the same, the only difference is splitting is now allowed. I verified that a non-split file can (of course) be read.

@pcanal
Copy link
Member Author

pcanal commented Mar 17, 2023

Is this class the only case in ROOT?

It is 'hard' to say. There about a 1000 pragma link C++ class with a + or -. Those are legitimate if the there is no ClassDef nor Streamer function (in which case there is no alternative to the StreamerInfo based I/O) and for classes which version is 0 (i.e. not intended for storage). I suppose we could write a script to find out.

@pcanal
Copy link
Member Author

pcanal commented Mar 17, 2023

So out of 1012 such pragma link, there 199 classes I could not (i.e. I did not build the plugin).
590 classes had a class version of 0 (so it is fine)
150 classes had no ClassDef (so it is fine)
12 were reported as having no Steamer function but inherited from TObject (and upon inspection seemed to have a ClassDef), so it looks a bug in the detection in rootcling for some case of class template instance.

That left 61 potential issue (list below) and from that list only TTime seemed to be both intended for I/O and might be put in a TTree:

Potential issue: TAttImage
Potential issue: TImage
Potential issue: TImagePalette
Potential issue: TPyReturn
Potential issue: TPyArg
Potential issue: TPyReturn
Potential issue: TPyArg
Potential issue: TRecEvent
Potential issue: TRecCmdEvent
Potential issue: TRecExtraEvent
Potential issue: TRecGuiEvent
Potential issue: TRecWinPair
Potential issue: TRecorder
Potential issue: TXMLPlayer
Potential issue: TXMLEngine
Potential issue: TXMLSetup
Potential issue: TXMLFile
Potential issue: TKeyXML
Potential issue: TKeySQL
Potential issue: TSQLClassInfo
Potential issue: TSQLClassColumnInfo
Potential issue: TSQLObjectInfo
Potential issue: TSQLObjectData
Potential issue: TSQLObjectDataPool
Potential issue: TSQLStructure
Potential issue: TSQLColumnData
Potential issue: TSQLTableData
Potential issue: TFree
Potential issue: TFumiliMinimizer
Potential issue: TStructNodeProperty
Potential issue: TGLVertex3
Potential issue: TGLVector3
Potential issue: TGLMatrix
Potential issue: TGLCamera
Potential issue: TGLOrthoCamera
Potential issue: TGLPerspectiveCamera
Potential issue: TGLCameraOverlay
Potential issue: TVirtualMonitoringReader
Potential issue: TUri
Potential issue: TUrl
Potential issue: SysInfo_t
Potential issue: CpuInfo_t
Potential issue: MemInfo_t
Potential issue: ProcInfo_t
Potential issue: TTime
Potential issue: TPluginHandler
Potential issue: TPluginManager
Potential issue: TArray!
Potential issue: TDataType
Potential issue: TGlobal
Potential issue: THostAuth
Potential issue: TProofDesc
Potential issue: TProofNodeInfo
Potential issue: TProofProgressStatus
Potential issue: TProofProgressInfo
Potential issue: TNetFile
Potential issue: TWebFile
Potential issue: TFTP
Potential issue: TSelectorDraw
Potential issue: TSelectorEntries
Potential issue: TChainElement

@pcanal pcanal merged commit d98341e into root-project:master Apr 19, 2023
@pcanal pcanal deleted the master-12498 branch April 19, 2023 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TTime class is not splitable
3 participants