[ntuple] Smaller improvement to the RClusterPool interface

tree/ntuple/v7/inc/ROOT/RClusterPool.hxx

@@ -74,7 +74,9 @@ private:
    };
 
 
-   RPageSource *fPageSource;
+   /// Every cluster pool is responsible for exactly one page source that triggers loading of the clusters
+   /// (GetCluster()) and is used for implementating the I/O and cluster memory allocation (PageSource::LoadCluster()).
+   RPageSource &fPageSource;
    /// The number of clusters before the currently active cluster that should stay in the pool if present
    unsigned int fWindowPre;
    /// The number of desired clusters in the pool, including the currently active cluster
@@ -111,8 +113,8 @@ private:
 
 public:
    static constexpr unsigned int kDefaultPoolSize = 4;
-   RClusterPool(RPageSource *pageSource, unsigned int size);
-   explicit RClusterPool(RPageSource *pageSource) : RClusterPool(pageSource, kDefaultPoolSize) {}
+   RClusterPool(RPageSource &pageSource, unsigned int size);
+   explicit RClusterPool(RPageSource &pageSource) : RClusterPool(pageSource, kDefaultPoolSize) {}
    RClusterPool(const RClusterPool &other) = delete;
    RClusterPool &operator =(const RClusterPool &other) = delete;
    ~RClusterPool();

tree/ntuple/v7/inc/ROOT/RNTuple.hxx

@@ -83,7 +83,7 @@ private:
    std::unique_ptr<RNTupleReader> fDisplayReader;
    Detail::RNTupleMetrics fMetrics;
 
-   void ConnectModel(RNTupleModel &model);
+   void ConnectModel(const RNTupleModel &model);
    RNTupleReader *GetDisplayReader();
 
 public:

tree/ntuple/v7/src/RClusterPool.cxx

@@ -49,7 +49,7 @@ bool ROOT::Experimental::Detail::RClusterPool::RInFlightCluster::operator <(cons
    return false;
 }
 
-ROOT::Experimental::Detail::RClusterPool::RClusterPool(RPageSource *pageSource, unsigned int size)
+ROOT::Experimental::Detail::RClusterPool::RClusterPool(RPageSource &pageSource, unsigned int size)
    : fPageSource(pageSource)
    , fPool(size, nullptr)
    , fThreadIo(&RClusterPool::ExecLoadClusters, this)
@@ -93,7 +93,7 @@ void ROOT::Experimental::Detail::RClusterPool::ExecLoadClusters()
             return;
 
          // TODO(jblomer): the page source needs to be capable of loading multiple clusters in one go
-         auto cluster = fPageSource->LoadCluster(item.fClusterId, item.fColumns);
+         auto cluster = fPageSource.LoadCluster(item.fClusterId, item.fColumns);
 
          // Meanwhile, the user might have requested clusters outside the look-ahead window, so that we don't
          // need the cluster anymore, in which case we simply discard it right away, before moving it to the pool
@@ -183,7 +183,7 @@ std::shared_ptr<ROOT::Experimental::Detail::RCluster>
 ROOT::Experimental::Detail::RClusterPool::GetCluster(
    DescriptorId_t clusterId, const RPageSource::ColumnSet_t &columns)
 {
-   const auto &desc = fPageSource->GetDescriptor();
+   const auto &desc = fPageSource.GetDescriptor();
 
    // Determine previous cluster ids that we keep if they happen to be in the pool
    std::set<DescriptorId_t> keep;

tree/ntuple/v7/src/RNTuple.cxx

@@ -31,7 +31,7 @@
 #include <TError.h>
 
 
-void ROOT::Experimental::RNTupleReader::ConnectModel(RNTupleModel &model) {
+void ROOT::Experimental::RNTupleReader::ConnectModel(const RNTupleModel &model) {
    std::unordered_map<const Detail::RFieldBase *, DescriptorId_t> fieldPtr2Id;
    fieldPtr2Id[model.GetFieldZero()] = fSource->GetDescriptor().GetFieldZeroId();
    for (auto &field : *model.GetFieldZero()) {

tree/ntuple/v7/src/RPageStorageFile.cxx

@@ -204,7 +204,7 @@ ROOT::Experimental::Detail::RPageSourceFile::RPageSourceFile(std::string_view nt
    , fMetrics("RPageSourceFile")
    , fPageAllocator(std::make_unique<RPageAllocatorFile>())
    , fPagePool(std::make_shared<RPagePool>())
-   , fClusterPool(std::make_unique<RClusterPool>(this))
+   , fClusterPool(std::make_unique<RClusterPool>(*this))
 {
    fCtrNReadV = fMetrics.MakeCounter<decltype(fCtrNReadV)>("nReadV", "", "number of vector read requests");
    fCtrNRead = fMetrics.MakeCounter<decltype(fCtrNRead)>("nRead", "", "number of byte ranges read");

tree/ntuple/v7/test/ntuple_cluster.cxx

@@ -202,48 +202,50 @@ TEST(Cluster, AdoptClusters)
 
 TEST(ClusterPool, Windows)
 {
-   EXPECT_DEATH(RClusterPool(nullptr, 0), ".*");
-   RClusterPool c1(nullptr, 1);
+   RPageSourceMock ps;
+
+   EXPECT_DEATH(RClusterPool(ps, 0), ".*");
+   RClusterPool c1(ps, 1);
    EXPECT_EQ(0U, c1.GetWindowPre());
    EXPECT_EQ(1U, c1.GetWindowPost());
-   RClusterPool c2(nullptr, 2);
+   RClusterPool c2(ps, 2);
    EXPECT_EQ(0U, c2.GetWindowPre());
    EXPECT_EQ(2U, c2.GetWindowPost());
-   RClusterPool c3(nullptr, 3);
+   RClusterPool c3(ps, 3);
    EXPECT_EQ(1U, c3.GetWindowPre());
    EXPECT_EQ(2U, c3.GetWindowPost());
-   RClusterPool c5(nullptr, 5);
+   RClusterPool c5(ps, 5);
    EXPECT_EQ(1U, c5.GetWindowPre());
    EXPECT_EQ(4U, c5.GetWindowPost());
-   RClusterPool c6(nullptr, 6);
+   RClusterPool c6(ps, 6);
    EXPECT_EQ(2U, c6.GetWindowPre());
    EXPECT_EQ(4U, c6.GetWindowPost());
-   RClusterPool c9(nullptr, 9);
+   RClusterPool c9(ps, 9);
    EXPECT_EQ(2U, c9.GetWindowPre());
    EXPECT_EQ(7U, c9.GetWindowPost());
-   RClusterPool c10(nullptr, 10);
+   RClusterPool c10(ps, 10);
    EXPECT_EQ(3U, c10.GetWindowPre());
    EXPECT_EQ(7U, c10.GetWindowPost());
-   RClusterPool c15(nullptr, 15);
+   RClusterPool c15(ps, 15);
    EXPECT_EQ(3U,  c15.GetWindowPre());
    EXPECT_EQ(12U, c15.GetWindowPost());
-   RClusterPool c16(nullptr, 16);
+   RClusterPool c16(ps, 16);
    EXPECT_EQ(4U,  c16.GetWindowPre());
    EXPECT_EQ(12U, c16.GetWindowPost());
 }
 
 TEST(ClusterPool, GetClusterBasics)
 {
    RPageSourceMock p1;
-   RClusterPool c1(&p1, 1);
+   RClusterPool c1(p1, 1);
    c1.GetCluster(3, {0});
    ASSERT_EQ(1U, p1.fReqsClusterIds.size());
    EXPECT_EQ(3U, p1.fReqsClusterIds[0]);
    EXPECT_EQ(RPageSource::ColumnSet_t({0U}), p1.fReqsColumns[0]);
 
    RPageSourceMock p2;
    {
-      RClusterPool c2(&p2, 2);
+      RClusterPool c2(p2, 2);
       c2.GetCluster(0, {0});
    }
    ASSERT_EQ(2U, p2.fReqsClusterIds.size());
@@ -254,7 +256,7 @@ TEST(ClusterPool, GetClusterBasics)
 
    RPageSourceMock p3;
    {
-      RClusterPool c3(&p3, 4);
+      RClusterPool c3(p3, 4);
       c3.GetCluster(2, {0});
    }
    ASSERT_EQ(3U, p3.fReqsClusterIds.size());
@@ -270,7 +272,7 @@ TEST(ClusterPool, GetClusterBasics)
 TEST(ClusterPool, GetClusterIncrementally)
 {
    RPageSourceMock p1;
-   RClusterPool c1(&p1, 1);
+   RClusterPool c1(p1, 1);
    c1.GetCluster(3, {0});
    ASSERT_EQ(1U, p1.fReqsClusterIds.size());
    EXPECT_EQ(3U, p1.fReqsClusterIds[0]);