Skip to content

v6-36: Record alignment of class in dictionary and use in TStreamerInfo#21976

Draft
pcanal wants to merge 29 commits intoroot-project:v6-36-00-patchesfrom
pcanal:v636-tclass-alignof
Draft

v6-36: Record alignment of class in dictionary and use in TStreamerInfo#21976
pcanal wants to merge 29 commits intoroot-project:v6-36-00-patchesfrom
pcanal:v636-tclass-alignof

Conversation

@pcanal
Copy link
Copy Markdown
Member

@pcanal pcanal commented Apr 20, 2026

Backport of #21669

@pcanal pcanal requested review from bellenot and dpiparo as code owners April 20, 2026 18:42
@pcanal pcanal force-pushed the v636-tclass-alignof branch from 3a28638 to 3b051c8 Compare April 20, 2026 18:56
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 20, 2026 •
edited
Loading

Test Results

    17 files      17 suites   2d 7h 3m 20s ⏱️
 2 744 tests  2 743 ✅  1 💤 0 ❌
45 202 runs  45 190 ✅ 12 💤 0 ❌

Results for commit ef4fde1.

♻️ This comment has been updated with latest results.

@pcanal pcanal mentioned this pull request Apr 20, 2026
Open
@pcanal pcanal closed this Apr 20, 2026
@pcanal pcanal reopened this Apr 20, 2026
@pcanal pcanal closed this Apr 21, 2026
@pcanal pcanal reopened this Apr 21, 2026
@pcanal pcanal force-pushed the v636-tclass-alignof branch from 3b051c8 to 4fd0297 Compare April 21, 2026 02:10
hahnjo
hahnjo requested changes Apr 21, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@hahnjo hahnjo left a comment

Choose a reason for hiding this comment

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

see #21669 (comment)

@pcanal pcanal requested a review from hahnjo April 22, 2026 19:59
pcanal and others added 17 commits April 22, 2026 21:59
@pcanal
meta: Record alignment of class in dictionary
6448f87 
Includes alignment of allocatation of emulated objects in container
@pcanal
meta: Add alignment info for emulated collections
8a958a9 
Handle alignment for Emulated Collection of pointer and numerical types
@pcanal @silverweed
io: Micro-optimization in alignment calculation.
e8578e0 
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>
@pcanal
io: Factor out alignment operation
cf07ebd
@pcanal
meta: Use std::max_align_t instead of sizeof(void*)
d67209c 
as the fall back when missing alignment information.
@pcanal
io: Explicitly forbid collection with content aligned to more than 4096
cb6895b
@pcanal
meta: Add alignment info to TDataType
b201943
@pcanal
meta: Remove over alignment of emulated class
37552a5
@pcanal
meta: Alignment extend code doc
e86acc1
@pcanal
meta: Take emulation header in consideration for alignment value
8ca7406
@pcanal
meta: Factor out and standardize alignment assert.
472d169
@pcanal
meta: Factor AlignUp function
89f286c
@pcanal
io: Remove TGenCollectionProxy::AlignedAllocator
d2e4abc
@pcanal
meta: Extend alignment asserts
1f83e57
@pcanal
io: Factor alignment calc for NewArray and DeleteArrray
4eb45e7
@pcanal
Meta: Alignment calc for non-current StreamerInfo
b2ba7c0
@pcanal
io: Make missing alignment fatal
4d1848f
@pcanal pcanal force-pushed the v636-tclass-alignof branch from 64c4561 to b44dc37 Compare April 23, 2026 03:00
hahnjo
hahnjo requested changes Apr 23, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@hahnjo hahnjo left a comment

Choose a reason for hiding this comment

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

I see you added the commit to change the type of TDataType::fAlignOf, but not the bump of the class version.

I would also like to remark that these are quite extensive changes at very core classes. I'm not sure if this absolutely requires backporting to a stable release used in data taking.

@pcanal
Copy link
Copy Markdown
Member Author

pcanal commented Apr 23, 2026

True on the other hand this is repairing a 'bug' that prevent CMS(SW) from enabling a rule to read older files.

@pcanal pcanal force-pushed the v636-tclass-alignof branch from b44dc37 to ef4fde1 Compare April 23, 2026 19:04
@pcanal
Copy link
Copy Markdown
Member Author

pcanal commented Apr 23, 2026

I see you added the commit to change the type of TDataType::fAlignOf, but not the bump of the class version

Thanks for spotting this inadvertent omission!

@pcanal pcanal requested a review from hahnjo April 23, 2026 19:04
@pcanal
Copy link
Copy Markdown
Member Author

pcanal commented Apr 23, 2026

Currently Checking with CMS how much this backport is needed.

@pcanal pcanal marked this pull request as draft April 23, 2026 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bellenot bellenot Awaiting requested review from bellenot bellenot is a code owner

@dpiparo dpiparo Awaiting requested review from dpiparo dpiparo is a code owner

@hahnjo hahnjo Awaiting requested review from hahnjo

Requested changes must be addressed to merge this pull request.

Assignees

@pcanal pcanal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pcanal @hahnjo