Skip to content

Commit 36bc25f

Browse files
jblomerjblomer
committed
[ntuple] fix up vector new/delete for over-aligned values
1 parent ab41dbe commit 36bc25f

3 files changed

Lines changed: 86 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
@@ -219,11 +219,12 @@ class RVectorField : public RFieldBase {
219219
class RVectorDeleter : public RDeleter {
220220
private:
221221
std::size_t fItemSize = 0;
222+
std::size_t fItemAlignment = 0;
222223
std::unique_ptr<RDeleter> fItemDeleter;
223224

224225
public:
225-
RVectorDeleter();
226-
RVectorDeleter(std::size_t itemSize, std::unique_ptr<RDeleter> itemDeleter);
226+
explicit RVectorDeleter(std::size_t itemAlignment);
227+
RVectorDeleter(std::size_t itemSize, std::size_t itemAlignment, std::unique_ptr<RDeleter> itemDeleter);
227228
void operator()(void *objPtr, bool dtorOnly) final;
228229
};
229230

@@ -248,7 +249,7 @@ protected:
248249
void GenerateColumns() final;
249250
void GenerateColumns(const ROOT::RNTupleDescriptor &desc) final;
250251

251-
void ConstructValue(void *where) const final { new (where) std::vector<char>(); }
252+
void ConstructValue(void *where) const final;
252253
std::unique_ptr<RDeleter> GetDeleter() const final;
253254

254255
std::size_t AppendImpl(const void *from) final;
@@ -402,7 +403,7 @@ protected:
402403
void GenerateColumns() final;
403404
using RFieldBase::GenerateColumns;
404405

405-
void ConstructValue(void *where) const final { new (where) std::vector<char>(); }
406+
void ConstructValue(void *where) const final;
406407
/// Returns an RVectorField::RVectorDeleter
407408
std::unique_ptr<RDeleter> GetDeleter() const final;
408409

‎tree/ntuple/src/RFieldSequenceContainer.cxx‎

Lines changed: 73 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,29 @@ std::size_t GetAlignOfVector()
4040
return alignof(std::vector<char>);
4141
}
4242

43+
void ConstructVector(void *where, std::size_t alignOfValue)
44+
{
45+
static_assert(ROOT::RVectorField::kMaxItemAlignment == 4096);
46+
// clang-format off
47+
switch (alignOfValue) {
48+
case 1: new (where) std::vector<ROOT::Internal::RAlignedStorage< 1>>(); break;
49+
case 2: new (where) std::vector<ROOT::Internal::RAlignedStorage< 2>>(); break;
50+
case 4: new (where) std::vector<ROOT::Internal::RAlignedStorage< 4>>(); break;
51+
case 8: new (where) std::vector<ROOT::Internal::RAlignedStorage< 8>>(); break;
52+
case 16: new (where) std::vector<ROOT::Internal::RAlignedStorage< 16>>(); break;
53+
case 32: new (where) std::vector<ROOT::Internal::RAlignedStorage< 32>>(); break;
54+
case 64: new (where) std::vector<ROOT::Internal::RAlignedStorage< 64>>(); break;
55+
case 128: new (where) std::vector<ROOT::Internal::RAlignedStorage< 128>>(); break;
56+
case 256: new (where) std::vector<ROOT::Internal::RAlignedStorage< 256>>(); break;
57+
case 512: new (where) std::vector<ROOT::Internal::RAlignedStorage< 512>>(); break;
58+
case 1024: new (where) std::vector<ROOT::Internal::RAlignedStorage<1024>>(); break;
59+
case 2048: new (where) std::vector<ROOT::Internal::RAlignedStorage<2048>>(); break;
60+
case 4096: new (where) std::vector<ROOT::Internal::RAlignedStorage<4096>>(); break;
61+
default: throw ROOT::RException(R__FAIL(std::string("Unsupported alignment: ") + std::to_string(alignOfValue)));
62+
}
63+
// clang-format on
64+
}
65+
4366
} // anonymous namespace
4467

4568
ROOT::RArrayField::RArrayField(std::string_view fieldName, std::unique_ptr<RFieldBase> itemField,
@@ -672,33 +695,65 @@ void ROOT::RVectorField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
672695
EnsureMatchingOnDiskCollection(desc).ThrowOnError();
673696
}
674697

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

677-
ROOT::RVectorField::RVectorDeleter::RVectorDeleter(std::size_t itemSize, std::unique_ptr<RDeleter> itemDeleter)
678-
: 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()),
711+
fItemSize(itemSize),
712+
fItemAlignment(itemAlignment),
713+
fItemDeleter(std::move(itemDeleter))
679714
{
680715
}
681716

682717
void ROOT::RVectorField::RVectorDeleter::operator()(void *objPtr, bool dtorOnly)
683718
{
684719
auto vecPtr = static_cast<std::vector<char> *>(objPtr);
685720
if (fItemDeleter) {
686-
R__ASSERT(fItemSize > 0);
687-
R__ASSERT((vecPtr->size() % fItemSize) == 0);
721+
assert(fItemSize > 0);
688722
auto nItems = vecPtr->size() / fItemSize;
723+
assert((vecPtr->size() % fItemSize) == 0);
689724
for (std::size_t i = 0; i < nItems; ++i) {
690725
fItemDeleter->operator()(vecPtr->data() + (i * fItemSize), true /* dtorOnly */);
691726
}
692727
}
693-
std::destroy_at(vecPtr);
728+
729+
static_assert(kMaxItemAlignment == 4096);
730+
// clang-format off
731+
switch (fItemAlignment) {
732+
case 1: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 1>> *>(objPtr)); break;
733+
case 2: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 2>> *>(objPtr)); break;
734+
case 4: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 4>> *>(objPtr)); break;
735+
case 8: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 8>> *>(objPtr)); break;
736+
case 16: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 16>> *>(objPtr)); break;
737+
case 32: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 32>> *>(objPtr)); break;
738+
case 64: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 64>> *>(objPtr)); break;
739+
case 128: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 128>> *>(objPtr)); break;
740+
case 256: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 256>> *>(objPtr)); break;
741+
case 512: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage< 512>> *>(objPtr)); break;
742+
case 1024: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage<1024>> *>(objPtr)); break;
743+
case 2048: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage<2048>> *>(objPtr)); break;
744+
case 4096: std::destroy_at(static_cast<std::vector<Internal::RAlignedStorage<4096>> *>(objPtr)); break;
745+
default: throw ROOT::RException(R__FAIL(std::string("Unsupported alignment: ") + std::to_string(fItemAlignment)));
746+
}
747+
// clang-format on
748+
694749
RDeleter::operator()(objPtr, dtorOnly);
695750
}
696751

697752
std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::RVectorField::GetDeleter() const
698753
{
699754
if (fItemDeleter)
700-
return std::make_unique<RVectorDeleter>(fItemSize, GetDeleterOf(*fSubfields[0]));
701-
return std::make_unique<RVectorDeleter>();
755+
return std::make_unique<RVectorDeleter>(fItemSize, fSubfields[0]->GetAlignment(), GetDeleterOf(*fSubfields[0]));
756+
return std::make_unique<RVectorDeleter>(fSubfields[0]->GetAlignment());
702757
}
703758

704759
std::vector<ROOT::RFieldBase::RValue> ROOT::RVectorField::SplitValue(const RValue &value) const
@@ -985,11 +1040,18 @@ void ROOT::RArrayAsVectorField::GenerateColumns()
9851040
throw RException(R__FAIL("RArrayAsVectorField fields must only be used for reading"));
9861041
}
9871042

1043+
void ROOT::RArrayAsVectorField::ConstructValue(void *where) const
1044+
{
1045+
ConstructVector(where, fSubfields[0]->GetAlignment());
1046+
}
1047+
9881048
std::unique_ptr<ROOT::RFieldBase::RDeleter> ROOT::RArrayAsVectorField::GetDeleter() const
9891049
{
990-
if (fItemDeleter)
991-
return std::make_unique<RVectorField::RVectorDeleter>(fItemSize, GetDeleterOf(*fSubfields[0]));
992-
return std::make_unique<RVectorField::RVectorDeleter>();
1050+
if (fItemDeleter) {
1051+
return std::make_unique<RVectorField::RVectorDeleter>(fItemSize, fSubfields[0]->GetAlignment(),
1052+
GetDeleterOf(*fSubfields[0]));
1053+
}
1054+
return std::make_unique<RVectorField::RVectorDeleter>(fSubfields[0]->GetAlignment());
9931055
}
9941056

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