Skip to content

Record alignment of class in dictionary and use in TStreamerInfo#21669

Open
pcanal wants to merge 12 commits intoroot-project:masterfrom
pcanal:tclass-alignof
Open

Record alignment of class in dictionary and use in TStreamerInfo#21669
pcanal wants to merge 12 commits intoroot-project:masterfrom
pcanal:tclass-alignof

Conversation

@pcanal
Copy link
Copy Markdown
Member

@pcanal pcanal commented Mar 24, 2026

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:

..../root/meta/alignment/evolution_C_ACLiC_dict.cxx:483:21: error: static assertion failed due to requirement 'alignof(std::pair<const int, Wrapper>) <= 4096': Class with
      alignment strictly greater than 4096 are currently not supported in CollectionProxy. Please report this case to the developers
  483 |       static_assert(alignof(map<int,Wrapper>::value_type) <= 64,
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 24, 2026

Test Results

    21 files      21 suites   3d 3h 1m 42s ⏱️
 3 835 tests  3 808 ✅  1 💤  26 ❌
72 233 runs  71 901 ✅ 18 💤 314 ❌

For more details on these failures, see this check.

Results for commit dbf6175.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown
Contributor

@jblomer jblomer left a comment

Choose a reason for hiding this comment

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

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.)

Comment on lines +5763 to +5765
// FIX-ALIGN: do we need to have something special for collection proxy?
//if (fCollectionProxy)
// return fCollectionProxy->AlignOf();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it would be good to resolve the question in this PR.

// return fCollectionProxy->AlignOf();
if (HasInterpreterInfo())
return gCling->ClassInfo_AlignOf(GetClassInfo());
return GetStreamerInfo()->AlignOf();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

assert return value >0.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This may be useful for RNTuple, too. Perhaps we can move it as a utility class to Core.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The custom allocator has been removed because it leads to the in-memory layout to be different from regular vector.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Actually it is still there but not used. Should I recommend or do you recommend I move it?

@pcanal pcanal closed this Mar 30, 2026
@pcanal pcanal reopened this Mar 30, 2026
@pcanal pcanal requested a review from jblomer March 30, 2026 22:20
@pcanal pcanal closed this Mar 30, 2026
@pcanal pcanal reopened this Mar 30, 2026
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.

Missing alignment information in TClass/TStreamerInfo

3 participants