root icon indicating copy to clipboard operation
root copied to clipboard

SetClusterPrefetch(true) breaks BulkIO with more than one basket

Open ktf opened this issue 4 years ago • 12 comments

Describe the bug

Doing:

#include <TBufferFile.h>
#include <TFile.h>
#include <TTree.h>
#include <iostream>

void bar() {
  auto f = TFile::Open("/Users/ktf/Downloads/AO2D-2.root");
  char const *dfs[1] = {"DF_2449800039997798139;1"};
  
  for (auto di = 0; di < 1; di++) {
    auto df = dfs[di];
    auto d = (TDirectoryFile*)f->Get(df);
    
    std::cout << "Reading DF " << df  << std::endl;
    char const *treeNames[4] = {"O2bc", "O2collision", "O2fdd", "O2ft0"};

    for (auto ti = 0; ti < 4; ti++) {
      std::cout << ti << std::endl;
      std::cout << treeNames[ti] << std::endl;
      auto t = (TTree*)d->Get(treeNames[ti]);
      t->SetClusterPrefetch(true);
      auto branchList = t->GetListOfBranches();
      auto e = t->GetEntries();
      for (auto bi = 0; bi < branchList->GetEntries(); ++bi) {
        auto b = (TBranch*)branchList->At(bi);
        assert(b);
        int pos = 0;
        while (pos < e) {
          b->Print();
          TBufferFile buf(TBuffer::EMode::kWrite, 32*1024);
          auto &r = b->GetBulkRead();
          auto s = r.GetBulkEntries(0, buf);
          pos += s;
          std::cout << "Read " << s << " elements " << std::endl;
        }
      }
    }
  }
}

crashes with:

[22971:internal-dpl-aod-reader_t0]: Fatal: fExtraBasket == nullptr && "fExtraBasket should have been set to nullptr by GetFreshBasket" violated at line 1523 of `/Users/ktf/src/sw/SOUR
CES/ROOT/ad1ddb8593/0/tree/tree/src/TBranch.cxx'

if one of the branches has more than one basket. Removing t->SetClusterPrefetch(true) fixes the issue. Notice how it still works fine with branches which have only one basket. I can provide a file to reproduce if needed.

Expected behavior

I would expect it to work.

Setup

Vanilla 6.24.02 on linux or macOS.

ktf avatar Sep 06 '21 13:09 ktf

@ktf how critical is this for ALICE?

Axel-Naumann avatar Sep 07 '21 10:09 Axel-Naumann

With the current benchmarking we have, turning it off does not seem to alter performance much, so I would say it's not critical to have it fixed by "yesterday". However it would be nice if a fix could make the 6.24.08 timeline.

That said I reserve the right to bump the priority if further benchmarks show that SetClusterPrefech(true) gives a significant gain.

ktf avatar Sep 07 '21 11:09 ktf

Out of curiosity, will this be fixed for 6.26?

ktf avatar Sep 19 '22 18:09 ktf

It did not make it. I'll try to give a try ....

pcanal avatar Sep 20 '22 16:09 pcanal

Of course I meant 6.28

ktf avatar Sep 20 '22 17:09 ktf

Ah ;) so the likelihood increased exponentially :)

pcanal avatar Sep 20 '22 17:09 pcanal

Read my lips, no new taxes. ;-)

ktf avatar Sep 21 '22 07:09 ktf

@ktf Can you try https://github.com/root-project/root/pull/11501 ?

pcanal avatar Oct 05 '22 18:10 pcanal