[core] Query TRefTable before TProcessID in TRefArray lookups#22771
Open
guitargeek wants to merge 1 commit into
Open
[core] Query TRefTable before TProcessID in TRefArray lookups#22771guitargeek wants to merge 1 commit into
guitargeek wants to merge 1 commit into
Conversation
ferdymercury
reviewed
Jul 7, 2026
Test Results 23 files 23 suites 3d 14h 40m 53s ⏱️ For more details on these failures, see this check. Results for commit f382af0. ♻️ This comment has been updated with latest results. |
Fixes ROOT-3594 (https://its.cern.ch/jira/browse/ROOT-3594). `TRefArray::At()` and `operator[]` looked up the referenced object in the TProcessID first and only consulted the TRefTable when nothing was found. When entries of a TTree are read selectively (as TTree::Draw and TTree::Scan do), an object from a previously read entry can still be registered under the same UID, so the lookup silently returned a stale object instead of triggering the TBranchRef auto-load of the referenced branch for the current entry. Invert the order in At(), operator[], First(), Last(), After() and Before(): query the TRefTable first, which lets TBranchRef load the referenced branch at the requested entry, and only then fall back to the plain TProcessID lookup. This matches what TRef::GetObject() has always done. Add a regression test (TTreeRegressions.TRefArrayReadsCurrentEntry) that reads only the TRefArray branch of a second entry after fully loading the first one. Update RefProxyTest.ref in roottest: its expected output encoded the bug. For the split2/split9 files, fHighPt.At(0) at entry 0 recorded the track values of the last previously loaded entry instead of entry 0, inconsistently with the split0/split1 sections which already showed the correct values. The new output is uniform across all split levels. Note that objects resolved through a TRefArray now consistently carry the PID-slot byte in the top byte of their in-memory fUniqueID, as objects resolved through a TRef already did; the masked UID (`& 0xFFFFFF`) is unchanged. 🤖 Done with the help of AI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes ROOT-3594 (https://its.cern.ch/jira/browse/ROOT-3594).
Follows up on previous attempts:
TRefArray::At()andoperator[]looked up the referenced object in the TProcessID first and only consulted the TRefTable when nothing was found. When entries of a TTree are read selectively (as TTree::Draw and TTree::Scan do), an object from a previously read entry can still be registered under the same UID, so the lookup silently returned a stale object instead of triggering the TBranchRef auto-load of the referenced branch for the current entry.Invert the order in At(), operator[], First(), Last(), After() and Before(): query the TRefTable first, which lets TBranchRef load the referenced branch at the requested entry, and only then fall back to the plain TProcessID lookup. This matches what TRef::GetObject() has always done.
Add a regression test (TTreeRegressions.TRefArrayReadsCurrentEntry) that reads only the TRefArray branch of a second entry after fully loading the first one.
Update RefProxyTest.ref in roottest: its expected output encoded the bug. For the split2/split9 files, fHighPt.At(0) at entry 0 recorded the track values of the last previously loaded entry instead of entry 0, inconsistently with the split0/split1 sections which already showed the correct values. The new output is uniform across all split levels. Note that objects resolved through a TRefArray now consistently carry the PID-slot byte in the top byte of their in-memory fUniqueID, as objects resolved through a TRef already did; the masked UID (
& 0xFFFFFF) is unchanged.🤖 Done with the help of AI.