Skip to content

[ntuple] Make RNTupleProcessor iteration more robust - #23589

Draft
enirolf wants to merge 3 commits into
root-project:masterfrom
enirolf:ntuple-proc-iterator
Draft

enirolf wants to merge 3 commits into
root-project:masterfrom
enirolf:ntuple-proc-iterator

Conversation

@enirolf

@enirolf enirolf commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

The current RNTupleProcessor implementation internally stores bookkeeping information, which is tightly coupled to the iterator over it.

In practice this means that as the iterator advances, it always loads an entry, which is not necessarily desired. Moreover, it also means that it is not possible to have two separate iterators at the same time, because there will be some superposition situation going on.

This PR tries to address this by moving the bookkeeping to the iterator, through a new (read-only) RNTupleProcessor::REntryMapping class, which recursively keeps track of which entry to load from which RNTuple in the processor composition. The mapping gets uploaded as the iterator increases, but no entries get loaded until LoadEntry is called by the application, which takes this mapping as an argument.

This also means that there is a slight change to the API.

I've opened this PR initially as a draft to collect input on the general idea and direction.

To be used to keep track of how entry indexes percolate down to inner
processors, instead of keeping track of that state in the objects
itself.
This move the processor state bookkeeping from the processor itself to
the iterator, making it possible to have multiple iterators for th same
provessor without mixing up the entry state.
@enirolf enirolf self-assigned this Oct 2, 2026
@enirolf
enirolf requested review from jblomer and silverweed October 2, 2026 11:03
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

Test Results

    24 files      24 suites   4d 0h 42m 37s ⏱️
 3 880 tests  3 875 ✅ 0 💤 5 ❌
83 958 runs  83 953 ✅ 0 💤 5 ❌

For more details on these failures, see this check.

Results for commit d16740a.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant