From a7e528e073355264343764da40883dfb271020ea Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Mon, 4 Aug 2025 13:39:25 +0200 Subject: [PATCH 1/7] [ntuple] Use GetRenormalizedTypeName also for enum fields I believe this occurrence was missed in commit 705376f934 ("improve type name renormalization") because it was not using the new function GetRenormalizedDemangledTypeName since commit dc4dd713f1 ("Ensure type name given by RField is renormalized"). --- tree/ntuple/inc/ROOT/RField.hxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/ntuple/inc/ROOT/RField.hxx b/tree/ntuple/inc/ROOT/RField.hxx index 0857c99e6eba2..7c0d9c9d808bd 100644 --- a/tree/ntuple/inc/ROOT/RField.hxx +++ b/tree/ntuple/inc/ROOT/RField.hxx @@ -297,7 +297,7 @@ public: template class RField>::type> final : public REnumField { public: - static std::string TypeName() { return ROOT::Internal::GetDemangledTypeName(typeid(T)); } + static std::string TypeName() { return ROOT::Internal::GetRenormalizedTypeName(typeid(T)); } RField(std::string_view name) : REnumField(name, TypeName()) {} RField(RField &&other) = default; RField &operator=(RField &&other) = default; From 0aa70a9b277631b1b8759ed33d0d574d35b2187a Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Mon, 4 Aug 2025 14:04:50 +0200 Subject: [PATCH 2/7] [ntuple] Reduce repeated RNTupleCardinality types --- tree/ntuple/inc/ROOT/RField.hxx | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RField.hxx b/tree/ntuple/inc/ROOT/RField.hxx index 7c0d9c9d808bd..f2fa44099ea58 100644 --- a/tree/ntuple/inc/ROOT/RField.hxx +++ b/tree/ntuple/inc/ROOT/RField.hxx @@ -393,12 +393,14 @@ namespace ROOT { template class RField> final : public RCardinalityField { + using CardinalityType = RNTupleCardinality; + protected: std::unique_ptr CloneImpl(std::string_view newName) const final { - return std::make_unique>>(newName); + return std::make_unique(newName); } - void ConstructValue(void *where) const final { new (where) RNTupleCardinality(0); } + void ConstructValue(void *where) const final { new (where) CardinalityType(0); } /// Get the number of elements of the collection identified by globalIndex void ReadGlobalImpl(ROOT::NTupleSize_t globalIndex, void *to) final @@ -406,7 +408,7 @@ protected: RNTupleLocalIndex collectionStart; ROOT::NTupleSize_t size; fPrincipalColumn->GetCollectionInfo(globalIndex, &collectionStart, &size); - *static_cast *>(to) = size; + *static_cast(to) = size; } /// Get the number of elements of the collection identified by clusterIndex @@ -415,7 +417,7 @@ protected: RNTupleLocalIndex collectionStart; ROOT::NTupleSize_t size; fPrincipalColumn->GetCollectionInfo(localIndex, &collectionStart, &size); - *static_cast *>(to) = size; + *static_cast(to) = size; } std::size_t ReadBulkImpl(const RBulkSpec &bulkSpec) final @@ -424,7 +426,7 @@ protected: ROOT::NTupleSize_t collectionSize; fPrincipalColumn->GetCollectionInfo(bulkSpec.fFirstIndex, &collectionStart, &collectionSize); - auto typedValues = static_cast *>(bulkSpec.fValues); + auto typedValues = static_cast(bulkSpec.fValues); typedValues[0] = collectionSize; auto lastOffset = collectionStart.GetIndexInCluster() + collectionSize; @@ -452,8 +454,8 @@ public: RField &operator=(RField &&other) = default; ~RField() final = default; - size_t GetValueSize() const final { return sizeof(RNTupleCardinality); } - size_t GetAlignment() const final { return alignof(RNTupleCardinality); } + size_t GetValueSize() const final { return sizeof(CardinalityType); } + size_t GetAlignment() const final { return alignof(CardinalityType); } }; /// TObject requires special handling of the fBits and fUniqueID members From 475c25d5703059ee170ff0f2678fc091cfc50864 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Mon, 4 Aug 2025 16:04:43 +0200 Subject: [PATCH 3/7] [ntuple] Make RSimpleField constructor protected Even though it is an abstract class, the constructor taking an arbitrary type should not be public. --- tree/ntuple/inc/ROOT/RField.hxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tree/ntuple/inc/ROOT/RField.hxx b/tree/ntuple/inc/ROOT/RField.hxx index f2fa44099ea58..c8163b64d9d3a 100644 --- a/tree/ntuple/inc/ROOT/RField.hxx +++ b/tree/ntuple/inc/ROOT/RField.hxx @@ -352,12 +352,13 @@ protected: void ConstructValue(void *where) const final { new (where) T{0}; } -public: RSimpleField(std::string_view name, std::string_view type) : RFieldBase(name, type, ROOT::ENTupleStructure::kLeaf, true /* isSimple */) { fTraits |= kTraitTrivialType; } + +public: RSimpleField(RSimpleField &&other) = default; RSimpleField &operator=(RSimpleField &&other) = default; ~RSimpleField() override = default; From f1ddeff5dd0806e5d019ae213a09b617115ebc12 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Fri, 11 Jul 2025 08:43:31 +0200 Subject: [PATCH 4/7] [ntuple] Correct RFieldBase::RValue --- tree/ntuple/doc/Architecture.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree/ntuple/doc/Architecture.md b/tree/ntuple/doc/Architecture.md index 311a24ee2f12d..bac0abc91f8c0 100644 --- a/tree/ntuple/doc/Architecture.md +++ b/tree/ntuple/doc/Architecture.md @@ -187,7 +187,7 @@ During its lifetime, a field undergoes the following possible state transitions: The RField class hierarchy is fixed and not meant to be extended by user classes. -### RField::RValue +### RFieldBase::RValue The `RValue` class makes the connection between an object in memory and the corresponding field used for I/O. It contains a shared pointer of the object, i.e. RNTuple and the application share ownership of objects. The object in an RValue can either be created by an RNTuple field (cf. `RField::CreateValue()` method) From faacaca41be9ecdb797da185f3f5286a9fa9cfdd Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Mon, 4 Aug 2025 20:41:18 +0200 Subject: [PATCH 5/7] [ntuple] Mark RFieldBase::{RValue,RBulkValues} as final User code should not inherit from these. --- tree/ntuple/inc/ROOT/RFieldBase.hxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RFieldBase.hxx b/tree/ntuple/inc/ROOT/RFieldBase.hxx index 90c9ff9b54cb3..ad361789873ed 100644 --- a/tree/ntuple/inc/ROOT/RFieldBase.hxx +++ b/tree/ntuple/inc/ROOT/RFieldBase.hxx @@ -697,7 +697,7 @@ public: /// Points to an object with RNTuple I/O support and keeps a pointer to the corresponding field. /// Fields can create RValue objects through RFieldBase::CreateValue(), RFieldBase::BindValue()) or /// RFieldBase::SplitValue(). -class RFieldBase::RValue { +class RFieldBase::RValue final { friend class RFieldBase; private: @@ -769,7 +769,7 @@ on the same range, where in each read operation a different subset of values is The memory of the value array is managed by the RBulkValues class. */ // clang-format on -class RFieldBase::RBulkValues { +class RFieldBase::RBulkValues final { private: friend class RFieldBase; From 987ad1f3569d4aefe95f47316ed4c8a4ce5aa7c0 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Mon, 4 Aug 2025 20:48:35 +0200 Subject: [PATCH 6/7] [ntuple] Make RValue::Append private Only REntry should call this for RValues where the RFieldBase is connected to a RPageSink, and orchestrate the calls across all top-level fields. --- tree/ntuple/inc/ROOT/RFieldBase.hxx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tree/ntuple/inc/ROOT/RFieldBase.hxx b/tree/ntuple/inc/ROOT/RFieldBase.hxx index ad361789873ed..404ffc85ddd8a 100644 --- a/tree/ntuple/inc/ROOT/RFieldBase.hxx +++ b/tree/ntuple/inc/ROOT/RFieldBase.hxx @@ -30,6 +30,7 @@ namespace ROOT { +class REntry; class RFieldBase; class RClassField; @@ -699,6 +700,7 @@ public: /// RFieldBase::SplitValue(). class RFieldBase::RValue final { friend class RFieldBase; + friend class ROOT::REntry; private: RFieldBase *fField = nullptr; ///< The field that created the RValue @@ -713,9 +715,13 @@ public: RValue &operator=(RValue &&other) = default; ~RValue() = default; +private: std::size_t Append() { return fField->Append(fObjPtr.get()); } + +public: void Read(ROOT::NTupleSize_t globalIndex) { fField->Read(globalIndex, fObjPtr.get()); } void Read(RNTupleLocalIndex localIndex) { fField->Read(localIndex, fObjPtr.get()); } + void Bind(std::shared_ptr objPtr) { fObjPtr = objPtr; } void BindRawPtr(void *rawPtr); /// Replace the current object pointer by a pointer to a new object constructed by the field From f917fcaa8f5e6a4888a633b6d275c1659ba9218c Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Mon, 4 Aug 2025 23:03:58 +0200 Subject: [PATCH 7/7] [ntuple] Move comment about void specializations --- tree/ntuple/inc/ROOT/RField.hxx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RField.hxx b/tree/ntuple/inc/ROOT/RField.hxx index c8163b64d9d3a..b1acdfc1bbdd2 100644 --- a/tree/ntuple/inc/ROOT/RField.hxx +++ b/tree/ntuple/inc/ROOT/RField.hxx @@ -498,8 +498,7 @@ public: void AcceptVisitor(ROOT::Detail::RFieldVisitor &visitor) const final; }; -// Has to be implemented after the definition of all RField types -// The void type is specialized in RField.cxx +// Have to be implemented after the definition of all RField types namespace Internal { @@ -530,6 +529,8 @@ std::unique_ptr::deleter> RField return std::unique_ptr(static_cast(CreateObjectRawPtr())); } +// The void type is specialized in RField.cxx + template <> struct RFieldBase::RCreateObjectDeleter { using deleter = RCreateObjectDeleter;