From a78d82802ca6b6105d8bf2d712cd568335525b02 Mon Sep 17 00:00:00 2001 From: JasMehta08 Date: Mon, 8 Jun 2026 19:55:47 +0530 Subject: [PATCH] [ntuple] Add RNTupleLocatorMulti class for kTypeMulti locator --- tree/ntuple/inc/ROOT/RNTupleTypes.hxx | 32 +++++++++++- tree/ntuple/src/RNTupleSerialize.cxx | 48 ++++++++++++++++- tree/ntuple/src/RNTupleTypes.cxx | 21 +++++++- tree/ntuple/test/ntuple_serialize.cxx | 74 +++++++++++++++++++++++++-- tree/ntuple/test/ntuple_test.hxx | 1 + 5 files changed, 168 insertions(+), 8 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RNTupleTypes.hxx b/tree/ntuple/inc/ROOT/RNTupleTypes.hxx index cbfb76c96b340..6ced44d73dfce 100644 --- a/tree/ntuple/inc/ROOT/RNTupleTypes.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleTypes.hxx @@ -201,6 +201,28 @@ public: std::uint64_t GetLocation() const { return fLocation; } }; +/// RNTupleLocator payload for the kTypeMulti locator (type 0x03). Used by storage +/// backends that pack multiple pages into shared objects (e.g., S3 Mode A). The two +/// 32-bit fields are written to disk as separate integers, so the wire format is +/// directly interpretable without knowledge of any internal bit packing. +class RNTupleLocatorMulti { +private: + std::uint32_t fObjectId = 0; + std::uint32_t fOffset = 0; + +public: + RNTupleLocatorMulti() = default; + RNTupleLocatorMulti(std::uint32_t objectId, std::uint32_t offset) : fObjectId(objectId), fOffset(offset) {} + + bool operator==(const RNTupleLocatorMulti &other) const + { + return fObjectId == other.fObjectId && fOffset == other.fOffset; + } + + std::uint32_t GetObjectId() const { return fObjectId; } + std::uint32_t GetOffset() const { return fOffset; } +}; + // Workaround missing return type overloading class RNTupleLocator; namespace Internal { @@ -216,6 +238,11 @@ template <> struct RNTupleLocatorHelper { static RNTupleLocatorObject64 Get(const RNTupleLocator &loc); }; + +template <> +struct RNTupleLocatorHelper { + static RNTupleLocatorMulti Get(const RNTupleLocator &loc); +}; } // namespace Internal /// Generic information about the physical location of data. Values depend on the concrete storage type. E.g., @@ -226,6 +253,7 @@ struct RNTupleLocatorHelper { class RNTupleLocator { friend struct Internal::RNTupleLocatorHelper; friend struct Internal::RNTupleLocatorHelper; + friend struct Internal::RNTupleLocatorHelper; public: /// Values for the _Type_ field in non-disk locators. Serializable types must have the MSb == 0; see @@ -277,7 +305,8 @@ public: void SetType(ELocatorType type); void SetReserved(std::uint8_t reserved); - /// Note that for GetPosition() / SetPosition(), the locator type must correspond (kTypeFile, kTypeObject64). + /// Note that for GetPosition() / SetPosition(), the locator type must correspond + /// (kTypeFile, kTypeObject64, kTypeMulti). template T GetPosition() const @@ -287,6 +316,7 @@ public: void SetPosition(std::uint64_t position); void SetPosition(RNTupleLocatorObject64 position); + void SetPosition(RNTupleLocatorMulti position); }; namespace Internal { diff --git a/tree/ntuple/src/RNTupleSerialize.cxx b/tree/ntuple/src/RNTupleSerialize.cxx index 77a63f863c3a0..0405b1bb2b72e 100644 --- a/tree/ntuple/src/RNTupleSerialize.cxx +++ b/tree/ntuple/src/RNTupleSerialize.cxx @@ -511,6 +511,50 @@ ROOT::RResult DeserializeLocatorPayloadObject64(const unsigned char *buffe return ROOT::RResult::Success(); } +std::uint32_t SerializeLocatorPayloadMulti(const ROOT::RNTupleLocator &locator, unsigned char *buffer) +{ + const auto &data = locator.GetPosition(); + const uint32_t sizeofNBytesOnStorage = (locator.GetNBytesOnStorage() > std::numeric_limits::max()) + ? sizeof(std::uint64_t) + : sizeof(std::uint32_t); + if (buffer) { + if (sizeofNBytesOnStorage == sizeof(std::uint32_t)) { + RNTupleSerializer::SerializeUInt32(locator.GetNBytesOnStorage(), buffer); + } else { + RNTupleSerializer::SerializeUInt64(locator.GetNBytesOnStorage(), buffer); + } + RNTupleSerializer::SerializeUInt32(data.GetObjectId(), buffer + sizeofNBytesOnStorage); + RNTupleSerializer::SerializeUInt32(data.GetOffset(), + buffer + sizeofNBytesOnStorage + sizeof(std::uint32_t)); + } + return sizeofNBytesOnStorage + 2 * sizeof(std::uint32_t); +} + +ROOT::RResult DeserializeLocatorPayloadMulti(const unsigned char *buffer, std::uint32_t sizeofLocatorPayload, + ROOT::RNTupleLocator &locator) +{ + std::uint32_t sizeofNBytesOnStorage; + if (sizeofLocatorPayload == 12) { + std::uint32_t nBytesOnStorage; + RNTupleSerializer::DeserializeUInt32(buffer, nBytesOnStorage); + locator.SetNBytesOnStorage(nBytesOnStorage); + sizeofNBytesOnStorage = sizeof(std::uint32_t); + } else if (sizeofLocatorPayload == 16) { + std::uint64_t nBytesOnStorage; + RNTupleSerializer::DeserializeUInt64(buffer, nBytesOnStorage); + locator.SetNBytesOnStorage(nBytesOnStorage); + sizeofNBytesOnStorage = sizeof(std::uint64_t); + } else { + return R__FAIL("invalid Multi locator payload size: " + std::to_string(sizeofLocatorPayload)); + } + std::uint32_t objectId; + std::uint32_t offset; + RNTupleSerializer::DeserializeUInt32(buffer + sizeofNBytesOnStorage, objectId); + RNTupleSerializer::DeserializeUInt32(buffer + sizeofNBytesOnStorage + sizeof(std::uint32_t), offset); + locator.SetPosition(ROOT::RNTupleLocatorMulti(objectId, offset)); + return ROOT::RResult::Success(); +} + std::uint32_t SerializeAliasColumn(const ROOT::RColumnDescriptor &columnDesc, const ROOT::Internal::RNTupleSerializer::RContext &context, void *buffer) { @@ -1091,7 +1135,7 @@ ROOT::Internal::RNTupleSerializer::SerializeLocator(const RNTupleLocator &locato locatorType = 0x02; break; case RNTupleLocator::kTypeMulti: - size += SerializeLocatorPayloadObject64(locator, payloadp); + size += SerializeLocatorPayloadMulti(locator, payloadp); locatorType = 0x03; break; default: @@ -1144,7 +1188,7 @@ ROOT::RResult ROOT::Internal::RNTupleSerializer::DeserializeLocat break; case 0x03: locator.SetType(RNTupleLocator::kTypeMulti); - DeserializeLocatorPayloadObject64(bytes, payloadSize, locator); + DeserializeLocatorPayloadMulti(bytes, payloadSize, locator); break; default: locator.SetType(RNTupleLocator::kTypeUnknown); } diff --git a/tree/ntuple/src/RNTupleTypes.cxx b/tree/ntuple/src/RNTupleTypes.cxx index 62e31d289e6ef..2f8b5837b4c32 100644 --- a/tree/ntuple/src/RNTupleTypes.cxx +++ b/tree/ntuple/src/RNTupleTypes.cxx @@ -80,11 +80,18 @@ void ROOT::RNTupleLocator::SetPosition(std::uint64_t position) void ROOT::RNTupleLocator::SetPosition(RNTupleLocatorObject64 position) { - if (GetType() != kTypeObject64 && GetType() != kTypeMulti) + if (GetType() != kTypeObject64) throw RException(R__FAIL("cannot set position as 64bit object for type " + std::to_string(GetType()))); fPosition = position.GetLocation(); } +void ROOT::RNTupleLocator::SetPosition(RNTupleLocatorMulti position) +{ + if (GetType() != kTypeMulti) + throw RException(R__FAIL("cannot set position as Multi locator for type " + std::to_string(GetType()))); + fPosition = (static_cast(position.GetObjectId()) << 32) | position.GetOffset(); +} + std::uint64_t ROOT::Internal::RNTupleLocatorHelper::Get(const RNTupleLocator &loc) { if (loc.GetType() != ROOT::RNTupleLocator::kTypeFile) @@ -95,7 +102,17 @@ std::uint64_t ROOT::Internal::RNTupleLocatorHelper::Get(const RNT ROOT::RNTupleLocatorObject64 ROOT::Internal::RNTupleLocatorHelper::Get(const RNTupleLocator &loc) { - if (loc.GetType() != ROOT::RNTupleLocator::kTypeObject64 && loc.GetType() != ROOT::RNTupleLocator::kTypeMulti) + if (loc.GetType() != ROOT::RNTupleLocator::kTypeObject64) throw RException(R__FAIL("cannot retrieve position as 64bit object for type " + std::to_string(loc.GetType()))); return RNTupleLocatorObject64{loc.fPosition}; } + +ROOT::RNTupleLocatorMulti +ROOT::Internal::RNTupleLocatorHelper::Get(const RNTupleLocator &loc) +{ + if (loc.GetType() != ROOT::RNTupleLocator::kTypeMulti) + throw RException( + R__FAIL("cannot retrieve position as Multi locator for type " + std::to_string(loc.GetType()))); + return RNTupleLocatorMulti(static_cast(loc.fPosition >> 32), + static_cast(loc.fPosition)); +} diff --git a/tree/ntuple/test/ntuple_serialize.cxx b/tree/ntuple/test/ntuple_serialize.cxx index 5a6acfb1cfc4b..92cf21953925d 100644 --- a/tree/ntuple/test/ntuple_serialize.cxx +++ b/tree/ntuple/test/ntuple_serialize.cxx @@ -402,7 +402,7 @@ TEST(RNTuple, SerializeLocator) // Multi locator round-trip with 32-bit nBytesOnStorage locator = RNTupleLocator{}; locator.SetType(RNTupleLocator::kTypeMulti); - locator.SetPosition(RNTupleLocatorObject64{42U}); + locator.SetPosition(RNTupleLocatorMulti{7, 1024}); locator.SetNBytesOnStorage(1024U); locator.SetReserved(0); EXPECT_EQ(16u, RNTupleSerializer::SerializeLocator(locator, buffer).Unwrap()); @@ -411,7 +411,9 @@ TEST(RNTuple, SerializeLocator) EXPECT_EQ(locator.GetType(), RNTupleLocator::kTypeMulti); EXPECT_EQ(locator.GetNBytesOnStorage(), 1024U); EXPECT_EQ(locator.GetReserved(), 0); - EXPECT_EQ(42U, locator.GetPosition().GetLocation()); + auto multi = locator.GetPosition(); + EXPECT_EQ(7U, multi.GetObjectId()); + EXPECT_EQ(1024U, multi.GetOffset()); // Multi locator round-trip with 64-bit nBytesOnStorage and reserved bit locator.SetNBytesOnStorage(static_cast(std::numeric_limits::max()) + 1); @@ -422,7 +424,9 @@ TEST(RNTuple, SerializeLocator) EXPECT_EQ(locator.GetType(), RNTupleLocator::kTypeMulti); EXPECT_EQ(locator.GetNBytesOnStorage(), static_cast(std::numeric_limits::max()) + 1); EXPECT_EQ(locator.GetReserved(), 1); - EXPECT_EQ(42U, locator.GetPosition().GetLocation()); + multi = locator.GetPosition(); + EXPECT_EQ(7U, multi.GetObjectId()); + EXPECT_EQ(1024U, multi.GetOffset()); std::int32_t *head = reinterpret_cast(buffer); #ifndef R__BYTESWAP @@ -435,6 +439,70 @@ TEST(RNTuple, SerializeLocator) EXPECT_EQ(locator.GetType(), RNTupleLocator::kTypeUnknown); } +TEST(RNTuple, RNTupleLocatorMultiFields) +{ + // Default-constructed: both fields zero + RNTupleLocatorMulti zero; + EXPECT_EQ(0U, zero.GetObjectId()); + EXPECT_EQ(0U, zero.GetOffset()); + + // Non-trivial values + RNTupleLocatorMulti m(0x12345, 0xABCDE); + EXPECT_EQ(0x12345U, m.GetObjectId()); + EXPECT_EQ(0xABCDEU, m.GetOffset()); + + // Max 32-bit values: both fields use the full uint32 range + RNTupleLocatorMulti maxVals(0xFFFFFFFFU, 0xFFFFFFFFU); + EXPECT_EQ(0xFFFFFFFFU, maxVals.GetObjectId()); + EXPECT_EQ(0xFFFFFFFFU, maxVals.GetOffset()); + + // Object id only: no leak into offset + RNTupleLocatorMulti idOnly(0xFFFFFFFFU, 0U); + EXPECT_EQ(0xFFFFFFFFU, idOnly.GetObjectId()); + EXPECT_EQ(0U, idOnly.GetOffset()); + + // Offset only: no leak into object id + RNTupleLocatorMulti offsetOnly(0U, 0xFFFFFFFFU); + EXPECT_EQ(0U, offsetOnly.GetObjectId()); + EXPECT_EQ(0xFFFFFFFFU, offsetOnly.GetOffset()); + + // Equality semantics + EXPECT_EQ(RNTupleLocatorMulti(1, 2), RNTupleLocatorMulti(1, 2)); + EXPECT_FALSE(RNTupleLocatorMulti(1, 2) == RNTupleLocatorMulti(1, 3)); + EXPECT_FALSE(RNTupleLocatorMulti(1, 2) == RNTupleLocatorMulti(2, 2)); +} + +TEST(RNTuple, RNTupleLocatorMultiTypeEnforcement) +{ + RNTupleLocator locator; + + // SetPosition(Multi) requires kTypeMulti. + locator.SetType(RNTupleLocator::kTypeObject64); + EXPECT_THROW(locator.SetPosition(RNTupleLocatorMulti(1, 2)), ROOT::RException); + locator.SetType(RNTupleLocator::kTypeFile); + EXPECT_THROW(locator.SetPosition(RNTupleLocatorMulti(1, 2)), ROOT::RException); + + // SetPosition(Object64) no longer accepts kTypeMulti after the split. + locator = RNTupleLocator{}; + locator.SetType(RNTupleLocator::kTypeMulti); + EXPECT_THROW(locator.SetPosition(RNTupleLocatorObject64{1}), ROOT::RException); + + // Correct usage round-trip. + locator.SetPosition(RNTupleLocatorMulti(1, 2)); + auto m = locator.GetPosition(); + EXPECT_EQ(1U, m.GetObjectId()); + EXPECT_EQ(2U, m.GetOffset()); + + // GetPosition() no longer accepts kTypeMulti after the split. + EXPECT_THROW(locator.GetPosition(), ROOT::RException); + + // GetPosition() rejects non-Multi types. + locator = RNTupleLocator{}; + locator.SetType(RNTupleLocator::kTypeObject64); + locator.SetPosition(RNTupleLocatorObject64{1}); + EXPECT_THROW(locator.GetPosition(), ROOT::RException); +} + TEST(RNTuple, SerializeEnvelopeLink) { RNTupleSerializer::REnvelopeLink link; diff --git a/tree/ntuple/test/ntuple_test.hxx b/tree/ntuple/test/ntuple_test.hxx index e4cd028af99f1..8c1b737b17413 100644 --- a/tree/ntuple/test/ntuple_test.hxx +++ b/tree/ntuple/test/ntuple_test.hxx @@ -57,6 +57,7 @@ using ROOT::EExtraTypeInfoIds; using ROOT::RNTupleLocalIndex; using ROOT::RNTupleLocator; +using ROOT::RNTupleLocatorMulti; using ROOT::RNTupleLocatorObject64; using ROOT::Internal::RColumnIndex; using RClusterDescriptor = ROOT::RClusterDescriptor;