Skip to content

Commit de3fb29

Browse files
jblomerjblomer
committed
[ntuple] return page (not page ref) from LoadPageImpl()
1 parent 3a14953 commit de3fb29

9 files changed

Lines changed: 21 additions & 27 deletions

‎tree/ntuple/inc/ROOT/RPageStorage.hxx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,7 @@ protected:
730730
// Only called if a task scheduler is set. No-op be default.
731731
virtual void UnzipClusterImpl(ROOT::Internal::RCluster *cluster);
732732
// Returns a page from storage if not found in the page pool. Will never receive requests for zero pages.
733-
virtual ROOT::Internal::RPageRef LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) = 0;
733+
virtual ROOT::Internal::RPage LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) = 0;
734734
// Returns a sealed page from storage without adding it to the page pool. The sealed pages buffer and buffer size
735735
// is already initialized.
736736
virtual void LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage) = 0;

‎tree/ntuple/inc/ROOT/RPageStorageDaos.hxx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ private:
159159

160160
ROOT::Internal::RNTupleDescriptorBuilder fDescriptorBuilder;
161161

162-
ROOT::Internal::RPageRef LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) final;
162+
ROOT::Internal::RPage LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) final;
163163
void LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage) final;
164164

165165
protected:

‎tree/ntuple/inc/ROOT/RPageStorageFile.hxx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ protected:
180180
/// The cloned page source creates a new raw file and reader and opens its own file descriptor to the data.
181181
std::unique_ptr<RPageSource> CloneImpl() const final;
182182

183-
RPageRef LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) final;
183+
RPage LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary) final;
184184
void LoadSealedPageImpl(const RNTupleLocator &locator, RSealedPage &sealedPage) final;
185185

186186
public:

‎tree/ntuple/src/RPageStorage.cxx‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -444,9 +444,8 @@ ROOT::Internal::RPageRef
444444
ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, ROOT::NTupleSize_t globalIndex)
445445
{
446446
const auto columnId = columnHandle.fPhysicalId;
447-
const auto columnElementId = columnHandle.fColumn->GetElement()->GetIdentifier();
448-
auto cachedPageRef =
449-
fPagePool.GetPage(ROOT::Internal::RPagePool::RKey{columnId, columnElementId.fInMemoryType}, globalIndex);
447+
const auto elementInMemoryType = columnHandle.fColumn->GetElement()->GetIdentifier().fInMemoryType;
448+
auto cachedPageRef = fPagePool.GetPage(ROOT::Internal::RPagePool::RKey{columnId, elementInMemoryType}, globalIndex);
450449
if (!cachedPageRef.Get().IsNull()) {
451450
UpdateLastUsedCluster(cachedPageRef.Get().GetClusterInfo().GetId());
452451
return cachedPageRef;
@@ -477,17 +476,17 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, ROOT::NTupleS
477476
}
478477

479478
UpdateLastUsedCluster(pageSummary.fClusterId);
480-
return LoadPageImpl(columnHandle, pageSummary);
479+
return fPagePool.RegisterPage(LoadPageImpl(columnHandle, pageSummary),
480+
RPagePool::RKey{columnId, elementInMemoryType});
481481
}
482482

483483
ROOT::Internal::RPageRef
484484
ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, RNTupleLocalIndex localIndex)
485485
{
486486
const auto clusterId = localIndex.GetClusterId();
487487
const auto columnId = columnHandle.fPhysicalId;
488-
const auto columnElementId = columnHandle.fColumn->GetElement()->GetIdentifier();
489-
auto cachedPageRef =
490-
fPagePool.GetPage(ROOT::Internal::RPagePool::RKey{columnId, columnElementId.fInMemoryType}, localIndex);
488+
const auto elementInMemoryType = columnHandle.fColumn->GetElement()->GetIdentifier().fInMemoryType;
489+
auto cachedPageRef = fPagePool.GetPage(ROOT::Internal::RPagePool::RKey{columnId, elementInMemoryType}, localIndex);
491490
if (!cachedPageRef.Get().IsNull()) {
492491
UpdateLastUsedCluster(clusterId);
493492
return cachedPageRef;
@@ -516,7 +515,8 @@ ROOT::Internal::RPageSource::LoadPage(ColumnHandle_t columnHandle, RNTupleLocalI
516515
}
517516

518517
UpdateLastUsedCluster(clusterId);
519-
return LoadPageImpl(columnHandle, pageSummary);
518+
return fPagePool.RegisterPage(LoadPageImpl(columnHandle, pageSummary),
519+
RPagePool::RKey{columnId, elementInMemoryType});
520520
}
521521

522522
void ROOT::Internal::RPageSource::EnableDefaultMetrics(const std::string &prefix)

‎tree/ntuple/src/RPageStorageDaos.cxx‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -504,16 +504,13 @@ void ROOT::Experimental::Internal::RPageSourceDaos::LoadSealedPageImpl(const RNT
504504
daosKey.fDkey, daosKey.fAkey);
505505
}
506506

507-
ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPageImpl(ColumnHandle_t columnHandle,
508-
const RPageSummary &pageSummary)
507+
ROOT::Internal::RPage ROOT::Experimental::Internal::RPageSourceDaos::LoadPageImpl(ColumnHandle_t columnHandle,
508+
const RPageSummary &pageSummary)
509509
{
510510
const auto &columnId = columnHandle.fPhysicalId;
511511
const auto &clusterId = pageSummary.fClusterId;
512512
const auto &pageInfo = pageSummary.fPageInfo;
513-
514513
const auto element = columnHandle.fColumn->GetElement();
515-
const auto elementSize = element->GetSize();
516-
const auto elementInMemoryType = element->GetIdentifier().fInMemoryType;
517514

518515
RSealedPage sealedPage;
519516
sealedPage.SetNElements(pageInfo.GetNElements());
@@ -546,13 +543,13 @@ ROOT::Internal::RPageRef ROOT::Experimental::Internal::RPageSourceDaos::LoadPage
546543
{
547544
Detail::RNTupleAtomicTimer timer(fCounters->fTimeWallUnzip, fCounters->fTimeCpuUnzip);
548545
newPage = UnsealPage(sealedPage, *element).Unwrap();
549-
fCounters->fSzUnzip.Add(elementSize * pageInfo.GetNElements());
546+
fCounters->fSzUnzip.Add(element->GetSize() * pageInfo.GetNElements());
550547
}
551548

552549
newPage.SetWindow(pageSummary.fColumnOffset + pageInfo.GetFirstElementIndex(),
553550
ROOT::Internal::RPage::RClusterInfo(clusterId, pageSummary.fColumnOffset));
554551
fCounters->fNPageUnsealed.Inc();
555-
return fPagePool.RegisterPage(std::move(newPage), ROOT::Internal::RPagePool::RKey{columnId, elementInMemoryType});
552+
return newPage;
556553
}
557554

558555
std::unique_ptr<ROOT::Internal::RPageSource> ROOT::Experimental::Internal::RPageSourceDaos::CloneImpl() const

‎tree/ntuple/src/RPageStorageFile.cxx‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -524,16 +524,13 @@ void ROOT::Internal::RPageSourceFile::LoadSealedPageImpl(const RNTupleLocator &l
524524
locator.GetPosition<std::uint64_t>());
525525
}
526526

527-
ROOT::Internal::RPageRef
527+
ROOT::Internal::RPage
528528
ROOT::Internal::RPageSourceFile::LoadPageImpl(ColumnHandle_t columnHandle, const RPageSummary &pageSummary)
529529
{
530530
const auto &columnId = columnHandle.fPhysicalId;
531531
const auto &clusterId = pageSummary.fClusterId;
532532
const auto &pageInfo = pageSummary.fPageInfo;
533-
534533
const auto element = columnHandle.fColumn->GetElement();
535-
const auto elementSize = element->GetSize();
536-
const auto elementInMemoryType = element->GetIdentifier().fInMemoryType;
537534

538535
RSealedPage sealedPage;
539536
sealedPage.SetNElements(pageInfo.GetNElements());
@@ -575,13 +572,13 @@ ROOT::Internal::RPageSourceFile::LoadPageImpl(ColumnHandle_t columnHandle, const
575572
{
576573
RNTupleAtomicTimer timer(fCounters->fTimeWallUnzip, fCounters->fTimeCpuUnzip);
577574
newPage = UnsealPage(sealedPage, *element).Unwrap();
578-
fCounters->fSzUnzip.Add(elementSize * pageInfo.GetNElements());
575+
fCounters->fSzUnzip.Add(element->GetSize() * pageInfo.GetNElements());
579576
}
580577

581578
newPage.SetWindow(pageSummary.fColumnOffset + pageInfo.GetFirstElementIndex(),
582579
ROOT::Internal::RPage::RClusterInfo(clusterId, pageSummary.fColumnOffset));
583580
fCounters->fNPageUnsealed.Inc();
584-
return fPagePool.RegisterPage(std::move(newPage), RPagePool::RKey{columnId, elementInMemoryType});
581+
return newPage;
585582
}
586583

587584
std::unique_ptr<ROOT::Internal::RPageSource> ROOT::Internal::RPageSourceFile::CloneImpl() const

‎tree/ntuple/test/ntuple_cluster.cxx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class RPageSourceMock : public RPageSource {
4141
void LoadStructureImpl() final {}
4242
RNTupleDescriptor AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode) final { return RNTupleDescriptor(); }
4343
std::unique_ptr<RPageSource> CloneImpl() const final { return nullptr; }
44-
RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPageRef(); }
44+
RPage LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPage(); }
4545
void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {}
4646
void LoadStreamerInfo() final {}
4747
std::unique_ptr<ROOT::Internal::RPageSource>

‎tree/ntuple/test/ntuple_endian.cxx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ class RPageSourceMock : public RPageSource {
9393
return RNTupleDescriptor();
9494
}
9595
std::unique_ptr<RPageSource> CloneImpl() const final { return nullptr; }
96-
RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPageRef(); }
96+
RPage LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPage(); }
9797
void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {}
9898
void LoadStreamerInfo() final {}
9999

‎tree/ntuple/test/ntuple_pages.cxx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ class RPageSourceMock : public RPageSource {
1111
void LoadStructureImpl() final {}
1212
RNTupleDescriptor AttachImpl(RNTupleSerializer::EDescriptorDeserializeMode) final { return RNTupleDescriptor(); }
1313
std::unique_ptr<RPageSource> CloneImpl() const final { return nullptr; }
14-
RPageRef LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPageRef(); }
14+
RPage LoadPageImpl(ColumnHandle_t, const RPageSummary &) final { return RPage(); }
1515
void LoadSealedPageImpl(const ROOT::RNTupleLocator &, RSealedPage &) final {}
1616
void LoadStreamerInfo() final {}
1717
std::unique_ptr<ROOT::Internal::RPageSource>

0 commit comments

Comments
 (0)