Skip to content

Commit b040221

Browse files
silverweedsilverweed
committed
[ntuple] Move RPageInfo(Extended) up one level of nesting
They are now inner classes of RClusterDescriptor directly rather than of RClusterDescriptor::RPageRange
1 parent ec3c112 commit b040221

7 files changed

+81
-78
lines changed

Diff for: ‎tree/ntuple/v7/inc/ROOT/RNTupleDescriptor.hxx

+62-57
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,68 @@ public:
307307
}
308308
};
309309

310+
// clang-format off
311+
/**
312+
\class ROOT::Experimental::RClusterDescriptor::RPageInfo
313+
\ingroup NTuple
314+
\brief Information about a single page in the context of a cluster's page range.
315+
*/
316+
// clang-format on
317+
// NOTE: We do not need to store the element size / uncompressed page size because we know to which column
318+
// the page belongs
319+
struct RPageInfo {
320+
protected:
321+
/// The sum of the elements of all the pages must match the corresponding fNElements field in fColumnRanges
322+
std::uint32_t fNElements = std::uint32_t(-1);
323+
/// The meaning of fLocator depends on the storage backend.
324+
RNTupleLocator fLocator;
325+
/// If true, the 8 bytes following the serialized page are an xxhash of the on-disk page data
326+
bool fHasChecksum = false;
327+
328+
public:
329+
RPageInfo() = default;
330+
RPageInfo(std::uint32_t nElements, const RNTupleLocator &locator, bool hasChecksum)
331+
: fNElements(nElements), fLocator(locator), fHasChecksum(hasChecksum)
332+
{
333+
}
334+
335+
bool operator==(const RPageInfo &other) const
336+
{
337+
return fNElements == other.fNElements && fLocator == other.fLocator;
338+
}
339+
340+
std::uint32_t GetNElements() const { return fNElements; }
341+
void SetNElements(std::uint32_t n) { fNElements = n; }
342+
343+
const RNTupleLocator &GetLocator() const { return fLocator; }
344+
RNTupleLocator &GetLocator() { return fLocator; }
345+
void SetLocator(const RNTupleLocator &locator) { fLocator = locator; }
346+
347+
bool HasChecksum() const { return fHasChecksum; }
348+
void SetHasChecksum(bool hasChecksum) { fHasChecksum = hasChecksum; }
349+
};
350+
351+
struct RPageInfoExtended : RPageInfo {
352+
private:
353+
/// Index (in cluster) of the first element in page.
354+
ROOT::NTupleSize_t fFirstElementIndex = 0;
355+
/// Page number in the corresponding RPageRange.
356+
ROOT::NTupleSize_t fPageNumber = 0;
357+
358+
public:
359+
RPageInfoExtended() = default;
360+
RPageInfoExtended(const RPageInfo &pageInfo, ROOT::NTupleSize_t firstElementIndex, ROOT::NTupleSize_t pageNumber)
361+
: RPageInfo(pageInfo), fFirstElementIndex(firstElementIndex), fPageNumber(pageNumber)
362+
{
363+
}
364+
365+
ROOT::NTupleSize_t GetFirstElementIndex() const { return fFirstElementIndex; }
366+
void SetFirstElementIndex(ROOT::NTupleSize_t firstInPage) { fFirstElementIndex = firstInPage; }
367+
368+
ROOT::NTupleSize_t GetPageNumber() const { return fPageNumber; }
369+
void SetPageNumber(ROOT::NTupleSize_t pageNumber) { fPageNumber = pageNumber; }
370+
};
371+
310372
// clang-format off
311373
/**
312374
\class ROOT::Experimental::RClusterDescriptor::RPageRange
@@ -317,63 +379,6 @@ public:
317379
class RPageRange {
318380
friend class Internal::RClusterDescriptorBuilder;
319381

320-
public:
321-
/// We do not need to store the element size / uncompressed page size because we know to which column
322-
/// the page belongs
323-
struct RPageInfo {
324-
protected:
325-
/// The sum of the elements of all the pages must match the corresponding fNElements field in fColumnRanges
326-
std::uint32_t fNElements = std::uint32_t(-1);
327-
/// The meaning of fLocator depends on the storage backend.
328-
RNTupleLocator fLocator;
329-
/// If true, the 8 bytes following the serialized page are an xxhash of the on-disk page data
330-
bool fHasChecksum = false;
331-
332-
public:
333-
RPageInfo() = default;
334-
RPageInfo(std::uint32_t nElements, const RNTupleLocator &locator, bool hasChecksum)
335-
: fNElements(nElements), fLocator(locator), fHasChecksum(hasChecksum)
336-
{
337-
}
338-
339-
bool operator==(const RPageInfo &other) const
340-
{
341-
return fNElements == other.fNElements && fLocator == other.fLocator;
342-
}
343-
344-
std::uint32_t GetNElements() const { return fNElements; }
345-
void SetNElements(std::uint32_t n) { fNElements = n; }
346-
347-
const RNTupleLocator &GetLocator() const { return fLocator; }
348-
RNTupleLocator &GetLocator() { return fLocator; }
349-
void SetLocator(const RNTupleLocator &locator) { fLocator = locator; }
350-
351-
bool HasChecksum() const { return fHasChecksum; }
352-
void SetHasChecksum(bool hasChecksum) { fHasChecksum = hasChecksum; }
353-
};
354-
355-
struct RPageInfoExtended : RPageInfo {
356-
private:
357-
/// Index (in cluster) of the first element in page.
358-
ROOT::NTupleSize_t fFirstElementIndex = 0;
359-
/// Page number in the corresponding RPageRange.
360-
ROOT::NTupleSize_t fPageNumber = 0;
361-
362-
public:
363-
RPageInfoExtended() = default;
364-
RPageInfoExtended(const RPageInfo &pageInfo, ROOT::NTupleSize_t firstElementIndex,
365-
ROOT::NTupleSize_t pageNumber)
366-
: RPageInfo(pageInfo), fFirstElementIndex(firstElementIndex), fPageNumber(pageNumber)
367-
{
368-
}
369-
370-
ROOT::NTupleSize_t GetFirstElementIndex() const { return fFirstElementIndex; }
371-
void SetFirstElementIndex(ROOT::NTupleSize_t firstInPage) { fFirstElementIndex = firstInPage; }
372-
373-
ROOT::NTupleSize_t GetPageNumber() const { return fPageNumber; }
374-
void SetPageNumber(ROOT::NTupleSize_t pageNumber) { fPageNumber = pageNumber; }
375-
};
376-
377382
private:
378383
/// Extend this RPageRange to fit the given RColumnRange, i.e. prepend as many synthetic RPageInfos as needed to
379384
/// cover the range in `columnRange`. `RPageInfo`s are constructed to contain as many elements of type `element`

Diff for: ‎tree/ntuple/v7/inc/ROOT/RPageStorage.hxx

+2-3
Original file line numberDiff line numberDiff line change
@@ -688,7 +688,7 @@ protected:
688688
struct RClusterInfo {
689689
ROOT::DescriptorId_t fClusterId = 0;
690690
/// Location of the page on disk
691-
RClusterDescriptor::RPageRange::RPageInfoExtended fPageInfo;
691+
RClusterDescriptor::RPageInfoExtended fPageInfo;
692692
/// The first element number of the page's column in the given cluster
693693
std::uint64_t fColumnOffset = 0;
694694
};
@@ -718,8 +718,7 @@ protected:
718718
/// commonly used as part of `LoadClusters()` in derived classes.
719719
void PrepareLoadCluster(
720720
const RCluster::RKey &clusterKey, ROnDiskPageMap &pageZeroMap,
721-
std::function<void(ROOT::DescriptorId_t, ROOT::NTupleSize_t, const RClusterDescriptor::RPageRange::RPageInfo &)>
722-
perPageFunc);
721+
std::function<void(ROOT::DescriptorId_t, ROOT::NTupleSize_t, const RClusterDescriptor::RPageInfo &)> perPageFunc);
723722

724723
/// Enables the default set of metrics provided by RPageSource. `prefix` will be used as the prefix for
725724
/// the counters registered in the internal RNTupleMetrics object.

Diff for: ‎tree/ntuple/v7/src/RNTupleDescriptor.cxx

+1-1
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ ROOT::Experimental::RColumnDescriptor ROOT::Experimental::RColumnDescriptor::Clo
194194

195195
////////////////////////////////////////////////////////////////////////////////
196196

197-
ROOT::Experimental::RClusterDescriptor::RPageRange::RPageInfoExtended
197+
ROOT::Experimental::RClusterDescriptor::RPageInfoExtended
198198
ROOT::Experimental::RClusterDescriptor::RPageRange::Find(ROOT::NTupleSize_t idxInCluster) const
199199
{
200200
const auto N = fCumulativeNElements.size();

Diff for: ‎tree/ntuple/v7/src/RPageStorage.cxx

+4-5
Original file line numberDiff line numberDiff line change
@@ -314,8 +314,7 @@ void ROOT::Experimental::Internal::RPageSource::UnzipClusterImpl(RCluster *clust
314314

315315
void ROOT::Experimental::Internal::RPageSource::PrepareLoadCluster(
316316
const RCluster::RKey &clusterKey, ROnDiskPageMap &pageZeroMap,
317-
std::function<void(ROOT::DescriptorId_t, ROOT::NTupleSize_t, const RClusterDescriptor::RPageRange::RPageInfo &)>
318-
perPageFunc)
317+
std::function<void(ROOT::DescriptorId_t, ROOT::NTupleSize_t, const RClusterDescriptor::RPageInfo &)> perPageFunc)
319318
{
320319
auto descriptorGuard = GetSharedDescriptorGuard();
321320
const auto &clusterDesc = descriptorGuard->GetClusterDescriptor(clusterKey.fClusterId);
@@ -1012,7 +1011,7 @@ void ROOT::Experimental::Internal::RPagePersistentSink::CommitPage(ColumnHandle_
10121011
{
10131012
fOpenColumnRanges.at(columnHandle.fPhysicalId).IncrementNElements(page.GetNElements());
10141013

1015-
RClusterDescriptor::RPageRange::RPageInfo pageInfo;
1014+
RClusterDescriptor::RPageInfo pageInfo;
10161015
pageInfo.SetNElements(page.GetNElements());
10171016
pageInfo.SetLocator(CommitPageImpl(columnHandle, page));
10181017
pageInfo.SetHasChecksum(GetWriteOptions().GetEnablePageChecksums());
@@ -1024,7 +1023,7 @@ void ROOT::Experimental::Internal::RPagePersistentSink::CommitSealedPage(ROOT::D
10241023
{
10251024
fOpenColumnRanges.at(physicalColumnId).IncrementNElements(sealedPage.GetNElements());
10261025

1027-
RClusterDescriptor::RPageRange::RPageInfo pageInfo;
1026+
RClusterDescriptor::RPageInfo pageInfo;
10281027
pageInfo.SetNElements(sealedPage.GetNElements());
10291028
pageInfo.SetLocator(CommitSealedPageImpl(physicalColumnId, sealedPage));
10301029
pageInfo.SetHasChecksum(sealedPage.GetHasChecksum());
@@ -1108,7 +1107,7 @@ void ROOT::Experimental::Internal::RPagePersistentSink::CommitSealedPageV(
11081107
for (auto sealedPageIt = range.fFirst; sealedPageIt != range.fLast; ++sealedPageIt) {
11091108
fOpenColumnRanges.at(range.fPhysicalColumnId).IncrementNElements(sealedPageIt->GetNElements());
11101109

1111-
RClusterDescriptor::RPageRange::RPageInfo pageInfo;
1110+
RClusterDescriptor::RPageInfo pageInfo;
11121111
pageInfo.SetNElements(sealedPageIt->GetNElements());
11131112
pageInfo.SetLocator(locators[locatorIndexes[i++]]);
11141113
pageInfo.SetHasChecksum(sealedPageIt->GetHasChecksum());

Diff for: ‎tree/ntuple/v7/src/RPageStorageDaos.cxx

+2-2
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,7 @@ void ROOT::Experimental::Internal::RPageSourceDaos::LoadSealedPage(ROOT::Descrip
550550
{
551551
const auto clusterId = localIndex.GetClusterId();
552552

553-
RClusterDescriptor::RPageRange::RPageInfo pageInfo;
553+
RClusterDescriptor::RPageInfo pageInfo;
554554
{
555555
auto descriptorGuard = GetSharedDescriptorGuard();
556556
const auto &clusterDescriptor = descriptorGuard->GetClusterDescriptor(clusterId);
@@ -699,7 +699,7 @@ ROOT::Experimental::Internal::RPageSourceDaos::LoadClusters(std::span<RCluster::
699699
PrepareLoadCluster(
700700
clusterKey, *pageZeroMap,
701701
[&](ROOT::DescriptorId_t physicalColumnId, ROOT::NTupleSize_t pageNo,
702-
const RClusterDescriptor::RPageRange::RPageInfo &pageInfo) {
702+
const RClusterDescriptor::RPageInfo &pageInfo) {
703703
const auto &pageLocator = pageInfo.GetLocator();
704704
uint32_t position, offset;
705705
std::tie(position, offset) = DecodeDaosPagePosition(pageLocator.GetPosition<RNTupleLocatorObject64>());

Diff for: ‎tree/ntuple/v7/src/RPageStorageFile.cxx

+2-2
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ void ROOT::Experimental::Internal::RPageSourceFile::LoadSealedPage(ROOT::Descrip
397397
{
398398
const auto clusterId = localIndex.GetClusterId();
399399

400-
RClusterDescriptor::RPageRange::RPageInfo pageInfo;
400+
RClusterDescriptor::RPageInfo pageInfo;
401401
{
402402
auto descriptorGuard = GetSharedDescriptorGuard();
403403
const auto &clusterDescriptor = descriptorGuard->GetClusterDescriptor(clusterId);
@@ -514,7 +514,7 @@ ROOT::Experimental::Internal::RPageSourceFile::PrepareSingleCluster(
514514
auto pageZeroMap = std::make_unique<ROnDiskPageMap>();
515515
PrepareLoadCluster(clusterKey, *pageZeroMap,
516516
[&](ROOT::DescriptorId_t physicalColumnId, ROOT::NTupleSize_t pageNo,
517-
const RClusterDescriptor::RPageRange::RPageInfo &pageInfo) {
517+
const RClusterDescriptor::RPageInfo &pageInfo) {
518518
const auto &pageLocator = pageInfo.GetLocator();
519519
if (pageLocator.GetType() == RNTupleLocator::kTypeUnknown)
520520
throw RException(R__FAIL("tried to read a page with an unknown locator"));

Diff for: ‎tree/ntuple/v7/test/ntuple_serialize.cxx

+8-8
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,7 @@ TEST(RNTuple, SerializeFooter)
672672
.Unwrap());
673673

674674
ROOT::Experimental::RClusterDescriptor::RColumnRange columnRange;
675-
ROOT::Experimental::RClusterDescriptor::RPageRange::RPageInfo pageInfo;
675+
ROOT::Experimental::RClusterDescriptor::RPageInfo pageInfo;
676676
RClusterDescriptorBuilder clusterBuilder;
677677
clusterBuilder.ClusterId(84).FirstEntryIndex(0).NEntries(100);
678678
ROOT::Experimental::RClusterDescriptor::RPageRange pageRange;
@@ -994,7 +994,7 @@ TEST(RNTuple, SerializeMultiColumnRepresentation)
994994

995995
RClusterDescriptorBuilder clusterBuilder;
996996
ROOT::Experimental::RClusterDescriptor::RPageRange pageRange;
997-
ROOT::Experimental::RClusterDescriptor::RPageRange::RPageInfo pageInfo;
997+
ROOT::Experimental::RClusterDescriptor::RPageInfo pageInfo;
998998
// First cluster
999999
clusterBuilder.ClusterId(13).FirstEntryIndex(0).NEntries(1);
10001000
clusterBuilder.MarkSuppressedColumnRange(0);
@@ -1190,7 +1190,7 @@ TEST(RNTuple, SerializeMultiColumnRepresentationProjection)
11901190

11911191
RClusterDescriptorBuilder clusterBuilder;
11921192
ROOT::Experimental::RClusterDescriptor::RPageRange pageRange;
1193-
ROOT::Experimental::RClusterDescriptor::RPageRange::RPageInfo pageInfo;
1193+
ROOT::Experimental::RClusterDescriptor::RPageInfo pageInfo;
11941194
// First cluster
11951195
clusterBuilder.ClusterId(13).FirstEntryIndex(0).NEntries(1);
11961196
clusterBuilder.MarkSuppressedColumnRange(0);
@@ -1315,7 +1315,7 @@ TEST(RNTuple, SerializeMultiColumnRepresentationDeferred)
13151315
context.MapSchema(builder.GetDescriptor(), /*forHeaderExtension=*/true);
13161316

13171317
ROOT::Experimental::RClusterDescriptor::RPageRange pageRange;
1318-
ROOT::Experimental::RClusterDescriptor::RPageRange::RPageInfo pageInfo;
1318+
ROOT::Experimental::RClusterDescriptor::RPageInfo pageInfo;
13191319
// Second cluster
13201320
clusterBuilder.ClusterId(17).FirstEntryIndex(1).NEntries(2);
13211321
clusterBuilder.MarkSuppressedColumnRange(1);
@@ -1425,7 +1425,7 @@ TEST(RNTuple, SerializeMultiColumnRepresentationIncremental)
14251425
// First cluster
14261426
RClusterDescriptorBuilder clusterBuilder;
14271427
ROOT::Experimental::RClusterDescriptor::RPageRange pageRange;
1428-
ROOT::Experimental::RClusterDescriptor::RPageRange::RPageInfo pageInfo;
1428+
ROOT::Experimental::RClusterDescriptor::RPageInfo pageInfo;
14291429
clusterBuilder.ClusterId(13).FirstEntryIndex(0).NEntries(1);
14301430
pageRange.SetPhysicalColumnId(0);
14311431
pageInfo.SetNElements(1);
@@ -1558,7 +1558,7 @@ TEST(RNTuple, DeserializeDescriptorModes)
15581558
sizeHeader = context.GetHeaderSize();
15591559

15601560
ROOT::Experimental::RClusterDescriptor::RPageRange pageRange;
1561-
ROOT::Experimental::RClusterDescriptor::RPageRange::RPageInfo pageInfo;
1561+
ROOT::Experimental::RClusterDescriptor::RPageInfo pageInfo;
15621562

15631563
// First cluster
15641564
RClusterDescriptorBuilder clusterBuilder;
@@ -1833,7 +1833,7 @@ TEST(RNTuple, SerializeMultiColumnRepresentationDeferred_HeaderExtBeforeSerializ
18331833
builder.AddCluster(clusterBuilder.MoveDescriptor().Unwrap());
18341834

18351835
ROOT::Experimental::RClusterDescriptor::RPageRange pageRange;
1836-
ROOT::Experimental::RClusterDescriptor::RPageRange::RPageInfo pageInfo;
1836+
ROOT::Experimental::RClusterDescriptor::RPageInfo pageInfo;
18371837
// Second cluster
18381838
clusterBuilder.ClusterId(17).FirstEntryIndex(1).NEntries(2);
18391839
clusterBuilder.MarkSuppressedColumnRange(1);
@@ -1959,7 +1959,7 @@ TEST(RNTuple, SerializeMultiColumnRepresentationDeferredInMainHeader)
19591959
builder.AddCluster(clusterBuilder.MoveDescriptor().Unwrap());
19601960

19611961
ROOT::Experimental::RClusterDescriptor::RPageRange pageRange;
1962-
ROOT::Experimental::RClusterDescriptor::RPageRange::RPageInfo pageInfo;
1962+
ROOT::Experimental::RClusterDescriptor::RPageInfo pageInfo;
19631963
// Second cluster
19641964
clusterBuilder.ClusterId(17).FirstEntryIndex(1).NEntries(2);
19651965
clusterBuilder.MarkSuppressedColumnRange(1);

0 commit comments

Comments
 (0)