Skip to content

Commit ab41dbe

Browse files
committed
[ntuple] fix up vector resize for over-aligned values
1 parent dad1770 commit ab41dbe

5 files changed

Lines changed: 48 additions & 4 deletions

File tree

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,9 @@ protected:
260260
void CommitClusterImpl() final { fNWritten = 0; }
261261

262262
public:
263+
/// Maximum alignment of the vector's value type
264+
static constexpr std::size_t kMaxItemAlignment = 4096;
265+
263266
RVectorField(std::string_view fieldName, std::unique_ptr<RFieldBase> itemField);
264267
RVectorField(RVectorField &&other) = default;
265268
RVectorField &operator=(RVectorField &&other) = default;

tree/ntuple/inc/ROOT/RFieldBase.hxx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ protected:
110110
/// the field has been destructed.
111111
class RDeleter {
112112
std::size_t fAlignment;
113+
void DeleteAligned(void *objPtr) const; // outlined to make Windows happy
113114

114115
public:
115116
explicit RDeleter(std::size_t align) : fAlignment(align) { assert(Internal::IsValidAlignment(align)); }
@@ -123,7 +124,7 @@ protected:
123124
if (fAlignment <= sizeof(std::max_align_t)) {
124125
operator delete(objPtr);
125126
} else {
126-
operator delete(objPtr, std::align_val_t(fAlignment));
127+
DeleteAligned(objPtr);
127128
}
128129
}
129130
};

tree/ntuple/src/RFieldBase.cxx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,13 @@ ROOT::Internal::CallFieldBaseCreate(const std::string &fieldName, const std::str
9696

9797
//------------------------------------------------------------------------------
9898

99+
void ROOT::RFieldBase::RDeleter::DeleteAligned(void *objPtr) const
100+
{
101+
operator delete(objPtr, std::align_val_t(fAlignment));
102+
}
103+
104+
//------------------------------------------------------------------------------
105+
99106
ROOT::RFieldBase::RColumnRepresentations::RColumnRepresentations()
100107
{
101108
// A single representations with an empty set of columns

tree/ntuple/src/RFieldSequenceContainer.cxx

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,41 @@ void ROOT::RVectorField::ResizeVector(void *vec, std::size_t nItems, std::size_t
567567
itemDeleter->operator()(typedValue->data() + (i * itemSize), true /* dtorOnly */);
568568
}
569569
}
570-
typedValue->resize(nItems * itemSize);
570+
571+
// Resize the vector with correct alignment
572+
const auto itemAlignment = itemField.GetAlignment();
573+
const auto nbytes = nItems * itemSize;
574+
575+
auto fnAlignedResize = [](auto &vecOfAlignedStorage, std::size_t targetSize) {
576+
constexpr auto valueTypeSize = sizeof(typename std::decay_t<decltype(vecOfAlignedStorage)>::value_type);
577+
constexpr auto valueTypeAlignment = alignof(typename std::decay_t<decltype(vecOfAlignedStorage)>::value_type);
578+
// Ensure that this function is used with RAlignedStorage as a template argument.
579+
// In general, the actual (user-defined) item type may have larger size than alignment.
580+
static_assert(valueTypeSize == valueTypeAlignment);
581+
assert(targetSize % valueTypeAlignment == 0);
582+
vecOfAlignedStorage.resize(targetSize / valueTypeSize);
583+
};
584+
585+
static_assert(kMaxItemAlignment == 4096);
586+
// clang-format off
587+
switch (itemAlignment) {
588+
case 1: fnAlignedResize(*static_cast<std::vector<Internal::RAlignedStorage< 1>> *>(vec), nbytes); break;
589+
case 2: fnAlignedResize(*static_cast<std::vector<Internal::RAlignedStorage< 2>> *>(vec), nbytes); break;
590+
case 4: fnAlignedResize(*static_cast<std::vector<Internal::RAlignedStorage< 4>> *>(vec), nbytes); break;
591+
case 8: fnAlignedResize(*static_cast<std::vector<Internal::RAlignedStorage< 8>> *>(vec), nbytes); break;
592+
case 16: fnAlignedResize(*static_cast<std::vector<Internal::RAlignedStorage< 16>> *>(vec), nbytes); break;
593+
case 32: fnAlignedResize(*static_cast<std::vector<Internal::RAlignedStorage< 32>> *>(vec), nbytes); break;
594+
case 64: fnAlignedResize(*static_cast<std::vector<Internal::RAlignedStorage< 64>> *>(vec), nbytes); break;
595+
case 128: fnAlignedResize(*static_cast<std::vector<Internal::RAlignedStorage< 128>> *>(vec), nbytes); break;
596+
case 256: fnAlignedResize(*static_cast<std::vector<Internal::RAlignedStorage< 256>> *>(vec), nbytes); break;
597+
case 512: fnAlignedResize(*static_cast<std::vector<Internal::RAlignedStorage< 512>> *>(vec), nbytes); break;
598+
case 1024: fnAlignedResize(*static_cast<std::vector<Internal::RAlignedStorage<1024>> *>(vec), nbytes); break;
599+
case 2048: fnAlignedResize(*static_cast<std::vector<Internal::RAlignedStorage<2048>> *>(vec), nbytes); break;
600+
case 4096: fnAlignedResize(*static_cast<std::vector<Internal::RAlignedStorage<4096>> *>(vec), nbytes); break;
601+
default: throw RException(R__FAIL(std::string("Unsupported alignment: ") + std::to_string(itemAlignment)));
602+
}
603+
// clang-format on
604+
571605
if (!(itemField.GetTraits() & kTraitTriviallyConstructible)) {
572606
for (std::size_t i = allDeallocated ? 0 : oldNItems; i < nItems; ++i) {
573607
CallConstructValueOn(itemField, typedValue->data() + (i * itemSize));

tree/ntuple/test/rfield_class.cxx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,5 @@ TEST(RNTuple, AlignmentCornerCases)
540540

541541
auto view = reader->GetView<AlignmentEnvelope>("f");
542542
EXPECT_EQ(1u, view(0).fVec.size());
543-
// TODO(jblomer): FIXME
544-
// EXPECT_EQ(0, reinterpret_cast<std::uintptr_t>(view(0).fVec.data()) % alignof(OverAligned));
543+
EXPECT_EQ(0, reinterpret_cast<std::uintptr_t>(view(0).fVec.data()) % alignof(OverAligned));
545544
}

0 commit comments

Comments
 (0)