[RF] RooWorkspace::data returns null pointer if the workspace contains 52 or more datasets

[Zehvogel]

Check duplicate issues.

• Checked for duplicates

Description

as the title says...

reproducer below prints:
dataset retrieval failed with 52 datasets

Reproducer

import ROOT


max_n = 100

for n in range(1, max_n):
    w = ROOT.RooWorkspace("w", "workspace")
    obs = [ROOT.RooRealVar("O", "O", 0., -1., 1.)]
    w.Import(obs)
    for i in range(n):
        ds_name = f"dataset_{i}"
        ds = ROOT.RooDataSet(ds_name, ds_name, obs)
        w.Import(ds)
    test_ds = w.data("dataset_0")
    if not test_ds:
        print(f"dataset retrieval failed with {n} datasets")
        break

ROOT version

------------------------------------------------------------------
  | Welcome to ROOT 6.38.00                        https://root.cern |
  | (c) 1995-2025, The ROOT Team; conception: R. Brun, F. Rademakers |
  | Built for linuxx8664gcc on Dec 21 2025, 15:58:20                 |
  | From tags/6-38-00@6-38-00                                        |
  | With g++ (Spack GCC) 14.2.0 std202002                            |
  | Try '.help'/'.?', '.demo', '.license', '.credits', '.quit'/'.q'  |
   ------------------------------------------------------------------

Installation method

source /cvmfs/sw-nightlies.hsf.org/key4hep/setup.sh -r 2026-01-15

Operating system

Alma9

Additional context

No response

[guitargeek]

I can also reproduce this with C++:

#include <RooWorkspace.h>
#include <RooRealVar.h>
#include <RooDataSet.h>

#include <iostream>
#include <sstream>

void test20904()
{
   const int max_n = 100;

   for (int n = 1; n < max_n; ++n) {
      RooWorkspace w("w", "workspace");

      RooRealVar obs("obs", "obs", 0.0, -1.0, 1.0);
      w.import(obs);

      for (int i = 0; i < n; ++i) {
         w.import(RooDataSet{"dataset_" + std::to_string(i), "", RooArgSet(obs)});
      }

      if (!w.data("dataset_0")) {
         std::cout << "dataset retrieval failed with " << n << " datasets" << std::endl;
         break;
      }
   }
}

Investigating.

[ferdymercury]

Hi Jonas, I think I found a solution (sorry I had started looking into it before your message but didn't inform here):

diff --git a/roofit/roofitcore/src/RooLinkedList.cxx b/roofit/roofitcore/src/RooLinkedList.cxx
index 7cf5c14619..f0d1e3cff7 100644
--- a/roofit/roofitcore/src/RooLinkedList.cxx
+++ b/roofit/roofitcore/src/RooLinkedList.cxx
@@ -342,6 +342,13 @@ void RooLinkedList::setHashTableSize(Int_t size)
       // (Re)create hash tables
       _htableName = std::make_unique<HashTableByName>(size);
       _htableLink = std::make_unique<HashTableByLink>(size);
+      // Refill preexisting elements into hash table
+      const auto preSize = this->_at.size();
+      for (size_t i = 0; i < preSize; ++i) {
+         auto arg = this->_at[i]->_arg;
+         _htableName->insert({arg->GetName(), arg});
+         _htableLink->insert({arg, i < preSize - 1 ? this->_at[i + 1]->_arg : nullptr});
+      }
    }
 
    _htableName->reserve(size);

although I do not know if it is the cleanest way to solve, so feel free to take it over to your PR.

The issue stems from setting 200 size when over 51 datasets

  RooLinkedList& dataList = embedded ? _embeddedDataList : _dataList ;
  if (dataList.size() > 50 && dataList.getHashTableSize() == 0) {
    // When the workspaces get larger, traversing the linked list becomes a bottleneck:
    dataList.setHashTableSize(200);
  }

[guitargeek]

Thanks! I found the same solution basically 👍

diff --git a/roofit/roofitcore/src/RooLinkedList.cxx b/roofit/roofitcore/src/RooLinkedList.cxx
index 7cf5c146195..a0989358f33 100644
--- a/roofit/roofitcore/src/RooLinkedList.cxx
+++ b/roofit/roofitcore/src/RooLinkedList.cxx
@@ -342,6 +342,11 @@ void RooLinkedList::setHashTableSize(Int_t size)
       // (Re)create hash tables
       _htableName = std::make_unique<HashTableByName>(size);
       _htableLink = std::make_unique<HashTableByLink>(size);
+
+      for (RooLinkedListElem *elem = _first; elem; elem = elem->_next) {
+         _htableName->insert({elem->_arg->GetName(), elem->_arg});
+         _htableLink->insert({elem->_arg, reinterpret_cast<TObject *>(elem)});
+      }
    }

    _htableName->reserve(size);

The only difference is that I iterate through the linked list as one should, because the random access you propose is slow for a linked list.

I really oversaw this simple problem in a recent optimization I did... 318faa0

[guitargeek]

@ferdymercury, it's okay if I open the PR? I caused the problem after all 🙂

[ferdymercury]

Of course go ahead :) 💃

[ferdymercury]

Btw maybe this also helps:

else {
_htableName->reserve(size);
_htableLink->reserve(size);
}

(no need to do that if you are recreating?)

[guitargeek]

I don't understand what you mean exactly, but maybe it's easier if you comment on my PR