Skip to content

[core] Query TRefTable before TProcessID in TRefArray lookups #22771

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
guitargeek wants to merge 1 commit into
base: master
Choose a base branch
from
+124 −50
Open

[core] Query TRefTable before TProcessID in TRefArray lookups #22771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions core/cont/inc/TRefArray.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,12 @@ inline TObject *TRefArray::operator[](Int_t at) const
if (j >= 0 && j < fSize) {
if (!fPID) return nullptr;
if (!TProcessID::IsValid(fPID)) return nullptr;
TObject *obj = fPID->GetObjectWithID(fUIDs[j]);
if (!obj) obj = GetFromTable(j);
// Query the TRefTable first so that a TBranchRef (if any) can load the
// entry of the referenced branch; a stale object from a previously read
// entry may still be registered under this UID (ROOT-3594).
TObject *obj = GetFromTable(j);
if (!obj)
obj = fPID->GetObjectWithID(fUIDs[j]);
return obj;
}
BoundsOk("At", at);
Expand All @@ -188,8 +192,10 @@ inline TObject *TRefArray::At(Int_t at) const
if (j >= 0 && j < fSize) {
if (!fPID) return nullptr;
if (!TProcessID::IsValid(fPID)) return nullptr;
TObject *obj = fPID->GetObjectWithID(fUIDs[j]);
if (!obj) obj = GetFromTable(j);
// See the comment in operator[] above (ROOT-3594).
TObject *obj = GetFromTable(j);
if (!obj)
obj = fPID->GetObjectWithID(fUIDs[j]);
return obj;
}
BoundsOk("At", at);
Expand Down
22 changes: 17 additions & 5 deletions core/cont/src/TRefArray.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,10 @@ TObject *TRefArray::After(const TObject *obj) const
Int_t idx = IndexOf(obj) - fLowerBound;
if (idx == -1 || idx == fSize-1) return nullptr;

return fPID->GetObjectWithID(fUIDs[idx+1]);
TObject *after = GetFromTable(idx + 1);
if (!after)
after = fPID->GetObjectWithID(fUIDs[idx + 1]);
return after;
}

////////////////////////////////////////////////////////////////////////////////
Expand All @@ -418,7 +421,10 @@ TObject *TRefArray::Before(const TObject *obj) const
Int_t idx = IndexOf(obj) - fLowerBound;
if (idx == -1 || idx == 0) return nullptr;

return fPID->GetObjectWithID(fUIDs[idx-1]);
TObject *before = GetFromTable(idx - 1);
if (!before)
before = fPID->GetObjectWithID(fUIDs[idx - 1]);
return before;
}

////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -568,7 +574,10 @@ void TRefArray::Streamer(TBuffer &R__b)

TObject *TRefArray::First() const
{
return fPID->GetObjectWithID(fUIDs[0]);
TObject *obj = GetFromTable(0);
if (!obj)
obj = fPID->GetObjectWithID(fUIDs[0]);
return obj;
}

////////////////////////////////////////////////////////////////////////////////
Expand All @@ -578,8 +587,11 @@ TObject *TRefArray::Last() const
{
if (fLast == -1)
return nullptr;
else
return fPID->GetObjectWithID(fUIDs[GetAbsLast()]);

TObject *obj = GetFromTable(GetAbsLast());
if (!obj)
obj = fPID->GetObjectWithID(fUIDs[GetAbsLast()]);
return obj;
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
72 changes: 36 additions & 36 deletions roottest/root/treeformula/references/RefProxyTest.ref
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,24 +155,24 @@ crap
***********************************************************************************************************************************
* Row * Instance * fHighPt.f * fHighPt.f * fHighPt.f * fHighPt.f * fHighPt.f * fTracks[f * fTracks[f * fTracks[f * fTracks[f *
***********************************************************************************************************************************
* 0 * 0 * 50331650 * 2 * 0.316 * 1.17 * 1.21 * 3 * -0.601 * 2.41 * 2.49 *
* 0 * 1 * 3 * 3 * -0.601 * 2.41 * 2.49 * 3 * -0.601 * 2.41 * 2.49 *
* 0 * 2 * 5 * 5 * 1.1 * -0.616 * 1.26 * 5 * 1.1 * -0.616 * 1.26 *
* 0 * 3 * 6 * 6 * 1.07 * 0.0945 * 1.07 * 7 * 0.338 * 0.282 * 0.44 *
* 0 * 4 * 8 * 8 * -1.53 * 1.47 * 2.12 * 9 * -1.11 * 0.926 * 1.44 *
* 0 * 5 * 9 * 9 * -1.11 * 0.926 * 1.44 * 9 * -1.11 * 0.926 * 1.44 *
* 0 * 6 * 10 * 10 * 0.564 * 1.31 * 1.42 * 11 * -0.328 * -0.291 * 0.439 *
* 0 * 7 * 13 * 13 * 1.4 * -1 * 1.72 * 13 * 1.4 * -1 * 1.72 *
* 0 * 8 * 14 * 14 * 1.63 * 0.221 * 1.65 * 15 * 1.3 * -0.933 * 1.6 *
* 0 * 9 * 15 * 15 * 1.3 * -0.933 * 1.6 * 15 * 1.3 * -0.933 * 1.6 *
* 0 * 10 * 16 * 16 * 0.837 * 1.13 * 1.41 * 17 * -1.86 * -0.0409 * 1.86 *
* 0 * 11 * 17 * 17 * -1.86 * -0.0409 * 1.86 * 17 * -1.86 * -0.0409 * 1.86 *
* 0 * 12 * 20 * 20 * 1.47 * -0.536 * 1.56 * 21 * -0.0558 * 0.891 * 0.893 *
* 0 * 13 * 23 * 23 * 0.582 * 0.869 * 1.05 * 23 * 0.582 * 0.869 * 1.05 *
* 0 * 14 * 24 * 24 * 0.799 * 0.778 * 1.11 * 25 * -0.304 * 1.99 * 2.01 *
* 0 * 15 * 25 * 25 * -0.304 * 1.99 * 2.01 * 25 * -0.304 * 1.99 * 2.01 *
* 0 * 16 * 27 * 27 * -0.832 * -0.984 * 1.29 * 27 * -0.832 * -0.984 * 1.29 *
* 0 * 17 * 29 * 29 * 1.16 * -1.16 * 1.64 * 29 * 1.16 * -1.16 * 1.64 *
* 0 * 0 * 50331650 * 2 * 1.1 * 0.299 * 1.14 * 3 * -0.601 * 2.41 * 2.49 *
* 0 * 1 * 50331651 * 3 * -0.601 * 2.41 * 2.49 * 3 * -0.601 * 2.41 * 2.49 *
* 0 * 2 * 50331653 * 5 * 1.1 * -0.616 * 1.26 * 5 * 1.1 * -0.616 * 1.26 *
* 0 * 3 * 50331654 * 6 * 1.07 * 0.0945 * 1.07 * 7 * 0.338 * 0.282 * 0.44 *
* 0 * 4 * 50331656 * 8 * -1.53 * 1.47 * 2.12 * 9 * -1.11 * 0.926 * 1.44 *
* 0 * 5 * 50331657 * 9 * -1.11 * 0.926 * 1.44 * 9 * -1.11 * 0.926 * 1.44 *
* 0 * 6 * 50331658 * 10 * 0.564 * 1.31 * 1.42 * 11 * -0.328 * -0.291 * 0.439 *
* 0 * 7 * 50331661 * 13 * 1.4 * -1 * 1.72 * 13 * 1.4 * -1 * 1.72 *
* 0 * 8 * 50331662 * 14 * 1.63 * 0.221 * 1.65 * 15 * 1.3 * -0.933 * 1.6 *
* 0 * 9 * 50331663 * 15 * 1.3 * -0.933 * 1.6 * 15 * 1.3 * -0.933 * 1.6 *
* 0 * 10 * 50331664 * 16 * 0.837 * 1.13 * 1.41 * 17 * -1.86 * -0.0409 * 1.86 *
* 0 * 11 * 50331665 * 17 * -1.86 * -0.0409 * 1.86 * 17 * -1.86 * -0.0409 * 1.86 *
* 0 * 12 * 50331668 * 20 * 1.47 * -0.536 * 1.56 * 21 * -0.0558 * 0.891 * 0.893 *
* 0 * 13 * 50331671 * 23 * 0.582 * 0.869 * 1.05 * 23 * 0.582 * 0.869 * 1.05 *
* 0 * 14 * 50331672 * 24 * 0.799 * 0.778 * 1.11 * 25 * -0.304 * 1.99 * 2.01 *
* 0 * 15 * 50331673 * 25 * -0.304 * 1.99 * 2.01 * 25 * -0.304 * 1.99 * 2.01 *
* 0 * 16 * 50331675 * 27 * -0.832 * -0.984 * 1.29 * 27 * -0.832 * -0.984 * 1.29 *
* 0 * 17 * 50331677 * 29 * 1.16 * -1.16 * 1.64 * 29 * 1.16 * -1.16 * 1.64 *
***********************************************************************************************************************************


Expand Down Expand Up @@ -213,23 +213,23 @@ crap
***********************************************************************************************************************************
* Row * Instance * fHighPt.f * fHighPt.f * fHighPt.f * fHighPt.f * fHighPt.f * fTracks[f * fTracks[f * fTracks[f * fTracks[f *
***********************************************************************************************************************************
* 0 * 0 * 67108866 * 2 * 0.316 * 1.17 * 1.21 * 3 * -0.601 * 2.41 * 2.49 *
* 0 * 1 * 3 * 3 * -0.601 * 2.41 * 2.49 * 3 * -0.601 * 2.41 * 2.49 *
* 0 * 2 * 5 * 5 * 1.1 * -0.616 * 1.26 * 5 * 1.1 * -0.616 * 1.26 *
* 0 * 3 * 6 * 6 * 1.07 * 0.0945 * 1.07 * 7 * 0.338 * 0.282 * 0.44 *
* 0 * 4 * 8 * 8 * -1.53 * 1.47 * 2.12 * 9 * -1.11 * 0.926 * 1.44 *
* 0 * 5 * 9 * 9 * -1.11 * 0.926 * 1.44 * 9 * -1.11 * 0.926 * 1.44 *
* 0 * 6 * 10 * 10 * 0.564 * 1.31 * 1.42 * 11 * -0.328 * -0.291 * 0.439 *
* 0 * 7 * 13 * 13 * 1.4 * -1 * 1.72 * 13 * 1.4 * -1 * 1.72 *
* 0 * 8 * 14 * 14 * 1.63 * 0.221 * 1.65 * 15 * 1.3 * -0.933 * 1.6 *
* 0 * 9 * 15 * 15 * 1.3 * -0.933 * 1.6 * 15 * 1.3 * -0.933 * 1.6 *
* 0 * 10 * 16 * 16 * 0.837 * 1.13 * 1.41 * 17 * -1.86 * -0.0409 * 1.86 *
* 0 * 11 * 17 * 17 * -1.86 * -0.0409 * 1.86 * 17 * -1.86 * -0.0409 * 1.86 *
* 0 * 12 * 20 * 20 * 1.47 * -0.536 * 1.56 * 21 * -0.0558 * 0.891 * 0.893 *
* 0 * 13 * 23 * 23 * 0.582 * 0.869 * 1.05 * 23 * 0.582 * 0.869 * 1.05 *
* 0 * 14 * 24 * 24 * 0.799 * 0.778 * 1.11 * 25 * -0.304 * 1.99 * 2.01 *
* 0 * 15 * 25 * 25 * -0.304 * 1.99 * 2.01 * 25 * -0.304 * 1.99 * 2.01 *
* 0 * 16 * 27 * 27 * -0.832 * -0.984 * 1.29 * 27 * -0.832 * -0.984 * 1.29 *
* 0 * 17 * 29 * 29 * 1.16 * -1.16 * 1.64 * 29 * 1.16 * -1.16 * 1.64 *
* 0 * 0 * 67108866 * 2 * 1.1 * 0.299 * 1.14 * 3 * -0.601 * 2.41 * 2.49 *
* 0 * 1 * 67108867 * 3 * -0.601 * 2.41 * 2.49 * 3 * -0.601 * 2.41 * 2.49 *
* 0 * 2 * 67108869 * 5 * 1.1 * -0.616 * 1.26 * 5 * 1.1 * -0.616 * 1.26 *
* 0 * 3 * 67108870 * 6 * 1.07 * 0.0945 * 1.07 * 7 * 0.338 * 0.282 * 0.44 *
* 0 * 4 * 67108872 * 8 * -1.53 * 1.47 * 2.12 * 9 * -1.11 * 0.926 * 1.44 *
* 0 * 5 * 67108873 * 9 * -1.11 * 0.926 * 1.44 * 9 * -1.11 * 0.926 * 1.44 *
* 0 * 6 * 67108874 * 10 * 0.564 * 1.31 * 1.42 * 11 * -0.328 * -0.291 * 0.439 *
* 0 * 7 * 67108877 * 13 * 1.4 * -1 * 1.72 * 13 * 1.4 * -1 * 1.72 *
* 0 * 8 * 67108878 * 14 * 1.63 * 0.221 * 1.65 * 15 * 1.3 * -0.933 * 1.6 *
* 0 * 9 * 67108879 * 15 * 1.3 * -0.933 * 1.6 * 15 * 1.3 * -0.933 * 1.6 *
* 0 * 10 * 67108880 * 16 * 0.837 * 1.13 * 1.41 * 17 * -1.86 * -0.0409 * 1.86 *
* 0 * 11 * 67108881 * 17 * -1.86 * -0.0409 * 1.86 * 17 * -1.86 * -0.0409 * 1.86 *
* 0 * 12 * 67108884 * 20 * 1.47 * -0.536 * 1.56 * 21 * -0.0558 * 0.891 * 0.893 *
* 0 * 13 * 67108887 * 23 * 0.582 * 0.869 * 1.05 * 23 * 0.582 * 0.869 * 1.05 *
* 0 * 14 * 67108888 * 24 * 0.799 * 0.778 * 1.11 * 25 * -0.304 * 1.99 * 2.01 *
* 0 * 15 * 67108889 * 25 * -0.304 * 1.99 * 2.01 * 25 * -0.304 * 1.99 * 2.01 *
* 0 * 16 * 67108891 * 27 * -0.832 * -0.984 * 1.29 * 27 * -0.832 * -0.984 * 1.29 *
* 0 * 17 * 67108893 * 29 * 1.16 * -1.16 * 1.64 * 29 * 1.16 * -1.16 * 1.64 *
***********************************************************************************************************************************
(bool) true
66 changes: 61 additions & 5 deletions tree/tree/test/TTreeRegressions.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
#include "TMemFile.h"
#include "TLeaf.h"
#include "TTree.h"
#include "TBranch.h"
#include "TClonesArray.h"
Comment thread
guitargeek marked this conversation as resolved.
#include "TH1F.h"
#include "TInterpreter.h"
#include "TSystem.h"
#include "TLeaf.h"
#include "TLeafObject.h"
#include "TH1F.h"
#include "TMemFile.h"
#include "TNamed.h"
#include "TProcessID.h"
#include "TROOT.h"
#include "TRefArray.h"
#include "TSystem.h"
#include "TTree.h"

#include "gtest/gtest.h"

Expand Down Expand Up @@ -322,3 +327,54 @@ TEST(TTreeRegressions, DrawAutoBinning)
delete h;
delete gROOT->FindObject("c1");
}

// https://its.cern.ch/jira/browse/ROOT-3594
// TRefArray::At must query the TRefTable (loading the referenced branch for
// the current entry) before looking up the TProcessID, like TRef::GetObject
// does; otherwise it returns a stale object from a previously read entry.
TEST(TTreeRegressions, TRefArrayReadsCurrentEntry)
{
TMemFile f("tree_trefarraycurrententry.root", "recreate");
{
TTree t("t", "t");
t.BranchRef();
auto *hits = new TClonesArray("TNamed");
auto *refs = new TRefArray;
t.Branch("hits", &hits);
t.Branch("refs", &refs);
for (int ev = 0; ev < 2; ++ev) {
const auto objcount = TProcessID::GetObjectCount();
hits->Clear();
refs->Clear();
for (int i = 0; i < 2; ++i) {
auto *h = new ((*hits)[i]) TNamed(TString::Format("ev%d_hit%d", ev, i).Data(), "");
refs->Add(h);
}
t.Fill();
TProcessID::SetObjectCount(objcount);
}
t.Write();
delete hits;
delete refs;
}

auto &t = *f.Get<TTree>("t");
TClonesArray *hits = nullptr;
TRefArray *refs = nullptr;
t.SetBranchAddress("hits", &hits);
t.SetBranchAddress("refs", &refs);

// Fully load entry 0: its hits get registered in the TProcessID.
t.GetEntry(0);
ASSERT_NE(refs->At(0), nullptr);
EXPECT_STREQ(refs->At(0)->GetName(), "ev0_hit0");

// Emulate TTree::Draw: move to entry 1 but only read the refs branch. The
// TRefTable must load the hits branch for entry 1 when dereferencing.
t.LoadTree(1);
t.GetBranch("refs")->GetEntry(1);
ASSERT_NE(refs->At(0), nullptr);
EXPECT_STREQ(refs->At(0)->GetName(), "ev1_hit0");
ASSERT_NE(refs->At(1), nullptr);
EXPECT_STREQ(refs->At(1)->GetName(), "ev1_hit1");
}
Loading