Skip to content

Commit 787669e

Browse files
jblomerjblomer
committed
[ntuple] fix up vector new/delete for over-aligned values
1 parent 458f2f2 commit 787669e

3 files changed

Lines changed: 83 additions & 18 deletions

File tree

‎tree/ntuple/inc/ROOT/RField/RFieldSequenceContainer.hxx‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,12 @@ class RVectorField : public RFieldBase {
214214
class RVectorDeleter : public RDeleter {
215215
private:
216216
std::size_t fItemSize = 0;
217+
std::size_t fItemAlignment = 0;
217218
std::unique_ptr<RDeleter> fItemDeleter;
218219

219220
public:
220-
RVectorDeleter();
221-
RVectorDeleter(std::size_t itemSize, std::unique_ptr<RDeleter> itemDeleter);
221+
explicit RVectorDeleter(std::size_t itemAlignment);
222+
RVectorDeleter(std::size_t itemSize, std::size_t itemAlignment, std::unique_ptr<RDeleter> itemDeleter);
222223
void operator()(void *objPtr, bool dtorOnly) final;
223224
};
224225

@@ -243,7 +244,7 @@ protected:
243244
void GenerateColumns() final;
244245
void GenerateColumns(const ROOT::RNTupleDescriptor &desc) final;
245246

246-
void ConstructValue(void *where) const final { new (where) std::vector<char>(); }
247+
void ConstructValue(void *where) const final;
247248
std::unique_ptr<RDeleter> GetDeleter() const final;
248249

249250
std::size_t AppendImpl(const void *from) final;
@@ -394,7 +395,7 @@ protected:
394395
void GenerateColumns() final;
395396
using RFieldBase::GenerateColumns;
396397

397-
void ConstructValue(void *where) const final { new (where) std::vector<char>(); }
398+
void ConstructValue(void *where) const final;
398399
/// Returns an RVectorField::RVectorDeleter
399400
std::unique_ptr<RDeleter> GetDeleter() const final;
400401

‎tree/ntuple/src/RFieldSequenceContainer.cxx‎

Lines changed: 70 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,29 @@ std::size_t GetAlignOfVector()
4444
return alignof(std::vector<char>);
4545
}
4646

47+
void ConstructVector(void *where, std::size_t alignOfValue)
48+
{
49+
if (alignOfValue <= sizeof(std::max_align_t)) {
50+
new (where) std::vector<char>();
51+
} else {
52+
static_assert(sizeof(std::max_align_t) >= 8 && ROOT::RFieldBase::kMaxAlignment == 4096);
53+
// clang-format off
54+
switch (alignOfValue) {
55+
case 16: new (where) std::vector<ROOT::Internal::RAlignedStorage< 16>>(); break;
56+
case 32: new (where) std::vector<ROOT::Internal::RAlignedStorage< 32>>(); break;
57+
case 64: new (where) std::vector<ROOT::Internal::RAlignedStorage< 64>>(); break;
58+
case 128: new (where) std::vector<ROOT::Internal::RAlignedStorage< 128>>(); break;
59+
case 256: new (where) std::vector<ROOT::Internal::RAlignedStorage< 256>>(); break;
60+
case 512: new (where) std::vector<ROOT::Internal::RAlignedStorage< 512>>(); break;
61+
case 1024: new (where) std::vector<ROOT::Internal::RAlignedStorage<1024>>(); break;
62+
case 2048: new (where) std::vector<ROOT::Internal::RAlignedStorage<2048>>(); break;
63+
case 4096: new (where) std::vector<ROOT::Internal::RAlignedStorage<4096>>(); break;
64+
default: R__ASSERT(false);
65+
}
66+
// clang-format on
67+
}
68+
}
69+
4770
} // anonymous namespace
4871

4972
ROOT::RArrayField::RArrayField(std::string_view fieldName, std::unique_ptr<RFieldBase> itemField,
@@ -673,33 +696,62 @@ void ROOT::RVectorField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
673696
EnsureMatchingOnDiskCollection(desc).ThrowOnError();
674697
}
675698

676-
ROOT::RVectorField::RVectorDeleter::RVectorDeleter() : RDeleter(GetAlignOfVector()) {}
699+
void ROOT::RVectorField::ConstructValue(void *where) const
700+
{
701+
ConstructVector(where, fSubfields[0]->GetAlignment());
702+
}
703+
704+
ROOT::RVectorField::RVectorDeleter::RVectorDeleter(std::size_t itemAlignment)
705+
: RDeleter(GetAlignOfVector()), fItemAlignment(itemAlignment)
706+
{}
677707

678-
ROOT::RVectorField::RVectorDeleter::RVectorDeleter(std::size_t itemSize, std::unique_ptr<RDeleter> itemDeleter)
679-
: RDeleter(GetAlignOfVector()), fItemSize(itemSize), fItemDeleter(std::move(itemDeleter))
708+
ROOT::RVectorField::RVectorDeleter::RVectorDeleter(std::size_t itemSize, std::size_t itemAlignment,
709+
std::unique_ptr<RDeleter> itemDeleter)
710+
: RDeleter(GetAlignOfVector()), fItemSize(itemSize), fItemAlignment(itemAlignment),
711+
fItemDeleter(std::move(itemDeleter))
680712
{
681713
}
682714

683715
void ROOT::RVectorField::RVectorDeleter::operator()(void *objPtr, bool dtorOnly)
684716
{
685717
auto vecPtr = static_cast<std::vector<char> *>(objPtr);
686718
if (fItemDeleter) {
687-
R__ASSERT(fItemSize > 0);
688-
R__ASSERT((vecPtr->size() % fItemSize) == 0);
719+
assert(fItemSize > 0);
689720
auto nItems = vecPtr->size() / fItemSize;
721+
assert((vecPtr->size() % fItemSize) == 0);
690722
for (std::size_t i = 0; i < nItems; ++i) {
691723
fItemDeleter->operator()(vecPtr->data() + (i * fItemSize), true /* dtorOnly */);
692724
}
693725
}
694-
std::destroy_at(vecPtr);
726+
727+
if (fItemAlignment <= sizeof(std::max_align_t)) {
728+
std::destroy_at(vecPtr);
729+
} else {
730+
static_assert(sizeof(std::max_align_t) >= 8 && ROOT::RFieldBase::kMaxAlignment == 4096);
731+
// clang-format off
732+
switch (fItemAlignment) {
733+
case 16: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 16>> *>(objPtr)); break;
734+
case 32: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 32>> *>(objPtr)); break;
735+
case 64: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 64>> *>(objPtr)); break;
736+
case 128: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 128>> *>(objPtr)); break;
737+
case 256: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 256>> *>(objPtr)); break;
738+
case 512: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 512>> *>(objPtr)); break;
739+
case 1024: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage<1024>> *>(objPtr)); break;
740+
case 2048: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage<2048>> *>(objPtr)); break;
741+
case 4096: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage<4096>> *>(objPtr)); break;
742+
default: R__ASSERT(false);
743+
}
744+
// clang-format on
745+
}
746+
695747
RDeleter::operator()(objPtr, dtorOnly);
696748
}
697749

698750
std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::RVectorField::GetDeleter() const
699751
{
700752
if (fItemDeleter)
701-
return std::make_unique<RVectorDeleter>(fItemSize, GetDeleterOf(*fSubfields[0]));
702-
return std::make_unique<RVectorDeleter>();
753+
return std::make_unique<RVectorDeleter>(fItemSize, fSubfields[0]->GetAlignment(), GetDeleterOf(*fSubfields[0]));
754+
return std::make_unique<RVectorDeleter>(fSubfields[0]->GetAlignment());
703755
}
704756

705757
std::vector<ROOT::RFieldBase::RValue> ROOT::RVectorField::SplitValue(const RValue &value) const
@@ -986,11 +1038,18 @@ void ROOT::RArrayAsVectorField::GenerateColumns()
9861038
throw RException(R__FAIL("RArrayAsVectorField fields must only be used for reading"));
9871039
}
9881040

1041+
void ROOT::RArrayAsVectorField::ConstructValue(void *where) const
1042+
{
1043+
ConstructVector(where, fSubfields[0]->GetAlignment());
1044+
}
1045+
9891046
std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::RArrayAsVectorField::GetDeleter() const
9901047
{
991-
if (fItemDeleter)
992-
return std::make_unique<RVectorField::RVectorDeleter>(fItemSize, GetDeleterOf(*fSubfields[0]));
993-
return std::make_unique<RVectorField::RVectorDeleter>();
1048+
if (fItemDeleter) {
1049+
return std::make_unique<RVectorField::RVectorDeleter>(fItemSize, fSubfields[0]->GetAlignment(),
1050+
GetDeleterOf(*fSubfields[0]));
1051+
}
1052+
return std::make_unique<RVectorField::RVectorDeleter>(fSubfields[0]->GetAlignment());
9941053
}
9951054

9961055
void ROOT::RArrayAsVectorField::ReadGlobalImpl(ROOT::NTupleSize_t globalIndex, void *to)

‎tree/ntuple/test/rfield_class.cxx‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -518,13 +518,18 @@ TEST(RNTuple, AlignmentCornerCases)
518518
auto f = RFieldBase::Create("", "AlignmentEnvelope").Unwrap();
519519
EXPECT_GT(alignof(AlignmentEnvelope), sizeof(std::max_align_t));
520520
EXPECT_EQ(alignof(AlignmentEnvelope), f->GetAlignment());
521+
std::unique_ptr<AlignmentEnvelope> ptr(f->CreateObject<AlignmentEnvelope>().release());
522+
EXPECT_EQ(0, reinterpret_cast<std::uintptr_t>(ptr.get()) % alignof(AlignmentEnvelope));
523+
524+
f = RFieldBase::Create("", "std::vector<AlignmentEnvelope>").Unwrap();
525+
auto vecPtr = f->CreateObject<std::vector<AlignmentEnvelope>>().release();
526+
EXPECT_TRUE(vecPtr->data() == nullptr ||
527+
reinterpret_cast<std::uintptr_t>(vecPtr->data()) % alignof(AlignmentEnvelope) == 0);
528+
521529
auto res = RFieldBase::Create("", "ROOT::RVec<OverAligned>");
522530
EXPECT_FALSE(res);
523531
EXPECT_THAT(res.GetError()->GetReport(), ::testing::HasSubstr("RVec does not support over-aligned types"));
524532

525-
std::unique_ptr<AlignmentEnvelope> ptr(f->CreateObject<AlignmentEnvelope>().release());
526-
EXPECT_EQ(0, reinterpret_cast<std::uintptr_t>(ptr.get()) % alignof(AlignmentEnvelope));
527-
528533
FileRaii fileGuard("test_ntuple_alignment_corner_cases.root");
529534
{
530535
auto model = ROOT::RNTupleModel::Create();

0 commit comments

Comments
 (0)