[ntuple] Mark RField methods as final and remove duplicate implementations #16721
Conversation
This doesn't make a difference because the classes are already final, but simplifies automated checking.
They should be leaves in the class hierarchy.
It is provided by the type-erased RRVecField. On model reconstruction and via RFieldBase::Create, we use this implementation anyway and it actually contains optimizations to use AppendV and ReadV, if possible.
The implementations were identical to the generic ones in RProxiedCollectionField.
This pessimizes the alignment of RMapField and RSetField to that of std::max_align_t, but they were wrong anyway because they assumed that all std::[unordered_][multi]map and all std::[unordered_][multi]set have the same alignment.
They are provided by the type-erased base classes.
It was only used in the constructor, and also not properly taken care of by CloneImpl().
Test Results 17 files 17 suites 3d 19h 37m 6s ⏱️ For more details on these failures, see this check. Results for commit 5e6d5a5c. |
| new (where) T(); | ||
| } else { | ||
| // If there is no default constructor, try with the IO constructor | ||
| new (where) T(static_cast<TRootIOCtor *>(nullptr)); |
There was a problem hiding this comment.
Since the tests still work, I assume TClass::New() takes proper care of using the I/O constructor where applicable...?
There was a problem hiding this comment.
Yes, in fact now I learned that TClass::New() allows more customization from the user: https://root.cern.ch/doc/master/classTClass.html#a1e952a88f08eb6ec665a7ac862860689. I cross-checked that removing the IO constructor of class IOConstructor in tree/ntuple/v7/test/CustomStruct.hxx fails the ntuple-types test, so it seems to be working with this PR.
There was a problem hiding this comment.
Yes, this is essentially the whole point of TClass::New: construct the object during I/O operations :)
(and usually it does not require any run-time compilation/interpretation as the dictionary provide an accelerator function wrapping the 'right' call to operator new + constructor.
RFieldspecializations asfinal; the only exception isRField<std::array<ItemT, N>>which is the base class ofRField<ItemT[N]>.RFieldBasehierarchy as final; the only exception isRSimpleField::GenerateColumnswhich is overriden byRRealField.RFieldBase::Create, when reconstructing a model from disk, or whenCloneing a model. So if they are not performant, we have to fix them anyway. For completeness, there seems to be no performance change in theiotoolsbenchmarks (with hot cache, to put emphasis on the RNTuple implementation).