Record alignment of class in dictionary and use in TStreamerInfo#21669
Record alignment of class in dictionary and use in TStreamerInfo#21669pcanal wants to merge 22 commits intoroot-project:masterfrom
Conversation
Test Results 22 files 22 suites 3d 5h 48m 15s ⏱️ Results for commit f485286. ♻️ This comment has been updated with latest results. |
jblomer
left a comment
jblomer
left a comment
There was a problem hiding this comment.
Thanks!
I think it would be nice to have a separate test for AlignOf just for the cling part that exercises the different special cases (forward declaration etc.)
core/meta/src/TClass.cxx
Outdated
| // return fCollectionProxy->AlignOf(); | ||
| if (HasInterpreterInfo()) | ||
| return gCling->ClassInfo_AlignOf(GetClassInfo()); | ||
| return GetStreamerInfo()->AlignOf(); |
There was a problem hiding this comment.
This is a good question ... Currently (see updated doc) TClingClassInfo return -1 for error case but 0 for semi-error cases (eg. a namespace). It is unclear whether we want to treat those as an exception (i.e. the caller would get an assert if this is not called on a Class.
| /// Allocator that allocates memory aligned to at least \a Align bytes. | ||
| /// The alignment is chosen at construction time and must be a power of two. | ||
| template <typename T> | ||
| struct AlignedAllocator { |
There was a problem hiding this comment.
This may be useful for RNTuple, too. Perhaps we can move it as a utility class to Core.
There was a problem hiding this comment.
The custom allocator has been removed because it leads to the in-memory layout to be different from regular vector.
There was a problem hiding this comment.
Actually it is still there but not used. Should I remove it or do you recommend I move it?
pcanal
force-pushed
the
tclass-alignof
branch
2 times, most recently
from
d85d5f8 to
136beb7
Compare
Since align is a power of 2 the modulo may be written as (offset & (align - 1)) (assuming offset is non-negative) Co-authored-by: silverweed <giacomo.parolini@cern.ch>
The compression factor the TTree is changed due the fact that the numerical value record for `TBranch::fOffset` are changed.
…d numerical types
Wrong now that we took base class alignment in consideration. This reverts commit 37eba5a.
3 participants
This fixes #21667 and thus addresses CMS issue seen at github.com/cms-sw/cmssw/pull/49969
With the current version storage of collection whose content needs to be aligned with more than 4096 is not supported. Extending it to higher number is trivial but require specific handling for each value so we had to stop at an arbitrary value. The error is seen at dictionary compilation time:
Alignment information has also been added to
TDataType, hence improving (tightening) the alignment of numerical type data member in emulated classes and requiring updates some test reference files.