Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ public:
/// The dictionary does not need to be available for this method.
/// Needs the full descriptor to look up sub fields.
bool IsCustomEnum(const RNTupleDescriptor &desc) const;
bool IsStdAtomic() const;
};

// clang-format off
Expand Down
10 changes: 6 additions & 4 deletions tree/ntuple/src/RField.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1165,10 +1165,12 @@ std::unique_ptr<ROOT::RFieldBase> ROOT::RAtomicField::CloneImpl(std::string_view

void ROOT::RAtomicField::ReconcileOnDiskField(const RNTupleDescriptor &desc)
{
static const std::vector<std::string> prefixes = {"std::atomic<"};

EnsureMatchingOnDiskField(desc, kDiffTypeName).ThrowOnError();
EnsureMatchingTypePrefix(desc, prefixes).ThrowOnError();
const auto &fieldDesc = desc.GetFieldDescriptor(GetOnDiskId());
if (fieldDesc.GetTypeName().rfind("std::atomic<", 0) == 0) {
EnsureMatchingOnDiskField(desc, kDiffTypeName).ThrowOnError();
} else {
fSubfields[0]->SetOnDiskId(GetOnDiskId());
}
}

std::vector<ROOT::RFieldBase::RValue> ROOT::RAtomicField::SplitValue(const RValue &value) const
Expand Down
16 changes: 14 additions & 2 deletions tree/ntuple/src/RFieldBase.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -952,6 +952,18 @@ void ROOT::RFieldBase::ConnectPageSource(ROOT::Internal::RPageSource &pageSource
if (!fDescription.empty())
throw RException(R__FAIL("setting description only valid when connecting to a page sink"));

if (!fIsArtificial) {
R__ASSERT(fOnDiskId != kInvalidDescriptorId);
// Handle moving from on-disk std::atomic<T> to (compatible of) T in memory centrally because otherwise
// we would need to handle it in each and every ReconcileOnDiskField()
// Note that we have to do this before calling BeforeConnectPageSource(), which already may compare the field
// to its on-disk description.
const auto &desc = pageSource.GetSharedDescriptorGuard().GetRef();
if (!dynamic_cast<RAtomicField *>(this) && desc.GetFieldDescriptor(GetOnDiskId()).IsStdAtomic()) {
SetOnDiskId(desc.GetFieldDescriptor(GetOnDiskId()).GetLinkIds()[0]);
}
}

auto substitute = BeforeConnectPageSource(pageSource);
if (substitute) {
const RFieldBase *itr = this;
Expand All @@ -974,8 +986,8 @@ void ROOT::RFieldBase::ConnectPageSource(ROOT::Internal::RPageSource &pageSource
}

if (!fIsArtificial) {
R__ASSERT(fOnDiskId != kInvalidDescriptorId);
ReconcileOnDiskField(pageSource.GetSharedDescriptorGuard().GetRef());
const auto &desc = pageSource.GetSharedDescriptorGuard().GetRef();
ReconcileOnDiskField(desc);
}

for (auto &f : fSubfields) {
Expand Down
7 changes: 7 additions & 0 deletions tree/ntuple/src/RNTupleDescriptor.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,13 @@ bool ROOT::RFieldDescriptor::IsCustomEnum(const RNTupleDescriptor &desc) const
desc.GetFieldDescriptor(subFieldId).GetTypeName()) != std::end(gIntTypeNames);
}

bool ROOT::RFieldDescriptor::IsStdAtomic() const
{
if (fStructure != ROOT::ENTupleStructure::kPlain)
return false;
return (fTypeName.rfind("std::atomic<", 0) == 0);
}

////////////////////////////////////////////////////////////////////////////////

bool ROOT::RColumnDescriptor::operator==(const RColumnDescriptor &other) const
Expand Down
5 changes: 5 additions & 0 deletions tree/ntuple/test/CustomStruct.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ enum class CustomEnumUInt32 : unsigned int {};
enum class CustomEnumInt64 : long int {};
enum class CustomEnumUInt64 : unsigned long int {};

// Used for std::atomic tests as an example of a class that is not lock-free.
struct CustomAtomicNotLockFree {
int a[100];
};

struct CustomStruct {
template <typename T>
using MyVec = std::vector<T>;
Expand Down
2 changes: 2 additions & 0 deletions tree/ntuple/test/CustomStructLinkDef.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#pragma link C++ enum CustomEnumInt64;
#pragma link C++ enum CustomEnumUInt64;

#pragma link C++ class CustomAtomicNotLockFree+;

#pragma link C++ class CustomStruct+;
#pragma link C++ class DerivedA+;
#pragma link C++ class DerivedA2+;
Expand Down
54 changes: 54 additions & 0 deletions tree/ntuple/test/ntuple_evolution_type.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,60 @@ TEST(RNTupleEvolution, Enum)
EXPECT_EQ(kRenamedCustomEnumVal, ve3(0));
}

TEST(RNTupleEvolution, CheckAtomic)
{
EXPECT_FALSE(std::atomic<CustomAtomicNotLockFree>{}.is_lock_free());

FileRaii fileGuard("test_ntuple_evolution_check_atomic.root");
{
auto model = ROOT::RNTupleModel::Create();
auto atomicInt = model->MakeField<std::atomic<std::int32_t>>("atomicInt");
auto regularInt = model->MakeField<std::int32_t>("regularInt");
auto atomicClass = model->MakeField<std::atomic<CustomAtomicNotLockFree>>("atomicClass");
auto regularClass = model->MakeField<CustomAtomicNotLockFree>("regularClass");
auto writer = ROOT::RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath());

*atomicInt = 7;
*regularInt = 13;
std::fill(std::begin(regularClass->a), std::end(regularClass->a), 137);
*atomicClass = *regularClass;
writer->Fill();
}

auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath());

auto v1 = reader->GetView<std::atomic<std::int64_t>>("atomicInt");
auto v2 = reader->GetView<std::atomic<std::int64_t>>("regularInt");
auto v3 = reader->GetView<std::int64_t>("atomicInt");

auto v4 = reader->GetView<CustomAtomicNotLockFree>("atomicClass");
auto v5 = reader->GetView<std::atomic<CustomAtomicNotLockFree>>("regularClass");

try {
reader->GetView<std::atomic<std::byte>>("atomicInt");
FAIL() << "automatic evolution into an invalid atomic inner type should fail";
} catch (const ROOT::RException &err) {
EXPECT_THAT(err.what(), testing::HasSubstr("incompatible with on-disk field"));
}

try {
reader->GetView<CustomAtomicNotLockFree>("atomicInt");
FAIL() << "automatic evolution into an invalid non-atomic inner type should fail";
} catch (const ROOT::RException &err) {
EXPECT_THAT(err.what(), testing::HasSubstr("incompatible type name for field"));
}

EXPECT_EQ(7, v1(0));
EXPECT_EQ(13, v2(0));
EXPECT_EQ(7, v3(0));

EXPECT_EQ(137, v4(0).a[0]);
EXPECT_EQ(137, v4(0).a[99]);
CustomAtomicNotLockFree tmp = v5(0);
EXPECT_EQ(137, tmp.a[0]);
EXPECT_EQ(137, tmp.a[99]);
}

TEST(RNTupleEvolution, ArrayAsRVec)
{
FileRaii fileGuard("test_ntuple_evolution_array_as_rvec.root");
Expand Down
Loading