From 13a641ae68ae90b09c215d1b93da45b09f9a0753 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Sun, 17 May 2026 12:05:36 +0200 Subject: [PATCH 1/8] [df] Split value reading into Load+Get Sets the stage for future bulk reading where the actual values that need to be passed to downstream nodes may need to get stored in a separate location. --- tree/dataframe/inc/ROOT/RCsvDS.hxx | 3 +- tree/dataframe/inc/ROOT/RDF/RAction.hxx | 20 ++++--- .../inc/ROOT/RDF/RActionSnapshot.hxx | 19 ++++--- .../inc/ROOT/RDF/RColumnReaderBase.hxx | 21 ++++++- .../inc/ROOT/RDF/RDSColumnReader.hxx | 3 +- .../inc/ROOT/RDF/RDefaultValueFor.hxx | 13 +++-- tree/dataframe/inc/ROOT/RDF/RDefine.hxx | 38 +++++++------ tree/dataframe/inc/ROOT/RDF/RDefineBase.hxx | 2 +- .../inc/ROOT/RDF/RDefinePerSample.hxx | 2 +- tree/dataframe/inc/ROOT/RDF/RDefineReader.hxx | 8 +-- tree/dataframe/inc/ROOT/RDF/RFilter.hxx | 52 +++++++++-------- .../inc/ROOT/RDF/RFilterWithMissingValues.hxx | 40 +++++++------ tree/dataframe/inc/ROOT/RDF/RJittedDefine.hxx | 2 +- .../inc/ROOT/RDF/RJittedVariation.hxx | 2 +- tree/dataframe/inc/ROOT/RDF/RLazyDSImpl.hxx | 3 +- .../inc/ROOT/RDF/RTreeColumnReader.hxx | 21 ++++--- tree/dataframe/inc/ROOT/RDF/RVariation.hxx | 21 ++++--- .../dataframe/inc/ROOT/RDF/RVariationBase.hxx | 2 +- .../inc/ROOT/RDF/RVariationReader.hxx | 8 +-- tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx | 20 ++++--- tree/dataframe/inc/ROOT/RSqliteDS.hxx | 3 +- tree/dataframe/inc/ROOT/RTrivialDS.hxx | 3 +- tree/dataframe/inc/ROOT/RVecDS.hxx | 3 +- tree/dataframe/src/RJittedDefine.cxx | 4 +- tree/dataframe/src/RJittedVariation.cxx | 4 +- tree/dataframe/src/RNTupleDS.cxx | 7 ++- tree/dataframe/src/RTreeColumnReader.cxx | 57 +++++++++++++------ tree/dataframe/test/RArraysDS.hxx | 6 +- 28 files changed, 234 insertions(+), 153 deletions(-) diff --git a/tree/dataframe/inc/ROOT/RCsvDS.hxx b/tree/dataframe/inc/ROOT/RCsvDS.hxx index 89bf6188edacc..d29e7d44c345c 100644 --- a/tree/dataframe/inc/ROOT/RCsvDS.hxx +++ b/tree/dataframe/inc/ROOT/RCsvDS.hxx @@ -27,7 +27,8 @@ namespace ROOT::Internal::RDF { class R__CLING_PTRCHECK(off) RCsvDSColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { void *fValuePtr; - void *GetImpl(Long64_t) final { return fValuePtr; } + void *GetImpl(std::size_t) final { return fValuePtr; } + void LoadImpl(Long64_t, bool) final {} public: RCsvDSColumnReader(void *valuePtr) : fValuePtr(valuePtr) {} diff --git a/tree/dataframe/inc/ROOT/RDF/RAction.hxx b/tree/dataframe/inc/ROOT/RDF/RAction.hxx index 2a9e512f65633..6fae6281fd8d0 100644 --- a/tree/dataframe/inc/ROOT/RDF/RAction.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RAction.hxx @@ -99,31 +99,33 @@ public: } template - auto GetValueChecked(unsigned int slot, std::size_t readerIdx, Long64_t entry) -> ColType & + auto GetValueChecked(unsigned int slot, std::size_t readerIdx, std::size_t idx) -> ColType & { - if (auto *val = fValues[slot][readerIdx]->template TryGet(entry)) + if (auto *val = fValues[slot][readerIdx]->template TryGet(idx)) return *val; throw std::out_of_range{"RDataFrame: Action (" + fHelper.GetActionName() + ") could not retrieve value for column '" + fColumnNames[readerIdx] + "' for entry " + - std::to_string(entry) + + std::to_string(idx) + ". You can use the DefaultValueFor operation to provide a default value, or " "FilterAvailable/FilterMissing to discard/keep entries with missing values instead."}; } template - void CallExec(unsigned int slot, Long64_t entry, TypeList, std::index_sequence) + void CallExec(unsigned int slot, std::size_t idx, TypeList, std::index_sequence) { ROOT::Internal::RDF::CallGuaranteedOrder{[&](auto &&...args) { return fHelper.Exec(slot, args...); }, - GetValueChecked(slot, S, entry)...}; - (void)entry; // avoid unused parameter warning (gcc 12.1) + GetValueChecked(slot, S, idx)...}; + (void)idx; // avoid unused parameter warning (gcc 12.1) } void Run(unsigned int slot, Long64_t entry) final { - // check if entry passes all filters - if (fPrevNode.CheckFilters(slot, entry)) - CallExec(slot, entry, ColumnTypes_t{}, TypeInd_t{}); + const auto mask = fPrevNode.CheckFilters(slot, entry); + std::for_each(fValues[slot].begin(), fValues[slot].end(), [entry, mask](auto *v) { v->Load(entry, mask); }); + + if (mask) + CallExec(slot, /*idx=*/0u, ColumnTypes_t{}, TypeInd_t{}); } void TriggerChildrenCount() final { fPrevNode.IncrChildrenCount(); } diff --git a/tree/dataframe/inc/ROOT/RDF/RActionSnapshot.hxx b/tree/dataframe/inc/ROOT/RDF/RActionSnapshot.hxx index 510b8b5ec273b..3e30013461e7f 100644 --- a/tree/dataframe/inc/ROOT/RDF/RActionSnapshot.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RActionSnapshot.hxx @@ -166,27 +166,27 @@ public: fHelper.InitTask(r, slot); } - void *GetValue(unsigned int slot, std::size_t readerIdx, Long64_t entry) + void *GetValue(unsigned int slot, std::size_t readerIdx, std::size_t idx) { assert(slot < fValues.size()); assert(readerIdx < fValues[slot].size()); - if (auto *val = fValues[slot][readerIdx]->template TryGet(entry)) + if (auto *val = fValues[slot][readerIdx]->template TryGet(idx)) return val; throw std::out_of_range{"RDataFrame: Action (" + fHelper.GetActionName() + ") could not retrieve value for column '" + fColumnNames[readerIdx] + "' for entry " + - std::to_string(entry) + + std::to_string(idx) + ". You can use the DefaultValueFor operation to provide a default value, or " "FilterAvailable/FilterMissing to discard/keep entries with missing values instead."}; } - void CallExec(unsigned int slot, Long64_t entry) + void CallExec(unsigned int slot, std::size_t idx) { std::vector untypedValues; auto nReaders = fValues[slot].size(); untypedValues.reserve(nReaders); for (decltype(nReaders) readerIdx{}; readerIdx < nReaders; readerIdx++) - untypedValues.push_back(GetValue(slot, readerIdx, entry)); + untypedValues.push_back(GetValue(slot, readerIdx, idx)); fHelper.Exec(slot, untypedValues); } @@ -207,14 +207,17 @@ public: std::vector untypedValues; auto nReaders = fValues[slot].size(); untypedValues.reserve(nReaders); + std::for_each(fValues[slot].begin(), fValues[slot].end(), [entry](auto *v) { v->Load(entry, true); }); for (decltype(nReaders) readerIdx{}; readerIdx < nReaders; readerIdx++) - untypedValues.push_back(GetValue(slot, readerIdx, entry)); + untypedValues.push_back(GetValue(slot, readerIdx, /*idx=*/0u)); fHelper.Exec(slot, untypedValues, filterPassed); } } else { - if (fPrevNodes.front()->CheckFilters(slot, entry)) - CallExec(slot, entry); + const auto mask = fPrevNodes.front()->CheckFilters(slot, entry); + std::for_each(fValues[slot].begin(), fValues[slot].end(), [entry, mask](auto *v) { v->Load(entry, mask); }); + if (mask) + CallExec(slot, /*idx=*/0u); } } diff --git a/tree/dataframe/inc/ROOT/RDF/RColumnReaderBase.hxx b/tree/dataframe/inc/ROOT/RDF/RColumnReaderBase.hxx index 29aeee4f306e7..a129a9f261e87 100644 --- a/tree/dataframe/inc/ROOT/RDF/RColumnReaderBase.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RColumnReaderBase.hxx @@ -26,9 +26,23 @@ This pure virtual class provides a common base class for the different column re RDSColumnReader. **/ class R__CLING_PTRCHECK(off) RColumnReaderBase { + Long64_t fLoadedEntry = -1; + public: virtual ~RColumnReaderBase() = default; + /// Load the column value for the given entry. + /// \param entry The entry number to load. + /// \param mask The entry mask. Values will be loaded only for entries for which the mask equals true. + void Load(Long64_t entry, bool mask) + { + // For now, as `mask` is just a single boolean, as an optimization we can return early here if `mask == false`. + if (mask) { + fLoadedEntry = entry; + this->LoadImpl(entry, mask); + } + } + /// Return the column value for the given entry. /// \tparam T The column type /// \param entry The entry number @@ -36,13 +50,14 @@ public: /// The caller is responsible for checking that the returned value actually /// exists. template - T *TryGet(Long64_t entry) + T *TryGet(std::size_t idx) { - return static_cast(GetImpl(entry)); + return static_cast(GetImpl(idx)); } private: - virtual void *GetImpl(Long64_t entry) = 0; + virtual void *GetImpl(std::size_t idx) = 0; + virtual void LoadImpl(Long64_t /*entry*/, bool /*mask*/) = 0; }; } // namespace RDF diff --git a/tree/dataframe/inc/ROOT/RDF/RDSColumnReader.hxx b/tree/dataframe/inc/ROOT/RDF/RDSColumnReader.hxx index 2e317573102b5..571154d80497b 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDSColumnReader.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDSColumnReader.hxx @@ -23,7 +23,8 @@ template class R__CLING_PTRCHECK(off) RDSColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { T **fDSValuePtr = nullptr; - void *GetImpl(Long64_t) final { return *fDSValuePtr; } + void *GetImpl(std::size_t) final { return *fDSValuePtr; } + void LoadImpl(Long64_t, bool) final {} public: RDSColumnReader(void *DSValuePtr) : fDSValuePtr(static_cast(DSValuePtr)) {} diff --git a/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx b/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx index a03af81745a1b..864a38583ecce 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx @@ -57,9 +57,9 @@ class R__CLING_PTRCHECK(off) RDefaultValueFor final : public RDefineBase { /// The map key is the full variation name, e.g. "pt:up". std::unordered_map> fVariedDefines; - T &GetValueOrDefault(unsigned int slot, Long64_t entry) + T &GetValueOrDefault(unsigned int slot, std::size_t idx) { - if (auto *value = fValues[slot]->template TryGet(entry)) + if (auto *value = fValues[slot]->template TryGet(idx)) return *value; else return fDefaultValue; @@ -104,12 +104,15 @@ public: } /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry - void Update(unsigned int slot, Long64_t entry) final + void Update(unsigned int slot, Long64_t entry, bool mask) final { if (entry != fLastCheckedEntry[slot * RDFInternal::CacheLineStep()]) { // evaluate this define expression, cache the result - fLastResults[slot * RDFInternal::CacheLineStep()] = GetValueOrDefault(slot, entry); - fLastCheckedEntry[slot * RDFInternal::CacheLineStep()] = entry; + fValues[slot]->Load(entry, mask); + if (mask) { + fLastResults[slot * RDFInternal::CacheLineStep()] = GetValueOrDefault(slot, /*idx=*/0u); + fLastCheckedEntry[slot * RDFInternal::CacheLineStep()] = entry; + } } } diff --git a/tree/dataframe/inc/ROOT/RDF/RDefine.hxx b/tree/dataframe/inc/ROOT/RDF/RDefine.hxx index 8c9a57351079a..1350e3989505c 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefine.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefine.hxx @@ -71,39 +71,43 @@ class R__CLING_PTRCHECK(off) RDefine final : public RDefineBase { std::unordered_map> fVariedDefines; template - auto GetValueChecked(unsigned int slot, std::size_t readerIdx, Long64_t entry) -> ColType & + auto GetValueChecked(unsigned int slot, std::size_t readerIdx, std::size_t idx) -> ColType & { - if (auto *val = fValues[slot][readerIdx]->template TryGet(entry)) + if (auto *val = fValues[slot][readerIdx]->template TryGet(idx)) return *val; throw std::out_of_range{"RDataFrame: Define could not retrieve value for column '" + fColumnNames[readerIdx] + - "' for entry " + std::to_string(entry) + + "' for entry " + std::to_string(idx) + ". You can use the DefaultValueFor operation to provide a default value, or " "FilterAvailable/FilterMissing to discard/keep entries with missing values instead."}; } template - void UpdateHelper(unsigned int slot, Long64_t entry, TypeList, std::index_sequence, NoneTag) + void UpdateHelper(unsigned int slot, std::size_t idx, Long64_t /*entry*/, TypeList, + std::index_sequence, NoneTag) { fLastResults[slot * RDFInternal::CacheLineStep()] = - fExpression(GetValueChecked(slot, S, entry)...); - (void)entry; // avoid unused parameter warning (gcc 12.1) + fExpression(GetValueChecked(slot, S, idx)...); + (void)idx; // avoid unused parameter warning (gcc 12.1) } template - void UpdateHelper(unsigned int slot, Long64_t entry, TypeList, std::index_sequence, SlotTag) + void UpdateHelper(unsigned int slot, std::size_t idx, Long64_t /*entry*/, TypeList, + std::index_sequence, SlotTag) { fLastResults[slot * RDFInternal::CacheLineStep()] = - fExpression(slot, GetValueChecked(slot, S, entry)...); - (void)entry; // avoid unused parameter warning (gcc 12.1) + fExpression(slot, GetValueChecked(slot, S, idx)...); + (void)idx; // avoid unused parameter warning (gcc 12.1) } template - void - UpdateHelper(unsigned int slot, Long64_t entry, TypeList, std::index_sequence, SlotAndEntryTag) + void UpdateHelper(unsigned int slot, std::size_t idx, Long64_t batchFirstEntry, TypeList, + std::index_sequence, SlotAndEntryTag) { fLastResults[slot * RDFInternal::CacheLineStep()] = - fExpression(slot, entry, GetValueChecked(slot, S, entry)...); + fExpression(slot, batchFirstEntry + idx, GetValueChecked(slot, S, idx)...); + (void)idx; // avoid unused parameter warning (gcc 12.1) + (void)batchFirstEntry; // avoid unused parameter warning (gcc 12.1) } public: @@ -134,12 +138,14 @@ public: } /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry - void Update(unsigned int slot, Long64_t entry) final + void Update(unsigned int slot, Long64_t entry, bool mask) final { if (entry != fLastCheckedEntry[slot * RDFInternal::CacheLineStep()]) { - // evaluate this define expression, cache the result - UpdateHelper(slot, entry, ColumnTypes_t{}, TypeInd_t{}, ExtraArgsTag{}); - fLastCheckedEntry[slot * RDFInternal::CacheLineStep()] = entry; + std::for_each(fValues[slot].begin(), fValues[slot].end(), [entry, mask](auto *v) { v->Load(entry, mask); }); + if (mask) { + UpdateHelper(slot, /*idx=*/0u, entry, ColumnTypes_t{}, TypeInd_t{}, ExtraArgsTag{}); + fLastCheckedEntry[slot * RDFInternal::CacheLineStep()] = entry; + } } } diff --git a/tree/dataframe/inc/ROOT/RDF/RDefineBase.hxx b/tree/dataframe/inc/ROOT/RDF/RDefineBase.hxx index f20b77e0b3286..058db69fc1d2a 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefineBase.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefineBase.hxx @@ -63,7 +63,7 @@ public: std::string GetName() const; std::string GetTypeName() const; /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry - virtual void Update(unsigned int slot, Long64_t entry) = 0; + virtual void Update(unsigned int slot, Long64_t entry, bool mask) = 0; /// Update function to be called once per sample, used if the derived type is a RDefinePerSample virtual void Update(unsigned int /*slot*/, const ROOT::RDF::RSampleInfo &/*id*/) {} /// Clean-up operations to be performed at the end of a task. diff --git a/tree/dataframe/inc/ROOT/RDF/RDefinePerSample.hxx b/tree/dataframe/inc/ROOT/RDF/RDefinePerSample.hxx index 7f0fb27af918b..4a962848a540f 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefinePerSample.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefinePerSample.hxx @@ -59,7 +59,7 @@ public: return static_cast(&fLastResults[slot * RDFInternal::CacheLineStep()]); } - void Update(unsigned int, Long64_t) final + void Update(unsigned int, Long64_t, bool) final { // no-op } diff --git a/tree/dataframe/inc/ROOT/RDF/RDefineReader.hxx b/tree/dataframe/inc/ROOT/RDF/RDefineReader.hxx index 754d4cdfcb5fe..6f708dabaabbc 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefineReader.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefineReader.hxx @@ -42,11 +42,9 @@ class R__CLING_PTRCHECK(off) RDefineReader final : public ROOT::Detail::RDF::RCo /// The slot this value belongs to. unsigned int fSlot = std::numeric_limits::max(); - void *GetImpl(Long64_t entry) final - { - fDefine.Update(fSlot, entry); - return fValuePtr; - } + void *GetImpl(std::size_t /*idx*/) final { return fValuePtr; } + + void LoadImpl(Long64_t entry, bool mask) final { fDefine.Update(fSlot, entry, mask); } public: RDefineReader(unsigned int slot, RDFDetail::RDefineBase &define) diff --git a/tree/dataframe/inc/ROOT/RDF/RFilter.hxx b/tree/dataframe/inc/ROOT/RDF/RFilter.hxx index a884fa6b631ba..c572406604300 100644 --- a/tree/dataframe/inc/ROOT/RDF/RFilter.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RFilter.hxx @@ -95,43 +95,47 @@ public: bool CheckFilters(unsigned int slot, Long64_t entry) final { - if (entry != fLastCheckedEntry[slot * RDFInternal::CacheLineStep()]) { - if (!fPrevNode.CheckFilters(slot, entry)) { - // a filter upstream returned false, cache the result - fLastResult[slot * RDFInternal::CacheLineStep()] = false; - } else { - // evaluate this filter, cache the result - auto passed = CheckFilterHelper(slot, entry, ColumnTypes_t{}, TypeInd_t{}); - passed ? ++fAccepted[slot * RDFInternal::CacheLineStep()] + auto &newMask = fLastResult[slot * RDFInternal::CacheLineStep()]; + auto &lastEntry = fLastCheckedEntry[slot * RDFInternal::CacheLineStep()]; + + if (entry != lastEntry) { + newMask = fPrevNode.CheckFilters(slot, entry); + + // evaluate this filter, cache the result + std::for_each(fValues[slot].begin(), fValues[slot].end(), + [entry, newMask](auto *v) { v->Load(entry, newMask); }); + CheckFilterHelper(slot, /*idx=*/0u, newMask, ColumnTypes_t{}, TypeInd_t{}); + + lastEntry = entry; + } + + return newMask; + } + + template + void CheckFilterHelper(unsigned int slot, std::size_t idx, int &entryMask, TypeList, + std::index_sequence) + { + if (entryMask) { + entryMask = fFilter(GetValueChecked(slot, S, idx)...); + entryMask ? ++fAccepted[slot * RDFInternal::CacheLineStep()] : ++fRejected[slot * RDFInternal::CacheLineStep()]; - fLastResult[slot * RDFInternal::CacheLineStep()] = passed; - } - fLastCheckedEntry[slot * RDFInternal::CacheLineStep()] = entry; } - return fLastResult[slot * RDFInternal::CacheLineStep()]; + (void)idx; // avoid unused parameter warning (gcc 12.1) } template - auto GetValueChecked(unsigned int slot, std::size_t readerIdx, Long64_t entry) -> ColType & + auto GetValueChecked(unsigned int slot, std::size_t readerIdx, std::size_t idx) -> ColType & { - if (auto *val = fValues[slot][readerIdx]->template TryGet(entry)) + if (auto *val = fValues[slot][readerIdx]->template TryGet(idx)) return *val; throw std::out_of_range{"RDataFrame: Filter could not retrieve value for column '" + fColumnNames[readerIdx] + - "' for entry " + std::to_string(entry) + + "' for entry " + std::to_string(idx) + ". You can use the DefaultValueFor operation to provide a default value, or " "FilterAvailable/FilterMissing to discard/keep entries with missing values instead."}; } - template - bool CheckFilterHelper(unsigned int slot, Long64_t entry, TypeList, std::index_sequence) - { - return fFilter(GetValueChecked(slot, S, entry)...); - // avoid unused parameter warnings (gcc 12.1) - (void)slot; - (void)entry; - } - void InitSlot(TTreeReader *r, unsigned int slot) final { RDFInternal::RColumnReadersInfo info{fColumnNames, fColRegister, fIsDefine.data(), *fLoopManager}; diff --git a/tree/dataframe/inc/ROOT/RDF/RFilterWithMissingValues.hxx b/tree/dataframe/inc/ROOT/RDF/RFilterWithMissingValues.hxx index 614513a5add6c..d265439206864 100644 --- a/tree/dataframe/inc/ROOT/RDF/RFilterWithMissingValues.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RFilterWithMissingValues.hxx @@ -102,24 +102,30 @@ public: constexpr static auto cacheLineStepint = RDFInternal::CacheLineStep(); constexpr static auto cacheLineStepULong64_t = RDFInternal::CacheLineStep(); - if (entry != fLastCheckedEntry[slot * cacheLineStepLong64_t]) { - if (!fPrevNodePtr->CheckFilters(slot, entry)) { - // a filter upstream returned false, cache the result - fLastResult[slot * cacheLineStepint] = false; - } else { - // evaluate this filter, cache the result - const bool valueIsMissing = fValues[slot]->template TryGet(entry) == nullptr; - if (fDiscardEntryWithMissingValue) { - valueIsMissing ? ++fRejected[slot * cacheLineStepULong64_t] : ++fAccepted[slot * cacheLineStepULong64_t]; - fLastResult[slot * cacheLineStepint] = !valueIsMissing; - } else { - valueIsMissing ? ++fAccepted[slot * cacheLineStepULong64_t] : ++fRejected[slot * cacheLineStepULong64_t]; - fLastResult[slot * cacheLineStepint] = valueIsMissing; - } - } - fLastCheckedEntry[slot * cacheLineStepLong64_t] = entry; + auto &newMask = fLastResult[slot * cacheLineStepint]; + auto &lastEntry = fLastCheckedEntry[slot * cacheLineStepLong64_t]; + + if (entry == lastEntry) + return newMask; + + newMask = fPrevNodePtr->CheckFilters(slot, entry); + if (!newMask) + return false; + + fValues[slot]->Load(entry, newMask); + + const bool valueIsMissing = fValues[slot]->template TryGet(/*idx=*/0u) == nullptr; + if (fDiscardEntryWithMissingValue) { + valueIsMissing ? ++fRejected[slot * cacheLineStepULong64_t] : ++fAccepted[slot * cacheLineStepULong64_t]; + newMask = !valueIsMissing; + } else { + valueIsMissing ? ++fAccepted[slot * cacheLineStepULong64_t] : ++fRejected[slot * cacheLineStepULong64_t]; + newMask = valueIsMissing; } - return fLastResult[slot * cacheLineStepint]; + + lastEntry = entry; + + return newMask; } void InitSlot(TTreeReader *r, unsigned int slot) final diff --git a/tree/dataframe/inc/ROOT/RDF/RJittedDefine.hxx b/tree/dataframe/inc/ROOT/RDF/RJittedDefine.hxx index 2bc79170de8f3..20592787fd2cc 100644 --- a/tree/dataframe/inc/ROOT/RDF/RJittedDefine.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RJittedDefine.hxx @@ -58,7 +58,7 @@ public: void InitSlot(TTreeReader *r, unsigned int slot) final; void *GetValuePtr(unsigned int slot) final; const std::type_info &GetTypeId() const final; - void Update(unsigned int slot, Long64_t entry) final; + void Update(unsigned int slot, Long64_t entry, bool mask) final; void Update(unsigned int slot, const ROOT::RDF::RSampleInfo &id) final; void FinalizeSlot(unsigned int slot) final; void MakeVariations(const std::vector &variations) final; diff --git a/tree/dataframe/inc/ROOT/RDF/RJittedVariation.hxx b/tree/dataframe/inc/ROOT/RDF/RJittedVariation.hxx index 0abd61df5109d..000aab52eb1a5 100644 --- a/tree/dataframe/inc/ROOT/RDF/RJittedVariation.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RJittedVariation.hxx @@ -43,7 +43,7 @@ public: void InitSlot(TTreeReader *r, unsigned int slot) final; void *GetValuePtr(unsigned int slot, const std::string &column, const std::string &variation) final; const std::type_info &GetTypeId() const final; - void Update(unsigned int slot, Long64_t entry) final; + void Update(unsigned int slot, Long64_t entry, bool mask) final; void FinalizeSlot(unsigned int slot) final; }; diff --git a/tree/dataframe/inc/ROOT/RDF/RLazyDSImpl.hxx b/tree/dataframe/inc/ROOT/RDF/RLazyDSImpl.hxx index 9f31d9d4a831e..3fe651520abd4 100644 --- a/tree/dataframe/inc/ROOT/RDF/RLazyDSImpl.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RLazyDSImpl.hxx @@ -27,7 +27,8 @@ namespace ROOT::Internal::RDF { class R__CLING_PTRCHECK(off) RLazyDSColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { ROOT::Internal::RDF::TPointerHolder *fPtr; - void *GetImpl(Long64_t) final { return fPtr->GetPointer(); } + void *GetImpl(std::size_t) final { return fPtr->GetPointer(); } + void LoadImpl(Long64_t, bool) final {} public: RLazyDSColumnReader(ROOT::Internal::RDF::TPointerHolder *ptr) : fPtr(ptr) {} diff --git a/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx b/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx index b2dc37c8e85a1..57ee799b1c83b 100644 --- a/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx @@ -38,7 +38,8 @@ namespace RDF { class R__CLING_PTRCHECK(off) RTreeOpaqueColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { std::unique_ptr fTreeValue; - void *GetImpl(Long64_t) override; + void *GetImpl(std::size_t) override; + void LoadImpl(Long64_t, bool) override; public: /// Construct the RTreeColumnReader. Actual initialization is performed lazily by the Init method. @@ -57,7 +58,8 @@ public: class R__CLING_PTRCHECK(off) RTreeUntypedValueColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { std::unique_ptr fTreeValue; - void *GetImpl(Long64_t) override; + void *GetImpl(std::size_t) override; + void LoadImpl(Long64_t, bool) override; public: RTreeUntypedValueColumnReader(TTreeReader &r, std::string_view colName, std::string_view typeName); @@ -111,11 +113,14 @@ private: /// Whether we already printed a warning about performing a copy of the TTreeReaderArray contents bool fCopyWarningPrinted = false; - void *GetImpl(Long64_t entry) override; + void *fValuePtr{nullptr}; - void *ReadStdArray(Long64_t entry); - void *ReadStdVector(Long64_t entry); - void *ReadRVec(Long64_t entry); + void *GetImpl(std::size_t) override; + void LoadImpl(Long64_t, bool) override; + + void *LoadStdArray(Long64_t entry); + void *LoadStdVector(Long64_t entry); + void *LoadRVec(Long64_t entry); }; class R__CLING_PTRCHECK(off) RMaskedColumnReader : public ROOT::Detail::RDF::RColumnReaderBase { @@ -123,7 +128,9 @@ class R__CLING_PTRCHECK(off) RMaskedColumnReader : public ROOT::Detail::RDF::RCo std::unique_ptr> fTreeValueMask; unsigned int fMaskIndex = 0; - void *GetImpl(Long64_t) override; + void *fValuePtr{nullptr}; + void *GetImpl(std::size_t) override; + void LoadImpl(Long64_t, bool) override; public: RMaskedColumnReader(TTreeReader &r, std::unique_ptr valueReader, diff --git a/tree/dataframe/inc/ROOT/RDF/RVariation.hxx b/tree/dataframe/inc/ROOT/RDF/RVariation.hxx index 0a3bc702d7e9e..ac7091f12aa7c 100644 --- a/tree/dataframe/inc/ROOT/RDF/RVariation.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RVariation.hxx @@ -160,23 +160,23 @@ class R__CLING_PTRCHECK(off) RVariation final : public RVariationBase { std::vector> fValues; template - auto GetValueChecked(unsigned int slot, std::size_t readerIdx, Long64_t entry) -> ColType & + auto GetValueChecked(unsigned int slot, std::size_t readerIdx, std::size_t idx) -> ColType & { - if (auto *val = fValues[slot][readerIdx]->template TryGet(entry)) + if (auto *val = fValues[slot][readerIdx]->template TryGet(idx)) return *val; throw std::out_of_range{"RDataFrame: Could not retrieve value for variation '" + fColNames[readerIdx] + - "' for entry " + std::to_string(entry) + + "' for entry " + std::to_string(idx) + ". You can use the DefaultValueFor operation to provide a default value, or " "FilterAvailable/FilterMissing to discard/keep entries with missing values instead."}; } template - void UpdateHelper(unsigned int slot, Long64_t entry, TypeList, std::index_sequence) + void UpdateHelper(unsigned int slot, std::size_t idx, TypeList, std::index_sequence) { // fExpression must return an RVec - auto &&results = fExpression(GetValueChecked(slot, S, entry)...); - (void)entry; // avoid unused parameter warnings (gcc 12.1) + auto &&results = fExpression(GetValueChecked(slot, S, idx)...); + (void)idx; // avoid unused parameter warnings (gcc 12.1) if (!ResultsSizeEq(results, fVariationNames.size(), fColNames.size(), std::integral_constant{})) { @@ -230,12 +230,15 @@ public: } /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry - void Update(unsigned int slot, Long64_t entry) final + void Update(unsigned int slot, Long64_t entry, bool mask) final { if (entry != fLastCheckedEntry[slot * CacheLineStep()]) { // evaluate this filter, cache the result - UpdateHelper(slot, entry, ColumnTypes_t{}, TypeInd_t{}); - fLastCheckedEntry[slot * CacheLineStep()] = entry; + std::for_each(fValues[slot].begin(), fValues[slot].end(), [entry, mask](auto *v) { v->Load(entry, mask); }); + if (mask) { + UpdateHelper(slot, /*idx=*/0u, ColumnTypes_t{}, TypeInd_t{}); + fLastCheckedEntry[slot * CacheLineStep()] = entry; + } } } diff --git a/tree/dataframe/inc/ROOT/RDF/RVariationBase.hxx b/tree/dataframe/inc/ROOT/RDF/RVariationBase.hxx index 34b54a9b0a245..46a9e4ce29e64 100644 --- a/tree/dataframe/inc/ROOT/RDF/RVariationBase.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RVariationBase.hxx @@ -70,7 +70,7 @@ public: const std::string &GetVariationName() const; std::string GetTypeName() const; /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry - virtual void Update(unsigned int slot, Long64_t entry) = 0; + virtual void Update(unsigned int slot, Long64_t entry, bool mask) = 0; /// Clean-up operations to be performed at the end of a task. virtual void FinalizeSlot(unsigned int slot) = 0; }; diff --git a/tree/dataframe/inc/ROOT/RDF/RVariationReader.hxx b/tree/dataframe/inc/ROOT/RDF/RVariationReader.hxx index a9e6dfacaafd6..a9ca2b9fcfdaf 100644 --- a/tree/dataframe/inc/ROOT/RDF/RVariationReader.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RVariationReader.hxx @@ -32,11 +32,9 @@ class R__CLING_PTRCHECK(off) RVariationReader final : public ROOT::Detail::RDF:: /// The slot this value belongs to. unsigned int fSlot = std::numeric_limits::max(); - void *GetImpl(Long64_t entry) final - { - fVariation->Update(fSlot, entry); - return fValuePtr; - } + void *GetImpl(std::size_t /*idx*/) final { return fValuePtr; } + + void LoadImpl(Long64_t entry, bool mask) final { fVariation->Update(fSlot, entry, mask); } public: RVariationReader(unsigned int slot, const std::string &colName, const std::string &variationName, diff --git a/tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx b/tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx index a14ec6df9ac3b..434f2558c78d2 100644 --- a/tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx @@ -141,31 +141,35 @@ public: } template - auto GetValueChecked(unsigned int slot, unsigned int varIdx, std::size_t readerIdx, Long64_t entry) -> ColType & + auto GetValueChecked(unsigned int slot, unsigned int varIdx, std::size_t readerIdx, std::size_t idx) -> ColType & { - if (auto *val = fInputValues[slot][varIdx][readerIdx]->template TryGet(entry)) + if (auto *val = fInputValues[slot][varIdx][readerIdx]->template TryGet(idx)) return *val; throw std::out_of_range{"RDataFrame: Varied action (" + fHelpers[0].GetActionName() + ") could not retrieve value for column '" + fColumnNames[readerIdx] + "' for entry " + - std::to_string(entry) + + std::to_string(idx) + ". You can use the DefaultValueFor operation to provide a default value, or " "FilterAvailable/FilterMissing to discard/keep entries with missing values instead."}; } template - void CallExec(unsigned int slot, unsigned int varIdx, Long64_t entry, TypeList, + void CallExec(unsigned int slot, unsigned int varIdx, std::size_t idx, TypeList, std::index_sequence) { - fHelpers[varIdx].Exec(slot, GetValueChecked(slot, varIdx, ReaderIdxs, entry)...); - (void)entry; + fHelpers[varIdx].Exec(slot, GetValueChecked(slot, varIdx, ReaderIdxs, idx)...); + (void)idx; } void Run(unsigned int slot, Long64_t entry) final { for (auto varIdx = 0u; varIdx < GetVariations().size(); ++varIdx) { - if (fPrevNodes[varIdx]->CheckFilters(slot, entry)) - CallExec(slot, varIdx, entry, ColumnTypes_t{}, TypeInd_t{}); + const auto mask = fPrevNodes[varIdx]->CheckFilters(slot, entry); + std::for_each(fInputValues[slot][varIdx].begin(), fInputValues[slot][varIdx].end(), + [entry, mask](auto *v) { v->Load(entry, mask); }); + + if (mask) + CallExec(slot, varIdx, /*idx=*/0u, ColumnTypes_t{}, TypeInd_t{}); } } diff --git a/tree/dataframe/inc/ROOT/RSqliteDS.hxx b/tree/dataframe/inc/ROOT/RSqliteDS.hxx index 5f46a5139dbda..a7a903eee15d0 100644 --- a/tree/dataframe/inc/ROOT/RSqliteDS.hxx +++ b/tree/dataframe/inc/ROOT/RSqliteDS.hxx @@ -22,7 +22,8 @@ namespace ROOT::Internal::RDF { class R__CLING_PTRCHECK(off) RSqliteDSColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { void *fValuePtr; - void *GetImpl(Long64_t) final { return fValuePtr; } + void *GetImpl(std::size_t) final { return fValuePtr; } + void LoadImpl(Long64_t, bool) final {} public: RSqliteDSColumnReader(void *valuePtr) : fValuePtr(valuePtr) {} diff --git a/tree/dataframe/inc/ROOT/RTrivialDS.hxx b/tree/dataframe/inc/ROOT/RTrivialDS.hxx index d7b7de89da425..97f40281b0ead 100644 --- a/tree/dataframe/inc/ROOT/RTrivialDS.hxx +++ b/tree/dataframe/inc/ROOT/RTrivialDS.hxx @@ -17,7 +17,8 @@ namespace ROOT::Internal::RDF { class R__CLING_PTRCHECK(off) RTrivialDSColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { ULong64_t *fValuePtr; - void *GetImpl(Long64_t) final { return fValuePtr; } + void *GetImpl(std::size_t) final { return fValuePtr; } + void LoadImpl(Long64_t, bool) final {} public: RTrivialDSColumnReader(ULong64_t *valuePtr) : fValuePtr(valuePtr) {} diff --git a/tree/dataframe/inc/ROOT/RVecDS.hxx b/tree/dataframe/inc/ROOT/RVecDS.hxx index 18f36f6942cd9..d3d27ba46a67c 100644 --- a/tree/dataframe/inc/ROOT/RVecDS.hxx +++ b/tree/dataframe/inc/ROOT/RVecDS.hxx @@ -30,7 +30,8 @@ namespace ROOT::Internal::RDF { class R__CLING_PTRCHECK(off) RVecDSColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { TPointerHolder *fPtrHolder; - void *GetImpl(Long64_t) final { return fPtrHolder->GetPointer(); } + void *GetImpl(std::size_t) final { return fPtrHolder->GetPointer(); } + void LoadImpl(Long64_t, bool) final {} public: RVecDSColumnReader(TPointerHolder *ptrHolder) : fPtrHolder(ptrHolder) {} diff --git a/tree/dataframe/src/RJittedDefine.cxx b/tree/dataframe/src/RJittedDefine.cxx index 0ead1a3b0ce1d..b094c3080b30f 100644 --- a/tree/dataframe/src/RJittedDefine.cxx +++ b/tree/dataframe/src/RJittedDefine.cxx @@ -39,10 +39,10 @@ const std::type_info &RJittedDefine::GetTypeId() const "retrieved. This should never happen, please report this as a bug."); } -void RJittedDefine::Update(unsigned int slot, Long64_t entry) +void RJittedDefine::Update(unsigned int slot, Long64_t entry, bool mask) { assert(fConcreteDefine != nullptr); - fConcreteDefine->Update(slot, entry); + fConcreteDefine->Update(slot, entry, mask); } void RJittedDefine::Update(unsigned int slot, const ROOT::RDF::RSampleInfo &id) diff --git a/tree/dataframe/src/RJittedVariation.cxx b/tree/dataframe/src/RJittedVariation.cxx index 50f43c7db64ff..663a87fdfddaf 100644 --- a/tree/dataframe/src/RJittedVariation.cxx +++ b/tree/dataframe/src/RJittedVariation.cxx @@ -34,10 +34,10 @@ const std::type_info &RJittedVariation::GetTypeId() const return fConcreteVariation->GetTypeId(); } -void RJittedVariation::Update(unsigned int slot, Long64_t entry) +void RJittedVariation::Update(unsigned int slot, Long64_t entry, bool mask) { assert(fConcreteVariation != nullptr); - fConcreteVariation->Update(slot, entry); + fConcreteVariation->Update(slot, entry, mask); } void RJittedVariation::FinalizeSlot(unsigned int slot) diff --git a/tree/dataframe/src/RNTupleDS.cxx b/tree/dataframe/src/RNTupleDS.cxx index 1628b5d6b7eb3..b92b3363a6d5a 100644 --- a/tree/dataframe/src/RNTupleDS.cxx +++ b/tree/dataframe/src/RNTupleDS.cxx @@ -137,13 +137,14 @@ class RNTupleColumnReader : public ROOT::Detail::RDF::RColumnReaderBase { fLastEntry = -1; } - void *GetImpl(Long64_t entry) final + void *GetImpl(std::size_t /*idx*/) final { return fValue->GetPtr().get(); } + + void LoadImpl(Long64_t entry, bool mask) final { - if (entry != fLastEntry) { + if (entry != fLastEntry && mask) { fValue->Read(entry - fEntryOffset); fLastEntry = entry; } - return fValue->GetPtr().get(); } }; } // namespace ROOT::Internal::RDF diff --git a/tree/dataframe/src/RTreeColumnReader.cxx b/tree/dataframe/src/RTreeColumnReader.cxx index 0786bb1a7c6af..37182a7c80cb9 100644 --- a/tree/dataframe/src/RTreeColumnReader.cxx +++ b/tree/dataframe/src/RTreeColumnReader.cxx @@ -5,11 +5,13 @@ #include -void *ROOT::Internal::RDF::RTreeOpaqueColumnReader::GetImpl(Long64_t) +void *ROOT::Internal::RDF::RTreeOpaqueColumnReader::GetImpl(std::size_t) { return fTreeValue->GetAddress(); } +void ROOT::Internal::RDF::RTreeOpaqueColumnReader::LoadImpl(Long64_t, bool) {} + ROOT::Internal::RDF::RTreeOpaqueColumnReader::RTreeOpaqueColumnReader(TTreeReader &r, std::string_view colName) : fTreeValue(std::make_unique(r, colName.data())) { @@ -17,11 +19,13 @@ ROOT::Internal::RDF::RTreeOpaqueColumnReader::RTreeOpaqueColumnReader(TTreeReade ROOT::Internal::RDF::RTreeOpaqueColumnReader::~RTreeOpaqueColumnReader() = default; -void *ROOT::Internal::RDF::RTreeUntypedValueColumnReader::GetImpl(Long64_t) +void *ROOT::Internal::RDF::RTreeUntypedValueColumnReader::GetImpl(std::size_t) { return fTreeValue->Get(); } +void ROOT::Internal::RDF::RTreeUntypedValueColumnReader::LoadImpl(Long64_t, bool) {} + ROOT::Internal::RDF::RTreeUntypedValueColumnReader::RTreeUntypedValueColumnReader(TTreeReader &r, std::string_view colName, std::string_view typeName) @@ -31,7 +35,7 @@ ROOT::Internal::RDF::RTreeUntypedValueColumnReader::RTreeUntypedValueColumnReade ROOT::Internal::RDF::RTreeUntypedValueColumnReader::~RTreeUntypedValueColumnReader() = default; -void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::ReadStdArray(Long64_t entry) +void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadStdArray(Long64_t entry) { if (entry == fLastEntry) return fRVec.data(); // We return the RVec we already created @@ -61,7 +65,7 @@ void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::ReadStdArray(Long64_t return fRVec.data(); } -void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::ReadStdVector(Long64_t entry) +void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadStdVector(Long64_t entry) { if (entry == fLastEntry) return &fStdVector; // We return the std::vector we already created @@ -95,7 +99,7 @@ void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::ReadStdVector(Long64_t return &fStdVector; } -void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::ReadRVec(Long64_t entry) +void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadRVec(Long64_t entry) { if (entry == fLastEntry) return &fRVec; // We return the RVec we already created @@ -160,15 +164,21 @@ void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::ReadRVec(Long64_t entr return &fRVec; } -void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::GetImpl(Long64_t entry) +void ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadImpl(Long64_t entry, bool mask) { - if (fCollectionType == ECollectionType::kStdArray) - return ReadStdArray(entry); - - if (fCollectionType == ECollectionType::kStdVector) - return ReadStdVector(entry); + if (entry != fLastEntry && mask) { + if (fCollectionType == ECollectionType::kStdArray) + fValuePtr = LoadStdArray(entry); + else if (fCollectionType == ECollectionType::kStdVector) + fValuePtr = LoadStdVector(entry); + else + fValuePtr = LoadRVec(entry); + } +} - return ReadRVec(entry); +void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::GetImpl(std::size_t /*idx*/) +{ + return fValuePtr; } ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::RTreeUntypedArrayColumnReader(TTreeReader &r, @@ -193,11 +203,24 @@ ROOT::Internal::RDF::RMaskedColumnReader::RMaskedColumnReader( ROOT::Internal::RDF::RMaskedColumnReader::~RMaskedColumnReader() = default; -void *ROOT::Internal::RDF::RMaskedColumnReader::GetImpl(Long64_t event) +void *ROOT::Internal::RDF::RMaskedColumnReader::GetImpl(std::size_t) { - const std::bitset<64> mask{*fTreeValueMask->Get()}; - if (mask.test(fMaskIndex) == false) - return nullptr; + return fValuePtr; +} + +void ROOT::Internal::RDF::RMaskedColumnReader::LoadImpl(Long64_t entry, bool mask) +{ + if (!mask) { + fValuePtr = nullptr; + return; + } + + const std::bitset<64> treeMask{*fTreeValueMask->Get()}; + if (treeMask.test(fMaskIndex) == false) { + fValuePtr = nullptr; + return; + } - return fValueReader->TryGet(event); + fValueReader->Load(entry, mask); + fValuePtr = fValueReader->TryGet(/*idx=*/0u); } diff --git a/tree/dataframe/test/RArraysDS.hxx b/tree/dataframe/test/RArraysDS.hxx index 5e0e654c6f43d..c21c09a673509 100644 --- a/tree/dataframe/test/RArraysDS.hxx +++ b/tree/dataframe/test/RArraysDS.hxx @@ -12,7 +12,8 @@ class R__CLING_PTRCHECK(off) RArraysDSVarReader final : public ROOT::Detail::RDF::RColumnReaderBase { std::vector *fPtr = nullptr; - void *GetImpl(Long64_t) final { return fPtr; } + void *GetImpl(std::size_t) final { return fPtr; } + void LoadImpl(Long64_t, bool) final {} public: RArraysDSVarReader(std::vector &v) : fPtr(&v) {} @@ -21,11 +22,12 @@ public: class R__CLING_PTRCHECK(off) RArraysDSVarSizeReader final : public ROOT::Detail::RDF::RColumnReaderBase { std::vector *fPtr = nullptr; std::size_t fSize = 0; - void *GetImpl(Long64_t) final + void *GetImpl(std::size_t) final { fSize = fPtr->size(); return &fSize; } + void LoadImpl(Long64_t, bool) final {} public: RArraysDSVarSizeReader(std::vector &v) : fPtr(&v) {} From 5385e0a00497a460849fb16b2f48f22fae454afe Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Tue, 2 Jun 2026 11:37:15 +0200 Subject: [PATCH 2/8] [df] Prepare for bulk filters Nodes of the computation graph which may operate a selection on which entries are valid or not are modified to accommodate for multiple entries being evaluated at the same time. For now, the size of the bulk is set to one. Note that this commit does not touch in any way the reading of data. --- roottest/root/dataframe/testIMT.cxx | 20 ++++--- tree/dataframe/CMakeLists.txt | 1 + tree/dataframe/inc/ROOT/RCsvDS.hxx | 2 +- tree/dataframe/inc/ROOT/RDF/RAction.hxx | 5 +- .../inc/ROOT/RDF/RActionSnapshot.hxx | 15 ++++-- .../inc/ROOT/RDF/RColumnReaderBase.hxx | 13 ++--- .../inc/ROOT/RDF/RDSColumnReader.hxx | 2 +- .../inc/ROOT/RDF/RDefaultValueFor.hxx | 20 ++++--- tree/dataframe/inc/ROOT/RDF/RDefine.hxx | 45 +++++++++------- tree/dataframe/inc/ROOT/RDF/RDefineBase.hxx | 7 ++- .../inc/ROOT/RDF/RDefinePerSample.hxx | 2 +- tree/dataframe/inc/ROOT/RDF/RDefineReader.hxx | 2 +- tree/dataframe/inc/ROOT/RDF/RFilter.hxx | 51 ++++++++++-------- tree/dataframe/inc/ROOT/RDF/RFilterBase.hxx | 4 +- .../inc/ROOT/RDF/RFilterWithMissingValues.hxx | 52 ++++++++++--------- tree/dataframe/inc/ROOT/RDF/RJittedDefine.hxx | 2 +- tree/dataframe/inc/ROOT/RDF/RJittedFilter.hxx | 2 +- .../inc/ROOT/RDF/RJittedVariation.hxx | 2 +- tree/dataframe/inc/ROOT/RDF/RLazyDSImpl.hxx | 2 +- tree/dataframe/inc/ROOT/RDF/RLoopManager.hxx | 2 +- .../inc/ROOT/RDF/RMaskedEntryRange.hxx | 38 ++++++++++++++ tree/dataframe/inc/ROOT/RDF/RNodeBase.hxx | 11 ++-- tree/dataframe/inc/ROOT/RDF/RRange.hxx | 46 +++++++++------- tree/dataframe/inc/ROOT/RDF/RRangeBase.hxx | 4 +- .../inc/ROOT/RDF/RTreeColumnReader.hxx | 10 ++-- tree/dataframe/inc/ROOT/RDF/RVariation.hxx | 18 ++++--- .../dataframe/inc/ROOT/RDF/RVariationBase.hxx | 3 +- .../inc/ROOT/RDF/RVariationReader.hxx | 2 +- tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx | 5 +- .../inc/ROOT/RDF/SnapshotHelpers.hxx | 3 +- tree/dataframe/inc/ROOT/RSqliteDS.hxx | 2 +- tree/dataframe/inc/ROOT/RTrivialDS.hxx | 2 +- tree/dataframe/inc/ROOT/RVecDS.hxx | 2 +- tree/dataframe/src/RDFSnapshotHelpers.cxx | 10 ++-- tree/dataframe/src/RFilterBase.cxx | 13 +++-- tree/dataframe/src/RJittedDefine.cxx | 4 +- tree/dataframe/src/RJittedFilter.cxx | 2 +- tree/dataframe/src/RJittedVariation.cxx | 4 +- tree/dataframe/src/RLoopManager.cxx | 5 +- tree/dataframe/src/RNTupleDS.cxx | 11 ++-- tree/dataframe/src/RRangeBase.cxx | 8 ++- tree/dataframe/src/RTreeColumnReader.cxx | 37 ++++++++----- tree/dataframe/test/RArraysDS.hxx | 4 +- 43 files changed, 297 insertions(+), 198 deletions(-) create mode 100644 tree/dataframe/inc/ROOT/RDF/RMaskedEntryRange.hxx diff --git a/roottest/root/dataframe/testIMT.cxx b/roottest/root/dataframe/testIMT.cxx index 74648ac7bff1b..9fa1d23b5adec 100644 --- a/roottest/root/dataframe/testIMT.cxx +++ b/roottest/root/dataframe/testIMT.cxx @@ -55,18 +55,18 @@ void getTracks(unsigned int mu, FourVectors& tracks) { // This makes the example stand-alone void FillTree(const char* filename, const char* treeName) { if (!gSystem->AccessPathName(filename)) return; - TFile f(filename,"RECREATE"); - TTree t(treeName,treeName); + auto f = std::make_unique(filename, "RECREATE"); + auto t = std::make_unique(treeName, treeName); double b1; int b2; std::vector tracks; std::vector dv {-1,2,3,4}; std::list sl {1,2,3,4}; - t.Branch("b1", &b1); - t.Branch("b2", &b2); - t.Branch("tracks", &tracks); - t.Branch("dv", &dv); - t.Branch("sl", &sl); + t->Branch("b1", &b1); + t->Branch("b2", &b2); + t->Branch("tracks", &tracks); + t->Branch("dv", &dv); + t->Branch("sl", &sl); int nevts = 16000; for(int i = 0; i < nevts; ++i) { @@ -77,11 +77,9 @@ void FillTree(const char* filename, const char* treeName) { dv.emplace_back(i); sl.emplace_back(i); - t.Fill(); + t->Fill(); } - t.Write(); - f.Close(); - return; + f->Write(); } auto fileName = "testIMT.root"; diff --git a/tree/dataframe/CMakeLists.txt b/tree/dataframe/CMakeLists.txt index 506151d24ab81..f0d2ef328aba0 100644 --- a/tree/dataframe/CMakeLists.txt +++ b/tree/dataframe/CMakeLists.txt @@ -84,6 +84,7 @@ ROOT_STANDARD_LIBRARY_PACKAGE(ROOTDataFrame ROOT/RDF/RJittedVariation.hxx ROOT/RDF/RLazyDSImpl.hxx ROOT/RDF/RLoopManager.hxx + ROOT/RDF/RMaskedEntryRange.hxx ROOT/RDF/RMergeableValue.hxx ROOT/RDF/RMetaData.hxx ROOT/RDF/RNodeBase.hxx diff --git a/tree/dataframe/inc/ROOT/RCsvDS.hxx b/tree/dataframe/inc/ROOT/RCsvDS.hxx index d29e7d44c345c..592aa94eeba8b 100644 --- a/tree/dataframe/inc/ROOT/RCsvDS.hxx +++ b/tree/dataframe/inc/ROOT/RCsvDS.hxx @@ -28,7 +28,7 @@ namespace ROOT::Internal::RDF { class R__CLING_PTRCHECK(off) RCsvDSColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { void *fValuePtr; void *GetImpl(std::size_t) final { return fValuePtr; } - void LoadImpl(Long64_t, bool) final {} + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) final {} public: RCsvDSColumnReader(void *valuePtr) : fValuePtr(valuePtr) {} diff --git a/tree/dataframe/inc/ROOT/RDF/RAction.hxx b/tree/dataframe/inc/ROOT/RDF/RAction.hxx index 6fae6281fd8d0..d52968236e671 100644 --- a/tree/dataframe/inc/ROOT/RDF/RAction.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RAction.hxx @@ -122,9 +122,10 @@ public: void Run(unsigned int slot, Long64_t entry) final { const auto mask = fPrevNode.CheckFilters(slot, entry); - std::for_each(fValues[slot].begin(), fValues[slot].end(), [entry, mask](auto *v) { v->Load(entry, mask); }); + std::for_each(fValues[slot].begin(), fValues[slot].end(), [&mask](auto *v) { v->Load(mask); }); - if (mask) + // Assume 1-size bulk for now + if (mask[0]) CallExec(slot, /*idx=*/0u, ColumnTypes_t{}, TypeInd_t{}); } diff --git a/tree/dataframe/inc/ROOT/RDF/RActionSnapshot.hxx b/tree/dataframe/inc/ROOT/RDF/RActionSnapshot.hxx index 3e30013461e7f..919bb9bb62142 100644 --- a/tree/dataframe/inc/ROOT/RDF/RActionSnapshot.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RActionSnapshot.hxx @@ -195,19 +195,23 @@ public: { if constexpr (std::is_same_v) { // check if entry passes all filters - std::vector filterPassed(fPrevNodes.size(), false); + std::vector filterPassed(fPrevNodes.size(), 1ul); for (unsigned int variation = 0; variation < fPrevNodes.size(); ++variation) { filterPassed[variation] = fPrevNodes[variation]->CheckFilters(slot, entry); } // Currently, every event where any of nominal or variations pass gets written to the output. // This logic could be extended for different use cases if the need arises. - if (std::any_of(filterPassed.begin(), filterPassed.end(), [](bool val) { return val; })) { + // Assume 1-size bulk for now + if (std::any_of(filterPassed.begin(), filterPassed.end(), + [](const ROOT::Internal::RDF::RMaskedEntryRange &val) { return val[0]; })) { // TODO: Don't allocate std::vector untypedValues; auto nReaders = fValues[slot].size(); untypedValues.reserve(nReaders); - std::for_each(fValues[slot].begin(), fValues[slot].end(), [entry](auto *v) { v->Load(entry, true); }); + std::for_each(fValues[slot].begin(), fValues[slot].end(), [entry](auto *v) { + v->Load(ROOT::Internal::RDF::RMaskedEntryRange{1ul, true, static_cast(entry)}); + }); for (decltype(nReaders) readerIdx{}; readerIdx < nReaders; readerIdx++) untypedValues.push_back(GetValue(slot, readerIdx, /*idx=*/0u)); @@ -215,8 +219,9 @@ public: } } else { const auto mask = fPrevNodes.front()->CheckFilters(slot, entry); - std::for_each(fValues[slot].begin(), fValues[slot].end(), [entry, mask](auto *v) { v->Load(entry, mask); }); - if (mask) + std::for_each(fValues[slot].begin(), fValues[slot].end(), [&mask](auto *v) { v->Load(mask); }); + // Assume 1-size bulk for now + if (mask[0]) CallExec(slot, /*idx=*/0u); } } diff --git a/tree/dataframe/inc/ROOT/RDF/RColumnReaderBase.hxx b/tree/dataframe/inc/ROOT/RDF/RColumnReaderBase.hxx index a129a9f261e87..be5f8a6847f55 100644 --- a/tree/dataframe/inc/ROOT/RDF/RColumnReaderBase.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RColumnReaderBase.hxx @@ -12,6 +12,7 @@ #define ROOT_INTERNAL_RDF_RCOLUMNREADERBASE #include +#include namespace ROOT { namespace Detail { @@ -26,7 +27,6 @@ This pure virtual class provides a common base class for the different column re RDSColumnReader. **/ class R__CLING_PTRCHECK(off) RColumnReaderBase { - Long64_t fLoadedEntry = -1; public: virtual ~RColumnReaderBase() = default; @@ -34,14 +34,7 @@ public: /// Load the column value for the given entry. /// \param entry The entry number to load. /// \param mask The entry mask. Values will be loaded only for entries for which the mask equals true. - void Load(Long64_t entry, bool mask) - { - // For now, as `mask` is just a single boolean, as an optimization we can return early here if `mask == false`. - if (mask) { - fLoadedEntry = entry; - this->LoadImpl(entry, mask); - } - } + void Load(const ROOT::Internal::RDF::RMaskedEntryRange &mask) { LoadImpl(mask); } /// Return the column value for the given entry. /// \tparam T The column type @@ -57,7 +50,7 @@ public: private: virtual void *GetImpl(std::size_t idx) = 0; - virtual void LoadImpl(Long64_t /*entry*/, bool /*mask*/) = 0; + virtual void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) = 0; }; } // namespace RDF diff --git a/tree/dataframe/inc/ROOT/RDF/RDSColumnReader.hxx b/tree/dataframe/inc/ROOT/RDF/RDSColumnReader.hxx index 571154d80497b..fe741579a6250 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDSColumnReader.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDSColumnReader.hxx @@ -24,7 +24,7 @@ class R__CLING_PTRCHECK(off) RDSColumnReader final : public ROOT::Detail::RDF::R T **fDSValuePtr = nullptr; void *GetImpl(std::size_t) final { return *fDSValuePtr; } - void LoadImpl(Long64_t, bool) final {} + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) final {} public: RDSColumnReader(void *DSValuePtr) : fDSValuePtr(static_cast(DSValuePtr)) {} diff --git a/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx b/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx index 864a38583ecce..a7e5f6a090475 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx @@ -104,16 +104,20 @@ public: } /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry - void Update(unsigned int slot, Long64_t entry, bool mask) final + void Update(unsigned int slot, const ROOT::Internal::RDF::RMaskedEntryRange &mask) final { - if (entry != fLastCheckedEntry[slot * RDFInternal::CacheLineStep()]) { - // evaluate this define expression, cache the result - fValues[slot]->Load(entry, mask); - if (mask) { - fLastResults[slot * RDFInternal::CacheLineStep()] = GetValueOrDefault(slot, /*idx=*/0u); - fLastCheckedEntry[slot * RDFInternal::CacheLineStep()] = entry; - } + if (static_cast(mask.GetFirstEntry()) == + fLastCheckedEntry[slot * RDFInternal::CacheLineStep()]) + return; + + // Assume 1-size bulk for now + fValues[slot]->Load(mask); + const std::size_t bulkSize = 1; + for (std::size_t i = 0; i < bulkSize; ++i) { + if (mask[i]) + fLastResults[slot * RDFInternal::CacheLineStep()] = GetValueOrDefault(slot, i); } + fLastCheckedEntry[slot * RDFInternal::CacheLineStep()] = mask.GetFirstEntry(); } void Update(unsigned int /*slot*/, const ROOT::RDF::RSampleInfo & /*id*/) final {} diff --git a/tree/dataframe/inc/ROOT/RDF/RDefine.hxx b/tree/dataframe/inc/ROOT/RDF/RDefine.hxx index 1350e3989505c..2c2eb8cb15620 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefine.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefine.hxx @@ -83,31 +83,31 @@ class R__CLING_PTRCHECK(off) RDefine final : public RDefineBase { } template - void UpdateHelper(unsigned int slot, std::size_t idx, Long64_t /*entry*/, TypeList, + auto UpdateHelper(unsigned int slot, std::size_t idx, Long64_t /*entry*/, TypeList, std::index_sequence, NoneTag) { - fLastResults[slot * RDFInternal::CacheLineStep()] = - fExpression(GetValueChecked(slot, S, idx)...); - (void)idx; // avoid unused parameter warning (gcc 12.1) + return fExpression(GetValueChecked(slot, S, idx)...); + (void)slot; // avoid unused parameter warning + (void)idx; // avoid unused parameter warning } template - void UpdateHelper(unsigned int slot, std::size_t idx, Long64_t /*entry*/, TypeList, + auto UpdateHelper(unsigned int slot, std::size_t idx, Long64_t /*entry*/, TypeList, std::index_sequence, SlotTag) { - fLastResults[slot * RDFInternal::CacheLineStep()] = - fExpression(slot, GetValueChecked(slot, S, idx)...); - (void)idx; // avoid unused parameter warning (gcc 12.1) + return fExpression(slot, GetValueChecked(slot, S, idx)...); + (void)slot; // avoid unused parameter warning + (void)idx; // avoid unused parameter warning } template - void UpdateHelper(unsigned int slot, std::size_t idx, Long64_t batchFirstEntry, TypeList, + auto UpdateHelper(unsigned int slot, std::size_t idx, Long64_t entryInBatch, TypeList, std::index_sequence, SlotAndEntryTag) { - fLastResults[slot * RDFInternal::CacheLineStep()] = - fExpression(slot, batchFirstEntry + idx, GetValueChecked(slot, S, idx)...); - (void)idx; // avoid unused parameter warning (gcc 12.1) - (void)batchFirstEntry; // avoid unused parameter warning (gcc 12.1) + return fExpression(slot, entryInBatch, GetValueChecked(slot, S, idx)...); + (void)slot; // avoid unused parameter warning + (void)idx; // avoid unused parameter warning + (void)entryInBatch; // avoid unused parameter warning } public: @@ -138,13 +138,20 @@ public: } /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry - void Update(unsigned int slot, Long64_t entry, bool mask) final + void Update(unsigned int slot, const ROOT::Internal::RDF::RMaskedEntryRange &mask) final { - if (entry != fLastCheckedEntry[slot * RDFInternal::CacheLineStep()]) { - std::for_each(fValues[slot].begin(), fValues[slot].end(), [entry, mask](auto *v) { v->Load(entry, mask); }); - if (mask) { - UpdateHelper(slot, /*idx=*/0u, entry, ColumnTypes_t{}, TypeInd_t{}, ExtraArgsTag{}); - fLastCheckedEntry[slot * RDFInternal::CacheLineStep()] = entry; + if (static_cast(mask.GetFirstEntry()) == + fLastCheckedEntry[slot * RDFInternal::CacheLineStep()]) + return; + + std::for_each(fValues[slot].begin(), fValues[slot].end(), [&mask](auto *v) { v->Load(mask); }); + // Assume 1-size bulk for now + const std::size_t bulkSize = 1; + auto &result = fLastResults[slot * RDFInternal::CacheLineStep()]; + for (std::size_t i = 0; i < bulkSize; ++i) { + if (mask[i]) { + result = UpdateHelper(slot, i, mask.GetFirstEntry() + i, ColumnTypes_t{}, TypeInd_t{}, ExtraArgsTag{}); + fLastCheckedEntry[slot * RDFInternal::CacheLineStep()] = mask.GetFirstEntry(); } } } diff --git a/tree/dataframe/inc/ROOT/RDF/RDefineBase.hxx b/tree/dataframe/inc/ROOT/RDF/RDefineBase.hxx index 058db69fc1d2a..730552a5c762b 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefineBase.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefineBase.hxx @@ -16,6 +16,7 @@ #include "ROOT/RDF/RSampleInfo.hxx" #include "ROOT/RDF/Utils.hxx" #include "ROOT/RVec.hxx" +#include #include #include @@ -40,7 +41,9 @@ class RDefineBase { protected: const std::string fName; ///< The name of the custom column const std::string fType; ///< The type of the custom column as a text string - std::vector fLastCheckedEntry; + std::vector fLastCheckedEntry; /// Starting entry of the last bulk processed, per slot + /// Which entries in the current bulk are valid, per slot + std::vector fMaskPerSlot; RDFInternal::RColumnRegister fColRegister; RLoopManager *fLoopManager; // non-owning pointer to the RLoopManager const ROOT::RDF::ColumnNames_t fColumnNames; @@ -63,7 +66,7 @@ public: std::string GetName() const; std::string GetTypeName() const; /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry - virtual void Update(unsigned int slot, Long64_t entry, bool mask) = 0; + virtual void Update(unsigned int slot, const ROOT::Internal::RDF::RMaskedEntryRange &mask) = 0; /// Update function to be called once per sample, used if the derived type is a RDefinePerSample virtual void Update(unsigned int /*slot*/, const ROOT::RDF::RSampleInfo &/*id*/) {} /// Clean-up operations to be performed at the end of a task. diff --git a/tree/dataframe/inc/ROOT/RDF/RDefinePerSample.hxx b/tree/dataframe/inc/ROOT/RDF/RDefinePerSample.hxx index 4a962848a540f..afa62a9670f35 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefinePerSample.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefinePerSample.hxx @@ -59,7 +59,7 @@ public: return static_cast(&fLastResults[slot * RDFInternal::CacheLineStep()]); } - void Update(unsigned int, Long64_t, bool) final + void Update(unsigned int, const ROOT::Internal::RDF::RMaskedEntryRange &) final { // no-op } diff --git a/tree/dataframe/inc/ROOT/RDF/RDefineReader.hxx b/tree/dataframe/inc/ROOT/RDF/RDefineReader.hxx index 6f708dabaabbc..2b10460e0a725 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefineReader.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefineReader.hxx @@ -44,7 +44,7 @@ class R__CLING_PTRCHECK(off) RDefineReader final : public ROOT::Detail::RDF::RCo void *GetImpl(std::size_t /*idx*/) final { return fValuePtr; } - void LoadImpl(Long64_t entry, bool mask) final { fDefine.Update(fSlot, entry, mask); } + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &mask) final { fDefine.Update(fSlot, mask); } public: RDefineReader(unsigned int slot, RDFDetail::RDefineBase &define) diff --git a/tree/dataframe/inc/ROOT/RDF/RFilter.hxx b/tree/dataframe/inc/ROOT/RDF/RFilter.hxx index c572406604300..e381c84ff4992 100644 --- a/tree/dataframe/inc/ROOT/RDF/RFilter.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RFilter.hxx @@ -93,35 +93,42 @@ public: fLoopManager->Deregister(this); } - bool CheckFilters(unsigned int slot, Long64_t entry) final + ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int slot, Long64_t entry) final { - auto &newMask = fLastResult[slot * RDFInternal::CacheLineStep()]; - auto &lastEntry = fLastCheckedEntry[slot * RDFInternal::CacheLineStep()]; - - if (entry != lastEntry) { - newMask = fPrevNode.CheckFilters(slot, entry); - - // evaluate this filter, cache the result - std::for_each(fValues[slot].begin(), fValues[slot].end(), - [entry, newMask](auto *v) { v->Load(entry, newMask); }); - CheckFilterHelper(slot, /*idx=*/0u, newMask, ColumnTypes_t{}, TypeInd_t{}); - - lastEntry = entry; + auto &cachedResults = fCachedResults[slot * RDFInternal::CacheLineStep>()]; + if (entry == fLastCheckedEntry[slot * ROOT::Internal::RDF::CacheLineStep()]) + return {cachedResults, static_cast(entry)}; + + auto mask = fPrevNode.CheckFilters(slot, entry); + std::for_each(fValues[slot].begin(), fValues[slot].end(), [&mask](auto *v) { v->Load(mask); }); + // Assume 1-size bulk for now + const std::size_t bulkSize{1}; + std::size_t accepted{0}; + std::size_t rejected{0}; + cachedResults.clear(); + cachedResults.resize(bulkSize); + for (std::size_t i = 0; i < bulkSize; ++i) { + if (mask[i]) { + cachedResults[i] = CheckFilterHelper(slot, i, ColumnTypes_t{}, TypeInd_t{}); + if (cachedResults[i]) + ++accepted; + else + ++rejected; + } } + fLastCheckedEntry[slot * ROOT::Internal::RDF::CacheLineStep()] = entry; + fAccepted[slot * RDFInternal::CacheLineStep()] += accepted; + fRejected[slot * RDFInternal::CacheLineStep()] += rejected; - return newMask; + return {cachedResults, static_cast(entry)}; } template - void CheckFilterHelper(unsigned int slot, std::size_t idx, int &entryMask, TypeList, - std::index_sequence) + bool CheckFilterHelper(unsigned int slot, std::size_t idx, TypeList, std::index_sequence) { - if (entryMask) { - entryMask = fFilter(GetValueChecked(slot, S, idx)...); - entryMask ? ++fAccepted[slot * RDFInternal::CacheLineStep()] - : ++fRejected[slot * RDFInternal::CacheLineStep()]; - } - (void)idx; // avoid unused parameter warning (gcc 12.1) + return fFilter(GetValueChecked(slot, S, idx)...); + (void)slot; // avoid unused parameter warning + (void)idx; // avoid unused parameter warning } template diff --git a/tree/dataframe/inc/ROOT/RDF/RFilterBase.hxx b/tree/dataframe/inc/ROOT/RDF/RFilterBase.hxx index dffbb6a46651a..72650e599f9d5 100644 --- a/tree/dataframe/inc/ROOT/RDF/RFilterBase.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RFilterBase.hxx @@ -37,8 +37,6 @@ class RLoopManager; class RFilterBase : public RNodeBase { protected: - std::vector fLastCheckedEntry; - std::vector fLastResult = {true}; // std::vector cannot be used in a MT context safely std::vector fAccepted = {0}; std::vector fRejected = {0}; const std::string fName; @@ -49,6 +47,8 @@ protected: std::string fVariation; ///< This indicates for what variation this filter evaluates values. std::unordered_map> fVariedFilters; + std::vector> fCachedResults{}; // Vector of cached results, per slot + public: RFilterBase(RLoopManager *df, std::string_view name, const unsigned int nSlots, const RDFInternal::RColumnRegister &colRegister, const ColumnNames_t &columns, diff --git a/tree/dataframe/inc/ROOT/RDF/RFilterWithMissingValues.hxx b/tree/dataframe/inc/ROOT/RDF/RFilterWithMissingValues.hxx index d265439206864..a88bb1821ff48 100644 --- a/tree/dataframe/inc/ROOT/RDF/RFilterWithMissingValues.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RFilterWithMissingValues.hxx @@ -96,36 +96,40 @@ public: fLoopManager->EraseSuppressErrorsForMissingBranch(fColumnNames[0]); } - bool CheckFilters(unsigned int slot, Long64_t entry) final + ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int slot, Long64_t entry) final { constexpr static auto cacheLineStepLong64_t = RDFInternal::CacheLineStep(); - constexpr static auto cacheLineStepint = RDFInternal::CacheLineStep(); constexpr static auto cacheLineStepULong64_t = RDFInternal::CacheLineStep(); - auto &newMask = fLastResult[slot * cacheLineStepint]; - auto &lastEntry = fLastCheckedEntry[slot * cacheLineStepLong64_t]; - - if (entry == lastEntry) - return newMask; - - newMask = fPrevNodePtr->CheckFilters(slot, entry); - if (!newMask) - return false; - - fValues[slot]->Load(entry, newMask); - - const bool valueIsMissing = fValues[slot]->template TryGet(/*idx=*/0u) == nullptr; - if (fDiscardEntryWithMissingValue) { - valueIsMissing ? ++fRejected[slot * cacheLineStepULong64_t] : ++fAccepted[slot * cacheLineStepULong64_t]; - newMask = !valueIsMissing; - } else { - valueIsMissing ? ++fAccepted[slot * cacheLineStepULong64_t] : ++fRejected[slot * cacheLineStepULong64_t]; - newMask = valueIsMissing; + if (entry == fLastCheckedEntry[slot * cacheLineStepLong64_t]) + return {fCachedResults[slot], static_cast(entry)}; + + // Assume 1-size bulk for now + const std::size_t bulkSize{1}; + auto mask = fPrevNodePtr->CheckFilters(slot, entry); + + fValues[slot]->Load(mask); + + std::size_t accepted{0}; + std::size_t rejected{0}; + fCachedResults[slot].clear(); + fCachedResults[slot].resize(bulkSize); + for (std::size_t i = 0; i < bulkSize; i++) { + + const bool valueIsMissing = fValues[slot]->template TryGet(i) == nullptr; + if (fDiscardEntryWithMissingValue) { + valueIsMissing ? ++rejected : ++accepted; + fCachedResults[slot][i] = mask[i] && !valueIsMissing; + } else { + valueIsMissing ? ++accepted : ++rejected; + fCachedResults[slot][i] = mask[i] && valueIsMissing; + } } + fAccepted[slot * cacheLineStepULong64_t] += accepted; + fRejected[slot * cacheLineStepULong64_t] += rejected; + fLastCheckedEntry[slot * cacheLineStepLong64_t] = entry; - lastEntry = entry; - - return newMask; + return {fCachedResults[slot], static_cast(entry)}; } void InitSlot(TTreeReader *r, unsigned int slot) final diff --git a/tree/dataframe/inc/ROOT/RDF/RJittedDefine.hxx b/tree/dataframe/inc/ROOT/RDF/RJittedDefine.hxx index 20592787fd2cc..4b470d5a761af 100644 --- a/tree/dataframe/inc/ROOT/RDF/RJittedDefine.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RJittedDefine.hxx @@ -58,7 +58,7 @@ public: void InitSlot(TTreeReader *r, unsigned int slot) final; void *GetValuePtr(unsigned int slot) final; const std::type_info &GetTypeId() const final; - void Update(unsigned int slot, Long64_t entry, bool mask) final; + void Update(unsigned int slot, const ROOT::Internal::RDF::RMaskedEntryRange &mask) final; void Update(unsigned int slot, const ROOT::RDF::RSampleInfo &id) final; void FinalizeSlot(unsigned int slot) final; void MakeVariations(const std::vector &variations) final; diff --git a/tree/dataframe/inc/ROOT/RDF/RJittedFilter.hxx b/tree/dataframe/inc/ROOT/RDF/RJittedFilter.hxx index b01325ed2e68e..42871a33fe669 100644 --- a/tree/dataframe/inc/ROOT/RDF/RJittedFilter.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RJittedFilter.hxx @@ -55,7 +55,7 @@ public: void SetFilter(std::unique_ptr f); void InitSlot(TTreeReader *r, unsigned int slot) final; - bool CheckFilters(unsigned int slot, Long64_t entry) final; + ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int, Long64_t) final; void Report(ROOT::RDF::RCutFlowReport &) const final; void PartialReport(ROOT::RDF::RCutFlowReport &) const final; void FillReport(ROOT::RDF::RCutFlowReport &) const final; diff --git a/tree/dataframe/inc/ROOT/RDF/RJittedVariation.hxx b/tree/dataframe/inc/ROOT/RDF/RJittedVariation.hxx index 000aab52eb1a5..f6b4cec863861 100644 --- a/tree/dataframe/inc/ROOT/RDF/RJittedVariation.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RJittedVariation.hxx @@ -43,7 +43,7 @@ public: void InitSlot(TTreeReader *r, unsigned int slot) final; void *GetValuePtr(unsigned int slot, const std::string &column, const std::string &variation) final; const std::type_info &GetTypeId() const final; - void Update(unsigned int slot, Long64_t entry, bool mask) final; + void Update(unsigned int slot, const ROOT::Internal::RDF::RMaskedEntryRange &mask) final; void FinalizeSlot(unsigned int slot) final; }; diff --git a/tree/dataframe/inc/ROOT/RDF/RLazyDSImpl.hxx b/tree/dataframe/inc/ROOT/RDF/RLazyDSImpl.hxx index 3fe651520abd4..e30d6ce016711 100644 --- a/tree/dataframe/inc/ROOT/RDF/RLazyDSImpl.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RLazyDSImpl.hxx @@ -28,7 +28,7 @@ namespace ROOT::Internal::RDF { class R__CLING_PTRCHECK(off) RLazyDSColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { ROOT::Internal::RDF::TPointerHolder *fPtr; void *GetImpl(std::size_t) final { return fPtr->GetPointer(); } - void LoadImpl(Long64_t, bool) final {} + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) final {} public: RLazyDSColumnReader(ROOT::Internal::RDF::TPointerHolder *ptr) : fPtr(ptr) {} diff --git a/tree/dataframe/inc/ROOT/RDF/RLoopManager.hxx b/tree/dataframe/inc/ROOT/RDF/RLoopManager.hxx index 47e5e747ed76f..f3328355a78d7 100644 --- a/tree/dataframe/inc/ROOT/RDF/RLoopManager.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RLoopManager.hxx @@ -256,7 +256,7 @@ public: void Deregister(RDefineBase *definePtr); void Register(RDFInternal::RVariationBase *varPtr); void Deregister(RDFInternal::RVariationBase *varPtr); - bool CheckFilters(unsigned int, Long64_t) final; + ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int, Long64_t) final; unsigned int GetNSlots() const { return fNSlots; } void Report(ROOT::RDF::RCutFlowReport &rep) const final; /// End of recursive chain of calls, does nothing diff --git a/tree/dataframe/inc/ROOT/RDF/RMaskedEntryRange.hxx b/tree/dataframe/inc/ROOT/RDF/RMaskedEntryRange.hxx new file mode 100644 index 0000000000000..55da9b3caea18 --- /dev/null +++ b/tree/dataframe/inc/ROOT/RDF/RMaskedEntryRange.hxx @@ -0,0 +1,38 @@ +// Author: Enrico Guiraud, Vincenzo Eduardo Padulano + +/************************************************************************* + * Copyright (C) 1995-2026, Rene Brun and Fons Rademakers. * + * All rights reserved. * + * * + * For the licensing terms see $ROOTSYS/LICENSE. * + * For the list of contributors see $ROOTSYS/README/CREDITS. * + *************************************************************************/ + +#ifndef ROOT_RDF_RMASKEDENTRYRANGE +#define ROOT_RDF_RMASKEDENTRYRANGE + +#include + +#include +#include + +namespace ROOT::Internal::RDF { + +/// RDataFrame's internal representation of an entry range with a boolean mask. +/// The mask has static size but depending on the dynamic bulk size fewer elements could be in use: +/// do not take the size of the mask as the size of the bulk. +class RMaskedEntryRange { + ROOT::RVec fMask{}; ///< Boolean mask. Its size is set at construction time. + std::uint64_t fBegin{}; ///< Entry number of the first entry in the range this mask corresponds to. + +public: + RMaskedEntryRange(std::size_t size, bool set = true, std::uint64_t entry = 0) : fMask(size, set), fBegin(entry) {} + RMaskedEntryRange(const ROOT::RVec &mask, std::uint64_t begin) : fMask(mask), fBegin(begin) {} + const bool &operator[](std::size_t idx) const { return fMask.at(idx); } + bool &operator[](std::size_t idx) { return fMask.at(idx); } + std::uint64_t GetFirstEntry() const { return fBegin; } + void SetFirstEntry(std::uint64_t e) { fBegin = e; } +}; + +} // namespace ROOT::Internal::RDF +#endif diff --git a/tree/dataframe/inc/ROOT/RDF/RNodeBase.hxx b/tree/dataframe/inc/ROOT/RDF/RNodeBase.hxx index ce38725449333..ff2c429bf0025 100644 --- a/tree/dataframe/inc/ROOT/RDF/RNodeBase.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RNodeBase.hxx @@ -13,6 +13,8 @@ #include "RtypesCore.h" #include "TError.h" // R__ASSERT +#include +#include #include #include @@ -46,10 +48,13 @@ protected: unsigned int fNChildren{0}; ///< Number of nodes of the functional graph hanging from this object unsigned int fNStopsReceived{0}; ///< Number of times that a children node signaled to stop processing entries. std::vector fVariations; ///< List of systematic variations that affect this node. + std::vector fLastCheckedEntry; public: - RNodeBase(const std::vector &variations = {}, RLoopManager *lm = nullptr) - : fLoopManager(lm), fVariations(variations) + RNodeBase(const std::vector &variations = {}, RLoopManager *lm = nullptr, unsigned int nSlots = 1) + : fLoopManager(lm), + fVariations(variations), + fLastCheckedEntry(nSlots * ROOT::Internal::RDF::CacheLineStep(), -1) { } @@ -60,7 +65,7 @@ public: RNodeBase &operator=(RNodeBase &&) = delete; virtual ~RNodeBase() = default; - virtual bool CheckFilters(unsigned int, Long64_t) = 0; + virtual ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int, Long64_t) = 0; virtual void Report(ROOT::RDF::RCutFlowReport &) const = 0; virtual void PartialReport(ROOT::RDF::RCutFlowReport &) const = 0; virtual void IncrChildrenCount() = 0; diff --git a/tree/dataframe/inc/ROOT/RDF/RRange.hxx b/tree/dataframe/inc/ROOT/RDF/RRange.hxx index ea2133ec3d879..198b69b514e2c 100644 --- a/tree/dataframe/inc/ROOT/RDF/RRange.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RRange.hxx @@ -68,30 +68,36 @@ public: ~RRange() final { fLoopManager->Deregister(this); } /// Ranges act as filters when it comes to selecting entries that downstream nodes should process - bool CheckFilters(unsigned int slot, Long64_t entry) final + ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int slot, Long64_t entry) final { - if (entry != fLastCheckedEntry) { - if (fHasStopped) - return false; - if (!fPrevNode.CheckFilters(slot, entry)) { - // a filter upstream returned false, cache the result - fLastResult = false; - } else { - // apply range filter logic, cache the result - if (fNProcessedEntries < fStart || (fStop > 0 && fNProcessedEntries >= fStop) || - (fStride != 1 && (fNProcessedEntries - fStart) % fStride != 0)) - fLastResult = false; - else - fLastResult = true; + if (entry == fLastCheckedEntry[slot * ROOT::Internal::RDF::CacheLineStep()]) + return {fCachedResults[slot * RDFInternal::CacheLineStep>()], + static_cast(entry)}; + + if (fHasStopped) + return {1ul, false, static_cast(entry)}; + + // Assume 1-size bulk for now + fLastCheckedEntry[slot * ROOT::Internal::RDF::CacheLineStep()] = entry; + auto mask = fPrevNode.CheckFilters(slot, entry); + const std::size_t bulkSize = 1; + fCachedResults[slot * RDFInternal::CacheLineStep>()].clear(); + fCachedResults[slot * RDFInternal::CacheLineStep>()].resize(bulkSize); + for (std::size_t i = 0; i < bulkSize; ++i) { + if (mask[i]) { + fCachedResults[slot * RDFInternal::CacheLineStep>()][i] = + fNProcessedEntries >= fStart && (fStop == 0 || fNProcessedEntries < fStop) && + (fStride == 1 || (fNProcessedEntries - fStart) % fStride == 0); ++fNProcessedEntries; - if (fNProcessedEntries == fStop) { - fHasStopped = true; - fPrevNode.StopProcessing(); - } } - fLastCheckedEntry = entry; } - return fLastResult; + + if (fStop > 0 && fNProcessedEntries >= fStop) { + fHasStopped = true; + fPrevNode.StopProcessing(); + } + + return {fCachedResults[slot * RDFInternal::CacheLineStep>()], static_cast(entry)}; } // recursive chain of `Report`s diff --git a/tree/dataframe/inc/ROOT/RDF/RRangeBase.hxx b/tree/dataframe/inc/ROOT/RDF/RRangeBase.hxx index c5fbed6dc5d83..d50d32c69724f 100644 --- a/tree/dataframe/inc/ROOT/RDF/RRangeBase.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RRangeBase.hxx @@ -35,13 +35,13 @@ protected: unsigned int fStart; unsigned int fStop; unsigned int fStride; - Long64_t fLastCheckedEntry{-1}; - bool fLastResult{true}; ULong64_t fNProcessedEntries{0}; bool fHasStopped{false}; ///< True if the end of the range has been reached const unsigned int fNSlots; ///< Number of thread slots used by this node, inherited from parent node. std::unordered_map> fVariedRanges; + std::vector> fCachedResults; + public: RRangeBase(RLoopManager *implPtr, unsigned int start, unsigned int stop, unsigned int stride, const unsigned int nSlots, const std::vector &prevVariations); diff --git a/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx b/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx index 57ee799b1c83b..b750a74f89bd3 100644 --- a/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx @@ -37,9 +37,10 @@ namespace RDF { class R__CLING_PTRCHECK(off) RTreeOpaqueColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { std::unique_ptr fTreeValue; + void *fValuePtr{nullptr}; void *GetImpl(std::size_t) override; - void LoadImpl(Long64_t, bool) override; + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) override; public: /// Construct the RTreeColumnReader. Actual initialization is performed lazily by the Init method. @@ -57,9 +58,10 @@ public: /// RTreeColumnReader specialization for TTree values read via TTreeReaderUntypedValue class R__CLING_PTRCHECK(off) RTreeUntypedValueColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { std::unique_ptr fTreeValue; + void *fValuePtr{nullptr}; void *GetImpl(std::size_t) override; - void LoadImpl(Long64_t, bool) override; + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) override; public: RTreeUntypedValueColumnReader(TTreeReader &r, std::string_view colName, std::string_view typeName); @@ -116,7 +118,7 @@ private: void *fValuePtr{nullptr}; void *GetImpl(std::size_t) override; - void LoadImpl(Long64_t, bool) override; + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) override; void *LoadStdArray(Long64_t entry); void *LoadStdVector(Long64_t entry); @@ -130,7 +132,7 @@ class R__CLING_PTRCHECK(off) RMaskedColumnReader : public ROOT::Detail::RDF::RCo void *fValuePtr{nullptr}; void *GetImpl(std::size_t) override; - void LoadImpl(Long64_t, bool) override; + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) override; public: RMaskedColumnReader(TTreeReader &r, std::unique_ptr valueReader, diff --git a/tree/dataframe/inc/ROOT/RDF/RVariation.hxx b/tree/dataframe/inc/ROOT/RDF/RVariation.hxx index ac7091f12aa7c..3b66497a69f1d 100644 --- a/tree/dataframe/inc/ROOT/RDF/RVariation.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RVariation.hxx @@ -230,14 +230,18 @@ public: } /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry - void Update(unsigned int slot, Long64_t entry, bool mask) final + void Update(unsigned int slot, const ROOT::Internal::RDF::RMaskedEntryRange &mask) final { - if (entry != fLastCheckedEntry[slot * CacheLineStep()]) { - // evaluate this filter, cache the result - std::for_each(fValues[slot].begin(), fValues[slot].end(), [entry, mask](auto *v) { v->Load(entry, mask); }); - if (mask) { - UpdateHelper(slot, /*idx=*/0u, ColumnTypes_t{}, TypeInd_t{}); - fLastCheckedEntry[slot * CacheLineStep()] = entry; + if (static_cast(mask.GetFirstEntry()) == fLastCheckedEntry[slot * CacheLineStep()]) + return; + + std::for_each(fValues[slot].begin(), fValues[slot].end(), [&mask](auto *v) { v->Load(mask); }); + // Assume 1-size bulk for now + const std::size_t bulkSize = 1; + for (std::size_t i = 0; i < bulkSize; ++i) { + if (mask[i]) { + UpdateHelper(slot, i, ColumnTypes_t{}, TypeInd_t{}); + fLastCheckedEntry[slot * CacheLineStep()] = mask.GetFirstEntry(); } } } diff --git a/tree/dataframe/inc/ROOT/RDF/RVariationBase.hxx b/tree/dataframe/inc/ROOT/RDF/RVariationBase.hxx index 46a9e4ce29e64..e44e971680ed1 100644 --- a/tree/dataframe/inc/ROOT/RDF/RVariationBase.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RVariationBase.hxx @@ -14,6 +14,7 @@ #include #include // ColumnNames_t #include +#include #include #include @@ -70,7 +71,7 @@ public: const std::string &GetVariationName() const; std::string GetTypeName() const; /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry - virtual void Update(unsigned int slot, Long64_t entry, bool mask) = 0; + virtual void Update(unsigned int slot, const ROOT::Internal::RDF::RMaskedEntryRange &mask) = 0; /// Clean-up operations to be performed at the end of a task. virtual void FinalizeSlot(unsigned int slot) = 0; }; diff --git a/tree/dataframe/inc/ROOT/RDF/RVariationReader.hxx b/tree/dataframe/inc/ROOT/RDF/RVariationReader.hxx index a9ca2b9fcfdaf..39661a5a98af8 100644 --- a/tree/dataframe/inc/ROOT/RDF/RVariationReader.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RVariationReader.hxx @@ -34,7 +34,7 @@ class R__CLING_PTRCHECK(off) RVariationReader final : public ROOT::Detail::RDF:: void *GetImpl(std::size_t /*idx*/) final { return fValuePtr; } - void LoadImpl(Long64_t entry, bool mask) final { fVariation->Update(fSlot, entry, mask); } + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &mask) final { fVariation->Update(fSlot, mask); } public: RVariationReader(unsigned int slot, const std::string &colName, const std::string &variationName, diff --git a/tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx b/tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx index 434f2558c78d2..432f9661552ed 100644 --- a/tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx @@ -166,9 +166,10 @@ public: for (auto varIdx = 0u; varIdx < GetVariations().size(); ++varIdx) { const auto mask = fPrevNodes[varIdx]->CheckFilters(slot, entry); std::for_each(fInputValues[slot][varIdx].begin(), fInputValues[slot][varIdx].end(), - [entry, mask](auto *v) { v->Load(entry, mask); }); + [&mask](auto *v) { v->Load(mask); }); - if (mask) + // Assume 1-size bulk for now + if (mask[0]) CallExec(slot, varIdx, /*idx=*/0u, ColumnTypes_t{}, TypeInd_t{}); } } diff --git a/tree/dataframe/inc/ROOT/RDF/SnapshotHelpers.hxx b/tree/dataframe/inc/ROOT/RDF/SnapshotHelpers.hxx index 698d58f697a85..fdaa4d3512b16 100644 --- a/tree/dataframe/inc/ROOT/RDF/SnapshotHelpers.hxx +++ b/tree/dataframe/inc/ROOT/RDF/SnapshotHelpers.hxx @@ -299,7 +299,8 @@ public: void InitTask(TTreeReader *, unsigned int slot); - void Exec(unsigned int /*slot*/, const std::vector &values, std::vector const &filterPassed); + void Exec(unsigned int /*slot*/, const std::vector &values, + std::vector const &filterPassed); /// Nothing to do. All initialisations run in the constructor or InitTask(). void Initialize() {} diff --git a/tree/dataframe/inc/ROOT/RSqliteDS.hxx b/tree/dataframe/inc/ROOT/RSqliteDS.hxx index a7a903eee15d0..0452e43bef097 100644 --- a/tree/dataframe/inc/ROOT/RSqliteDS.hxx +++ b/tree/dataframe/inc/ROOT/RSqliteDS.hxx @@ -23,7 +23,7 @@ namespace ROOT::Internal::RDF { class R__CLING_PTRCHECK(off) RSqliteDSColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { void *fValuePtr; void *GetImpl(std::size_t) final { return fValuePtr; } - void LoadImpl(Long64_t, bool) final {} + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) final {} public: RSqliteDSColumnReader(void *valuePtr) : fValuePtr(valuePtr) {} diff --git a/tree/dataframe/inc/ROOT/RTrivialDS.hxx b/tree/dataframe/inc/ROOT/RTrivialDS.hxx index 97f40281b0ead..340c738f1aed0 100644 --- a/tree/dataframe/inc/ROOT/RTrivialDS.hxx +++ b/tree/dataframe/inc/ROOT/RTrivialDS.hxx @@ -18,7 +18,7 @@ namespace ROOT::Internal::RDF { class R__CLING_PTRCHECK(off) RTrivialDSColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { ULong64_t *fValuePtr; void *GetImpl(std::size_t) final { return fValuePtr; } - void LoadImpl(Long64_t, bool) final {} + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) final {} public: RTrivialDSColumnReader(ULong64_t *valuePtr) : fValuePtr(valuePtr) {} diff --git a/tree/dataframe/inc/ROOT/RVecDS.hxx b/tree/dataframe/inc/ROOT/RVecDS.hxx index d3d27ba46a67c..7bda21c71ad95 100644 --- a/tree/dataframe/inc/ROOT/RVecDS.hxx +++ b/tree/dataframe/inc/ROOT/RVecDS.hxx @@ -31,7 +31,7 @@ namespace ROOT::Internal::RDF { class R__CLING_PTRCHECK(off) RVecDSColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { TPointerHolder *fPtrHolder; void *GetImpl(std::size_t) final { return fPtrHolder->GetPointer(); } - void LoadImpl(Long64_t, bool) final {} + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) final {} public: RVecDSColumnReader(TPointerHolder *ptrHolder) : fPtrHolder(ptrHolder) {} diff --git a/tree/dataframe/src/RDFSnapshotHelpers.cxx b/tree/dataframe/src/RDFSnapshotHelpers.cxx index 95d0b6c0c230a..8f8b114384ac0 100644 --- a/tree/dataframe/src/RDFSnapshotHelpers.cxx +++ b/tree/dataframe/src/RDFSnapshotHelpers.cxx @@ -1225,9 +1225,11 @@ void ROOT::Internal::RDF::SnapshotHelperWithVariations::InitTask(TTreeReader *, /// Connect all output fields to the values pointed to by `values`, fill the output dataset, /// call the Fill of the output tree, and clear the mask bits that show whether a variation was reached. -void ROOT::Internal::RDF::SnapshotHelperWithVariations::Exec(unsigned int /*slot*/, const std::vector &values, - std::vector const &filterPassed) +void ROOT::Internal::RDF::SnapshotHelperWithVariations::Exec( + unsigned int /*slot*/, const std::vector &values, + std::vector const &filterPassed) { + // Assume 1-size bulk for now // Rebind branch pointers to RDF values assert(fBranchData.size() == values.size()); for (std::size_t i = 0; i < values.size(); i++) { @@ -1237,14 +1239,14 @@ void ROOT::Internal::RDF::SnapshotHelperWithVariations::Exec(unsigned int /*slot SetBranchesHelper(fInputTree, *fOutputHandle->fTree, fBranchData, i, fOptions.fBasketSize, values[i]); } else { // Nominal will always be written, systematics only if needed - if (variationIndex == 0 || filterPassed[variationIndex]) { + if (variationIndex == 0 || filterPassed[variationIndex][0]) { const bool fundamentalType = fBranchData[i].WriteValueIfFundamental(values[i]); if (!fundamentalType) { SetBranchesHelper(fInputTree, *fOutputHandle->fTree, fBranchData, i, fOptions.fBasketSize, values[i]); } } - if (filterPassed[variationIndex]) { + if (filterPassed[variationIndex][0]) { fOutputHandle->SetMaskBit(variationIndex); } } diff --git a/tree/dataframe/src/RFilterBase.cxx b/tree/dataframe/src/RFilterBase.cxx index bf42e8199925d..248590cbfc462 100644 --- a/tree/dataframe/src/RFilterBase.cxx +++ b/tree/dataframe/src/RFilterBase.cxx @@ -19,12 +19,15 @@ using namespace ROOT::Detail::RDF; RFilterBase::RFilterBase(RLoopManager *implPtr, std::string_view name, const unsigned int nSlots, const RDFInternal::RColumnRegister &colRegister, const ColumnNames_t &columns, const std::vector &prevVariations, const std::string &variation) - : RNodeBase(ROOT::Internal::RDF::Union(colRegister.GetVariationDeps(columns), prevVariations), implPtr), - fLastCheckedEntry(nSlots * RDFInternal::CacheLineStep(), -1), - fLastResult(nSlots * RDFInternal::CacheLineStep()), + : RNodeBase(ROOT::Internal::RDF::Union(colRegister.GetVariationDeps(columns), prevVariations), implPtr, nSlots), fAccepted(nSlots * RDFInternal::CacheLineStep()), - fRejected(nSlots * RDFInternal::CacheLineStep()), fName(name), fColumnNames(columns), - fColRegister(colRegister), fIsDefine(columns.size()), fVariation(variation) + fRejected(nSlots * RDFInternal::CacheLineStep()), + fName(name), + fColumnNames(columns), + fColRegister(colRegister), + fIsDefine(columns.size()), + fVariation(variation), + fCachedResults(nSlots * RDFInternal::CacheLineStep>()) { const auto nColumns = fColumnNames.size(); for (auto i = 0u; i < nColumns; ++i) { diff --git a/tree/dataframe/src/RJittedDefine.cxx b/tree/dataframe/src/RJittedDefine.cxx index b094c3080b30f..e10dd0b47f852 100644 --- a/tree/dataframe/src/RJittedDefine.cxx +++ b/tree/dataframe/src/RJittedDefine.cxx @@ -39,10 +39,10 @@ const std::type_info &RJittedDefine::GetTypeId() const "retrieved. This should never happen, please report this as a bug."); } -void RJittedDefine::Update(unsigned int slot, Long64_t entry, bool mask) +void RJittedDefine::Update(unsigned int slot, const ROOT::Internal::RDF::RMaskedEntryRange &mask) { assert(fConcreteDefine != nullptr); - fConcreteDefine->Update(slot, entry, mask); + fConcreteDefine->Update(slot, mask); } void RJittedDefine::Update(unsigned int slot, const ROOT::RDF::RSampleInfo &id) diff --git a/tree/dataframe/src/RJittedFilter.cxx b/tree/dataframe/src/RJittedFilter.cxx index 9e94e74e1c414..c4d6cb6270071 100644 --- a/tree/dataframe/src/RJittedFilter.cxx +++ b/tree/dataframe/src/RJittedFilter.cxx @@ -55,7 +55,7 @@ void RJittedFilter::InitSlot(TTreeReader *r, unsigned int slot) fConcreteFilter->InitSlot(r, slot); } -bool RJittedFilter::CheckFilters(unsigned int slot, Long64_t entry) +ROOT::Internal::RDF::RMaskedEntryRange RJittedFilter::CheckFilters(unsigned int slot, Long64_t entry) { assert(fConcreteFilter != nullptr); return fConcreteFilter->CheckFilters(slot, entry); diff --git a/tree/dataframe/src/RJittedVariation.cxx b/tree/dataframe/src/RJittedVariation.cxx index 663a87fdfddaf..eb0257f474ccd 100644 --- a/tree/dataframe/src/RJittedVariation.cxx +++ b/tree/dataframe/src/RJittedVariation.cxx @@ -34,10 +34,10 @@ const std::type_info &RJittedVariation::GetTypeId() const return fConcreteVariation->GetTypeId(); } -void RJittedVariation::Update(unsigned int slot, Long64_t entry, bool mask) +void RJittedVariation::Update(unsigned int slot, const ROOT::Internal::RDF::RMaskedEntryRange &mask) { assert(fConcreteVariation != nullptr); - fConcreteVariation->Update(slot, entry, mask); + fConcreteVariation->Update(slot, mask); } void RJittedVariation::FinalizeSlot(unsigned int slot) diff --git a/tree/dataframe/src/RLoopManager.cxx b/tree/dataframe/src/RLoopManager.cxx index d750b4dfa8ddf..a587ac44982aa 100644 --- a/tree/dataframe/src/RLoopManager.cxx +++ b/tree/dataframe/src/RLoopManager.cxx @@ -1027,9 +1027,10 @@ void RLoopManager::Deregister(RDFInternal::RVariationBase *v) } // dummy call, end of recursive chain of calls -bool RLoopManager::CheckFilters(unsigned int, Long64_t) +ROOT::Internal::RDF::RMaskedEntryRange RLoopManager::CheckFilters(unsigned int, Long64_t entry) { - return true; + // Assume 1-size bulk for now + return ROOT::Internal::RDF::RMaskedEntryRange{1ul, true, static_cast(entry)}; } /// Call `FillReport` on all booked filters diff --git a/tree/dataframe/src/RNTupleDS.cxx b/tree/dataframe/src/RNTupleDS.cxx index b92b3363a6d5a..5787343c858da 100644 --- a/tree/dataframe/src/RNTupleDS.cxx +++ b/tree/dataframe/src/RNTupleDS.cxx @@ -71,7 +71,6 @@ class RNTupleColumnReader : public ROOT::Detail::RDF::RColumnReaderBase { std::unique_ptr fField; ///< The field backing the RDF column std::unique_ptr fValue; ///< The memory location used to read from fField std::shared_ptr fValuePtr; ///< Used to reuse the object created by fValue when reconnecting sources - Long64_t fLastEntry = -1; ///< Last entry number that was read /// For chains, the logical entry and the physical entry in any particular file can be different. /// The entry offset stores the logical entry number (sum of all previous physical entries) when a file of the /// corresponding data source was opened. @@ -84,7 +83,6 @@ class RNTupleColumnReader : public ROOT::Detail::RDF::RColumnReaderBase { /// Connect the field and its subfields to the page source void Connect(RPageSource &source, Long64_t entryOffset) { - assert(fLastEntry == -1); fEntryOffset = entryOffset; @@ -134,16 +132,15 @@ class RNTupleColumnReader : public ROOT::Detail::RDF::RColumnReaderBase { } fValue = nullptr; fField = nullptr; - fLastEntry = -1; } void *GetImpl(std::size_t /*idx*/) final { return fValue->GetPtr().get(); } - void LoadImpl(Long64_t entry, bool mask) final + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &mask) final { - if (entry != fLastEntry && mask) { - fValue->Read(entry - fEntryOffset); - fLastEntry = entry; + // Assume 1-size bulk for now + if (mask[0]) { + fValue->Read(mask.GetFirstEntry() - fEntryOffset); } } }; diff --git a/tree/dataframe/src/RRangeBase.cxx b/tree/dataframe/src/RRangeBase.cxx index 6d7f0fe01d3f8..8b0f91c0fc0e9 100644 --- a/tree/dataframe/src/RRangeBase.cxx +++ b/tree/dataframe/src/RRangeBase.cxx @@ -14,13 +14,17 @@ using ROOT::Detail::RDF::RRangeBase; RRangeBase::RRangeBase(RLoopManager *implPtr, unsigned int start, unsigned int stop, unsigned int stride, const unsigned int nSlots, const std::vector &prevVariations) - : RNodeBase(prevVariations, implPtr), fStart(start), fStop(stop), fStride(stride), fNSlots(nSlots) + : RNodeBase(prevVariations, implPtr, nSlots), + fStart(start), + fStop(stop), + fStride(stride), + fNSlots(nSlots), + fCachedResults(nSlots * ROOT::Internal::RDF::CacheLineStep>()) { } void RRangeBase::InitNode() { - fLastCheckedEntry = -1; fNProcessedEntries = 0; fHasStopped = false; } diff --git a/tree/dataframe/src/RTreeColumnReader.cxx b/tree/dataframe/src/RTreeColumnReader.cxx index 37182a7c80cb9..6a6c35c2965c0 100644 --- a/tree/dataframe/src/RTreeColumnReader.cxx +++ b/tree/dataframe/src/RTreeColumnReader.cxx @@ -7,10 +7,15 @@ void *ROOT::Internal::RDF::RTreeOpaqueColumnReader::GetImpl(std::size_t) { - return fTreeValue->GetAddress(); + return fValuePtr; } -void ROOT::Internal::RDF::RTreeOpaqueColumnReader::LoadImpl(Long64_t, bool) {} +void ROOT::Internal::RDF::RTreeOpaqueColumnReader::LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &mask) +{ + // Assume size-1 bulk for now + if (mask[0]) + fValuePtr = fTreeValue->GetAddress(); +} ROOT::Internal::RDF::RTreeOpaqueColumnReader::RTreeOpaqueColumnReader(TTreeReader &r, std::string_view colName) : fTreeValue(std::make_unique(r, colName.data())) @@ -21,10 +26,15 @@ ROOT::Internal::RDF::RTreeOpaqueColumnReader::~RTreeOpaqueColumnReader() = defau void *ROOT::Internal::RDF::RTreeUntypedValueColumnReader::GetImpl(std::size_t) { - return fTreeValue->Get(); + return fValuePtr; } -void ROOT::Internal::RDF::RTreeUntypedValueColumnReader::LoadImpl(Long64_t, bool) {} +void ROOT::Internal::RDF::RTreeUntypedValueColumnReader::LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &mask) +{ + // Assume size-1 bulk for now + if (mask[0]) + fValuePtr = fTreeValue->Get(); +} ROOT::Internal::RDF::RTreeUntypedValueColumnReader::RTreeUntypedValueColumnReader(TTreeReader &r, std::string_view colName, @@ -164,19 +174,20 @@ void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadRVec(Long64_t entr return &fRVec; } -void ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadImpl(Long64_t entry, bool mask) +void ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &mask) { - if (entry != fLastEntry && mask) { + // Assume size-1 bulk for now + if (mask[0]) { if (fCollectionType == ECollectionType::kStdArray) - fValuePtr = LoadStdArray(entry); + fValuePtr = LoadStdArray(mask.GetFirstEntry()); else if (fCollectionType == ECollectionType::kStdVector) - fValuePtr = LoadStdVector(entry); + fValuePtr = LoadStdVector(mask.GetFirstEntry()); else - fValuePtr = LoadRVec(entry); + fValuePtr = LoadRVec(mask.GetFirstEntry()); } } -void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::GetImpl(std::size_t /*idx*/) +void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::GetImpl(std::size_t) { return fValuePtr; } @@ -208,9 +219,9 @@ void *ROOT::Internal::RDF::RMaskedColumnReader::GetImpl(std::size_t) return fValuePtr; } -void ROOT::Internal::RDF::RMaskedColumnReader::LoadImpl(Long64_t entry, bool mask) +void ROOT::Internal::RDF::RMaskedColumnReader::LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &mask) { - if (!mask) { + if (!mask[0]) { fValuePtr = nullptr; return; } @@ -221,6 +232,6 @@ void ROOT::Internal::RDF::RMaskedColumnReader::LoadImpl(Long64_t entry, bool mas return; } - fValueReader->Load(entry, mask); + fValueReader->Load(mask); fValuePtr = fValueReader->TryGet(/*idx=*/0u); } diff --git a/tree/dataframe/test/RArraysDS.hxx b/tree/dataframe/test/RArraysDS.hxx index c21c09a673509..38b54a06d0402 100644 --- a/tree/dataframe/test/RArraysDS.hxx +++ b/tree/dataframe/test/RArraysDS.hxx @@ -13,7 +13,7 @@ class R__CLING_PTRCHECK(off) RArraysDSVarReader final : public ROOT::Detail::RDF::RColumnReaderBase { std::vector *fPtr = nullptr; void *GetImpl(std::size_t) final { return fPtr; } - void LoadImpl(Long64_t, bool) final {} + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) final {} public: RArraysDSVarReader(std::vector &v) : fPtr(&v) {} @@ -27,7 +27,7 @@ class R__CLING_PTRCHECK(off) RArraysDSVarSizeReader final : public ROOT::Detail: fSize = fPtr->size(); return &fSize; } - void LoadImpl(Long64_t, bool) final {} + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) final {} public: RArraysDSVarSizeReader(std::vector &v) : fPtr(&v) {} From 85c554a18c502175c89b37476fd0555aca17d0de Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Wed, 3 Jun 2026 17:30:13 +0200 Subject: [PATCH 3/8] [df] Centrally propagate bulk size through RLoopManager The RLoopManager decides the current bulk size that all nodes of the computation graph must adhere to. Currently this is set to one, in the future it may vary. --- tree/dataframe/inc/ROOT/RDF/RAction.hxx | 4 +- tree/dataframe/inc/ROOT/RDF/RActionBase.hxx | 3 +- .../inc/ROOT/RDF/RActionSnapshot.hxx | 13 ++--- .../inc/ROOT/RDF/RDefaultValueFor.hxx | 2 +- tree/dataframe/inc/ROOT/RDF/RFilter.hxx | 16 +++--- .../inc/ROOT/RDF/RFilterWithMissingValues.hxx | 15 +++-- tree/dataframe/inc/ROOT/RDF/RJittedAction.hxx | 2 +- tree/dataframe/inc/ROOT/RDF/RJittedFilter.hxx | 2 +- tree/dataframe/inc/ROOT/RDF/RLoopManager.hxx | 9 ++- tree/dataframe/inc/ROOT/RDF/RNodeBase.hxx | 4 +- tree/dataframe/inc/ROOT/RDF/RRange.hxx | 18 +++--- tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx | 4 +- tree/dataframe/src/RJittedAction.cxx | 4 +- tree/dataframe/src/RJittedFilter.cxx | 5 +- tree/dataframe/src/RLoopManager.cxx | 56 ++++++++++++------- 15 files changed, 91 insertions(+), 66 deletions(-) diff --git a/tree/dataframe/inc/ROOT/RDF/RAction.hxx b/tree/dataframe/inc/ROOT/RDF/RAction.hxx index d52968236e671..1568313a967f8 100644 --- a/tree/dataframe/inc/ROOT/RDF/RAction.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RAction.hxx @@ -119,9 +119,9 @@ public: (void)idx; // avoid unused parameter warning (gcc 12.1) } - void Run(unsigned int slot, Long64_t entry) final + void Run(unsigned int slot, Long64_t bulkBeginEntry, std::size_t bulkSize) final { - const auto mask = fPrevNode.CheckFilters(slot, entry); + const auto mask = fPrevNode.CheckFilters(slot, bulkBeginEntry, bulkSize); std::for_each(fValues[slot].begin(), fValues[slot].end(), [&mask](auto *v) { v->Load(mask); }); // Assume 1-size bulk for now diff --git a/tree/dataframe/inc/ROOT/RDF/RActionBase.hxx b/tree/dataframe/inc/ROOT/RDF/RActionBase.hxx index ff765a7ca69cd..a95ba5a8734f6 100644 --- a/tree/dataframe/inc/ROOT/RDF/RActionBase.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RActionBase.hxx @@ -63,7 +63,6 @@ public: RColumnRegister &GetColRegister() { return fColRegister; } RLoopManager *GetLoopManager() { return fLoopManager; } unsigned int GetNSlots() const { return fNSlots; } - virtual void Run(unsigned int slot, Long64_t entry) = 0; virtual void Initialize() = 0; virtual void InitSlot(TTreeReader *r, unsigned int slot) = 0; virtual void TriggerChildrenCount() = 0; @@ -92,6 +91,8 @@ public: virtual std::unique_ptr MakeVariedAction(std::vector &&results) = 0; virtual std::unique_ptr CloneAction(void *newResult) = 0; + + virtual void Run(unsigned int slot, Long64_t bulkBeginEntry, std::size_t bulkSize) = 0; }; } // namespace RDF } // namespace Internal diff --git a/tree/dataframe/inc/ROOT/RDF/RActionSnapshot.hxx b/tree/dataframe/inc/ROOT/RDF/RActionSnapshot.hxx index 919bb9bb62142..1c3e9a7a87e34 100644 --- a/tree/dataframe/inc/ROOT/RDF/RActionSnapshot.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RActionSnapshot.hxx @@ -191,26 +191,26 @@ public: fHelper.Exec(slot, untypedValues); } - void Run(unsigned int slot, Long64_t entry) final + void Run(unsigned int slot, Long64_t bulkBeginEntry, std::size_t bulkSize) final { if constexpr (std::is_same_v) { // check if entry passes all filters std::vector filterPassed(fPrevNodes.size(), 1ul); for (unsigned int variation = 0; variation < fPrevNodes.size(); ++variation) { - filterPassed[variation] = fPrevNodes[variation]->CheckFilters(slot, entry); + filterPassed[variation] = fPrevNodes[variation]->CheckFilters(slot, bulkBeginEntry, bulkSize); } // Currently, every event where any of nominal or variations pass gets written to the output. // This logic could be extended for different use cases if the need arises. - // Assume 1-size bulk for now if (std::any_of(filterPassed.begin(), filterPassed.end(), [](const ROOT::Internal::RDF::RMaskedEntryRange &val) { return val[0]; })) { // TODO: Don't allocate std::vector untypedValues; auto nReaders = fValues[slot].size(); untypedValues.reserve(nReaders); - std::for_each(fValues[slot].begin(), fValues[slot].end(), [entry](auto *v) { - v->Load(ROOT::Internal::RDF::RMaskedEntryRange{1ul, true, static_cast(entry)}); + std::for_each(fValues[slot].begin(), fValues[slot].end(), [bulkBeginEntry, bulkSize](auto *v) { + v->Load( + ROOT::Internal::RDF::RMaskedEntryRange{bulkSize, true, static_cast(bulkBeginEntry)}); }); for (decltype(nReaders) readerIdx{}; readerIdx < nReaders; readerIdx++) untypedValues.push_back(GetValue(slot, readerIdx, /*idx=*/0u)); @@ -218,9 +218,8 @@ public: fHelper.Exec(slot, untypedValues, filterPassed); } } else { - const auto mask = fPrevNodes.front()->CheckFilters(slot, entry); + const auto mask = fPrevNodes.front()->CheckFilters(slot, bulkBeginEntry, bulkSize); std::for_each(fValues[slot].begin(), fValues[slot].end(), [&mask](auto *v) { v->Load(mask); }); - // Assume 1-size bulk for now if (mask[0]) CallExec(slot, /*idx=*/0u); } diff --git a/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx b/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx index a7e5f6a090475..c7cb63af96def 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx @@ -112,7 +112,7 @@ public: // Assume 1-size bulk for now fValues[slot]->Load(mask); - const std::size_t bulkSize = 1; + const std::size_t bulkSize = fLoopManager->GetCurrentBulkSize(); for (std::size_t i = 0; i < bulkSize; ++i) { if (mask[i]) fLastResults[slot * RDFInternal::CacheLineStep()] = GetValueOrDefault(slot, i); diff --git a/tree/dataframe/inc/ROOT/RDF/RFilter.hxx b/tree/dataframe/inc/ROOT/RDF/RFilter.hxx index e381c84ff4992..6f4762b7073b3 100644 --- a/tree/dataframe/inc/ROOT/RDF/RFilter.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RFilter.hxx @@ -93,16 +93,16 @@ public: fLoopManager->Deregister(this); } - ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int slot, Long64_t entry) final + ROOT::Internal::RDF::RMaskedEntryRange + CheckFilters(unsigned int slot, Long64_t bulkBeginEntry, std::size_t bulkSize) final { auto &cachedResults = fCachedResults[slot * RDFInternal::CacheLineStep>()]; - if (entry == fLastCheckedEntry[slot * ROOT::Internal::RDF::CacheLineStep()]) - return {cachedResults, static_cast(entry)}; + if (bulkBeginEntry == fLastCheckedEntry[slot * ROOT::Internal::RDF::CacheLineStep()]) + return {cachedResults, static_cast(bulkBeginEntry)}; - auto mask = fPrevNode.CheckFilters(slot, entry); + auto mask = fPrevNode.CheckFilters(slot, bulkBeginEntry, bulkSize); std::for_each(fValues[slot].begin(), fValues[slot].end(), [&mask](auto *v) { v->Load(mask); }); - // Assume 1-size bulk for now - const std::size_t bulkSize{1}; + std::size_t accepted{0}; std::size_t rejected{0}; cachedResults.clear(); @@ -116,11 +116,11 @@ public: ++rejected; } } - fLastCheckedEntry[slot * ROOT::Internal::RDF::CacheLineStep()] = entry; + fLastCheckedEntry[slot * ROOT::Internal::RDF::CacheLineStep()] = bulkBeginEntry; fAccepted[slot * RDFInternal::CacheLineStep()] += accepted; fRejected[slot * RDFInternal::CacheLineStep()] += rejected; - return {cachedResults, static_cast(entry)}; + return {cachedResults, static_cast(bulkBeginEntry)}; } template diff --git a/tree/dataframe/inc/ROOT/RDF/RFilterWithMissingValues.hxx b/tree/dataframe/inc/ROOT/RDF/RFilterWithMissingValues.hxx index a88bb1821ff48..e3e518e7e5f46 100644 --- a/tree/dataframe/inc/ROOT/RDF/RFilterWithMissingValues.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RFilterWithMissingValues.hxx @@ -96,17 +96,16 @@ public: fLoopManager->EraseSuppressErrorsForMissingBranch(fColumnNames[0]); } - ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int slot, Long64_t entry) final + ROOT::Internal::RDF::RMaskedEntryRange + CheckFilters(unsigned int slot, Long64_t bulkBeginEntry, std::size_t bulkSize) final { constexpr static auto cacheLineStepLong64_t = RDFInternal::CacheLineStep(); constexpr static auto cacheLineStepULong64_t = RDFInternal::CacheLineStep(); - if (entry == fLastCheckedEntry[slot * cacheLineStepLong64_t]) - return {fCachedResults[slot], static_cast(entry)}; + if (bulkBeginEntry == fLastCheckedEntry[slot * cacheLineStepLong64_t]) + return {fCachedResults[slot], static_cast(bulkBeginEntry)}; - // Assume 1-size bulk for now - const std::size_t bulkSize{1}; - auto mask = fPrevNodePtr->CheckFilters(slot, entry); + auto mask = fPrevNodePtr->CheckFilters(slot, bulkBeginEntry, bulkSize); fValues[slot]->Load(mask); @@ -127,9 +126,9 @@ public: } fAccepted[slot * cacheLineStepULong64_t] += accepted; fRejected[slot * cacheLineStepULong64_t] += rejected; - fLastCheckedEntry[slot * cacheLineStepLong64_t] = entry; + fLastCheckedEntry[slot * cacheLineStepLong64_t] = bulkBeginEntry; - return {fCachedResults[slot], static_cast(entry)}; + return {fCachedResults[slot], static_cast(bulkBeginEntry)}; } void InitSlot(TTreeReader *r, unsigned int slot) final diff --git a/tree/dataframe/inc/ROOT/RDF/RJittedAction.hxx b/tree/dataframe/inc/ROOT/RDF/RJittedAction.hxx index 9d31bf25d2591..fbcc1ae9402d7 100644 --- a/tree/dataframe/inc/ROOT/RDF/RJittedAction.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RJittedAction.hxx @@ -50,7 +50,7 @@ public: void SetAction(std::unique_ptr a) { fConcreteAction = std::move(a); } - void Run(unsigned int slot, Long64_t entry) final; + void Run(unsigned int, Long64_t, std::size_t) final; void Initialize() final; void InitSlot(TTreeReader *r, unsigned int slot) final; void TriggerChildrenCount() final; diff --git a/tree/dataframe/inc/ROOT/RDF/RJittedFilter.hxx b/tree/dataframe/inc/ROOT/RDF/RJittedFilter.hxx index 42871a33fe669..33802ba8d5999 100644 --- a/tree/dataframe/inc/ROOT/RDF/RJittedFilter.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RJittedFilter.hxx @@ -55,7 +55,7 @@ public: void SetFilter(std::unique_ptr f); void InitSlot(TTreeReader *r, unsigned int slot) final; - ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int, Long64_t) final; + ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int, Long64_t, std::size_t) final; void Report(ROOT::RDF::RCutFlowReport &) const final; void PartialReport(ROOT::RDF::RCutFlowReport &) const final; void FillReport(ROOT::RDF::RCutFlowReport &) const final; diff --git a/tree/dataframe/inc/ROOT/RDF/RLoopManager.hxx b/tree/dataframe/inc/ROOT/RDF/RLoopManager.hxx index f3328355a78d7..2b0bd15f441fd 100644 --- a/tree/dataframe/inc/ROOT/RDF/RLoopManager.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RLoopManager.hxx @@ -181,7 +181,7 @@ class RLoopManager : public RNodeBase { void RunEmptySource(); void RunDataSourceMT(); void RunDataSource(); - void RunAndCheckFilters(unsigned int slot, Long64_t entry); + void RunAndCheckFilters(unsigned int slot, Long64_t bulkBeginEntry, std::size_t bulkSize); void InitNodeSlots(TTreeReader *r, unsigned int slot); void InitNodes(); void CleanUpNodes(); @@ -224,6 +224,9 @@ class RLoopManager : public RNodeBase { std::vector fJitHelperCalls{}; std::hash fStringHasher{}; + // Assume 1-size bulk for now + std::size_t fCurrentBulkSize{1}; + public: RLoopManager(const ColumnNames_t &defaultColumns = {}); RLoopManager(TTree *tree, const ColumnNames_t &defaultBranches); @@ -256,7 +259,7 @@ public: void Deregister(RDefineBase *definePtr); void Register(RDFInternal::RVariationBase *varPtr); void Deregister(RDFInternal::RVariationBase *varPtr); - ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int, Long64_t) final; + ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int, Long64_t, std::size_t) final; unsigned int GetNSlots() const { return fNSlots; } void Report(ROOT::RDF::RCutFlowReport &rep) const final; /// End of recursive chain of calls, does nothing @@ -343,6 +346,8 @@ public: /// The task run by every thread on the input entry range, for the RNTuple data source. void RNTupleThreadTask(const std::pair &entryRange, unsigned int slot, std::uint64_t columnReaderOffset); + + std::size_t GetCurrentBulkSize() { return fCurrentBulkSize; } }; /// \brief Create an RLoopManager that reads a TChain. diff --git a/tree/dataframe/inc/ROOT/RDF/RNodeBase.hxx b/tree/dataframe/inc/ROOT/RDF/RNodeBase.hxx index ff2c429bf0025..7984e3fb6596a 100644 --- a/tree/dataframe/inc/ROOT/RDF/RNodeBase.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RNodeBase.hxx @@ -65,7 +65,6 @@ public: RNodeBase &operator=(RNodeBase &&) = delete; virtual ~RNodeBase() = default; - virtual ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int, Long64_t) = 0; virtual void Report(ROOT::RDF::RCutFlowReport &) const = 0; virtual void PartialReport(ROOT::RDF::RCutFlowReport &) const = 0; virtual void IncrChildrenCount() = 0; @@ -92,6 +91,9 @@ public: "GetVariedFilter was called on a node type that does not implement it. This should never happen."); return nullptr; } + + virtual ROOT::Internal::RDF::RMaskedEntryRange + CheckFilters(unsigned int slot, Long64_t bulkBeginEntry, std::size_t bulkSize) = 0; }; } // ns RDF } // ns Detail diff --git a/tree/dataframe/inc/ROOT/RDF/RRange.hxx b/tree/dataframe/inc/ROOT/RDF/RRange.hxx index 198b69b514e2c..142604ba06433 100644 --- a/tree/dataframe/inc/ROOT/RDF/RRange.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RRange.hxx @@ -68,19 +68,18 @@ public: ~RRange() final { fLoopManager->Deregister(this); } /// Ranges act as filters when it comes to selecting entries that downstream nodes should process - ROOT::Internal::RDF::RMaskedEntryRange CheckFilters(unsigned int slot, Long64_t entry) final + ROOT::Internal::RDF::RMaskedEntryRange + CheckFilters(unsigned int slot, Long64_t bulkBeginEntry, std::size_t bulkSize) final { - if (entry == fLastCheckedEntry[slot * ROOT::Internal::RDF::CacheLineStep()]) + if (bulkBeginEntry == fLastCheckedEntry[slot * ROOT::Internal::RDF::CacheLineStep()]) return {fCachedResults[slot * RDFInternal::CacheLineStep>()], - static_cast(entry)}; + static_cast(bulkBeginEntry)}; if (fHasStopped) - return {1ul, false, static_cast(entry)}; + return {bulkSize, false, static_cast(bulkBeginEntry)}; - // Assume 1-size bulk for now - fLastCheckedEntry[slot * ROOT::Internal::RDF::CacheLineStep()] = entry; - auto mask = fPrevNode.CheckFilters(slot, entry); - const std::size_t bulkSize = 1; + fLastCheckedEntry[slot * ROOT::Internal::RDF::CacheLineStep()] = bulkBeginEntry; + auto mask = fPrevNode.CheckFilters(slot, bulkBeginEntry, bulkSize); fCachedResults[slot * RDFInternal::CacheLineStep>()].clear(); fCachedResults[slot * RDFInternal::CacheLineStep>()].resize(bulkSize); for (std::size_t i = 0; i < bulkSize; ++i) { @@ -97,7 +96,8 @@ public: fPrevNode.StopProcessing(); } - return {fCachedResults[slot * RDFInternal::CacheLineStep>()], static_cast(entry)}; + return {fCachedResults[slot * RDFInternal::CacheLineStep>()], + static_cast(bulkBeginEntry)}; } // recursive chain of `Report`s diff --git a/tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx b/tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx index 432f9661552ed..899bd2f10f4e1 100644 --- a/tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RVariedAction.hxx @@ -161,10 +161,10 @@ public: (void)idx; } - void Run(unsigned int slot, Long64_t entry) final + void Run(unsigned int slot, Long64_t bulkBeginEntry, std::size_t bulkSize) final { for (auto varIdx = 0u; varIdx < GetVariations().size(); ++varIdx) { - const auto mask = fPrevNodes[varIdx]->CheckFilters(slot, entry); + const auto mask = fPrevNodes[varIdx]->CheckFilters(slot, bulkBeginEntry, bulkSize); std::for_each(fInputValues[slot][varIdx].begin(), fInputValues[slot][varIdx].end(), [&mask](auto *v) { v->Load(mask); }); diff --git a/tree/dataframe/src/RJittedAction.cxx b/tree/dataframe/src/RJittedAction.cxx index 65b6f392b6360..2d95881b50dc9 100644 --- a/tree/dataframe/src/RJittedAction.cxx +++ b/tree/dataframe/src/RJittedAction.cxx @@ -68,10 +68,10 @@ RJittedAction::RJittedAction(RLoopManager &lm, const ROOT::RDF::ColumnNames_t &c RJittedAction::~RJittedAction() {} -void RJittedAction::Run(unsigned int slot, Long64_t entry) +void RJittedAction::Run(unsigned int slot, Long64_t bulkBeginEntry, std::size_t bulkSize) { assert(fConcreteAction != nullptr); - fConcreteAction->Run(slot, entry); + fConcreteAction->Run(slot, bulkBeginEntry, bulkSize); } void RJittedAction::Initialize() diff --git a/tree/dataframe/src/RJittedFilter.cxx b/tree/dataframe/src/RJittedFilter.cxx index c4d6cb6270071..25c33a26efd90 100644 --- a/tree/dataframe/src/RJittedFilter.cxx +++ b/tree/dataframe/src/RJittedFilter.cxx @@ -55,10 +55,11 @@ void RJittedFilter::InitSlot(TTreeReader *r, unsigned int slot) fConcreteFilter->InitSlot(r, slot); } -ROOT::Internal::RDF::RMaskedEntryRange RJittedFilter::CheckFilters(unsigned int slot, Long64_t entry) +ROOT::Internal::RDF::RMaskedEntryRange +RJittedFilter::CheckFilters(unsigned int slot, Long64_t bulkBeginEntry, std::size_t bulkSize) { assert(fConcreteFilter != nullptr); - return fConcreteFilter->CheckFilters(slot, entry); + return fConcreteFilter->CheckFilters(slot, bulkBeginEntry, bulkSize); } void RJittedFilter::Report(ROOT::RDF::RCutFlowReport &cr) const diff --git a/tree/dataframe/src/RLoopManager.cxx b/tree/dataframe/src/RLoopManager.cxx index a587ac44982aa..8d827dbf0f6d1 100644 --- a/tree/dataframe/src/RLoopManager.cxx +++ b/tree/dataframe/src/RLoopManager.cxx @@ -501,8 +501,12 @@ void RLoopManager::RunEmptySourceMT() R__LOG_DEBUG(0, RDFLogChannel()) << LogRangeProcessing({"an empty source", range.first, range.second, slot}); try { UpdateSampleInfo(slot, range); - for (auto currEntry = range.first; currEntry < range.second; ++currEntry) { - RunAndCheckFilters(slot, currEntry); + auto bulkBeginEntry = range.first; + std::size_t bulkSize = std::min(GetCurrentBulkSize(), static_cast(range.second - bulkBeginEntry)); + while (bulkBeginEntry < range.second) { + RunAndCheckFilters(slot, bulkBeginEntry, bulkSize); + bulkBeginEntry += bulkSize; + bulkSize = std::min(GetCurrentBulkSize(), static_cast(range.second - bulkBeginEntry)); } } catch (...) { // Error might throw in experiment frameworks like CMSSW @@ -526,9 +530,13 @@ void RLoopManager::RunEmptySource() RCallCleanUpTask cleanup(*this); try { UpdateSampleInfo(/*slot*/ 0, fEmptyEntryRange); - for (ULong64_t currEntry = fEmptyEntryRange.first; - currEntry < fEmptyEntryRange.second && fNStopsReceived < fNChildren; ++currEntry) { - RunAndCheckFilters(0, currEntry); + auto bulkBeginEntry = fEmptyEntryRange.first; + std::size_t bulkSize = + std::min(GetCurrentBulkSize(), static_cast(fEmptyEntryRange.second - bulkBeginEntry)); + while (bulkBeginEntry < fEmptyEntryRange.second) { + RunAndCheckFilters(0, bulkBeginEntry, bulkSize); + bulkBeginEntry += bulkSize; + bulkSize = std::min(GetCurrentBulkSize(), static_cast(fEmptyEntryRange.second - bulkBeginEntry)); } } catch (...) { std::cerr << "RDataFrame::Run: event loop was interrupted\n"; @@ -622,11 +630,15 @@ void RLoopManager::RunDataSource() const auto start = range.first; const auto end = range.second; R__LOG_DEBUG(0, RDFLogChannel()) << LogRangeProcessing({fDataSource->GetLabel(), start, end, 0u}); - for (auto entry = start; entry < end && fNStopsReceived < fNChildren; ++entry) { - if (fDataSource->SetEntry(0u, entry)) { - RunAndCheckFilters(0u, entry); + auto bulkBeginEntry = start; + std::size_t bulkSize = std::min(GetCurrentBulkSize(), static_cast(end - bulkBeginEntry)); + while (bulkBeginEntry < end && fNStopsReceived < fNChildren) { + if (fDataSource->SetEntry(0u, bulkBeginEntry)) { + RunAndCheckFilters(0u, bulkBeginEntry, bulkSize); } - processedEntries++; + bulkBeginEntry += bulkSize; + processedEntries += bulkSize; + bulkSize = std::min(GetCurrentBulkSize(), static_cast(end - bulkBeginEntry)); } } } catch (...) { @@ -675,7 +687,7 @@ void RLoopManager::RunDataSourceMT() /// Execute actions and make sure named filters are called for each event. /// Named filters must be called even if the analysis logic would not require it, lest they report confusing results. -void RLoopManager::RunAndCheckFilters(unsigned int slot, Long64_t entry) +void RLoopManager::RunAndCheckFilters(unsigned int slot, Long64_t bulkBeginEntry, std::size_t bulkSize) { // data-block callbacks run before the rest of the graph if (fNewSampleNotifier.CheckFlag(slot)) { @@ -685,9 +697,9 @@ void RLoopManager::RunAndCheckFilters(unsigned int slot, Long64_t entry) } for (auto *actionPtr : fBookedActions) - actionPtr->Run(slot, entry); + actionPtr->Run(slot, bulkBeginEntry, bulkSize); for (auto *namedFilterPtr : fBookedNamedFilters) - namedFilterPtr->CheckFilters(slot, entry); + namedFilterPtr->CheckFilters(slot, bulkBeginEntry, bulkSize); for (auto &callback : fCallbacksEveryNEvents) callback(slot); } @@ -1027,10 +1039,10 @@ void RLoopManager::Deregister(RDFInternal::RVariationBase *v) } // dummy call, end of recursive chain of calls -ROOT::Internal::RDF::RMaskedEntryRange RLoopManager::CheckFilters(unsigned int, Long64_t entry) +ROOT::Internal::RDF::RMaskedEntryRange +RLoopManager::CheckFilters(unsigned int, Long64_t bulkBeginEntry, std::size_t bulkSize) { - // Assume 1-size bulk for now - return ROOT::Internal::RDF::RMaskedEntryRange{1ul, true, static_cast(entry)}; + return ROOT::Internal::RDF::RMaskedEntryRange{bulkSize, true, static_cast(bulkBeginEntry)}; } /// Call `FillReport` on all booked filters @@ -1328,10 +1340,14 @@ void ROOT::Detail::RDF::RLoopManager::DataSourceThreadTask(const std::pairGetLabel(), start, end, slot}); try { - for (auto entry = start; entry < end; ++entry) { - if (fDataSource->SetEntry(slot, entry)) { - RunAndCheckFilters(slot, entry); + auto bulkBeginEntry = start; + std::size_t bulkSize = std::min(GetCurrentBulkSize(), static_cast(end - bulkBeginEntry)); + while (bulkBeginEntry < end) { + if (fDataSource->SetEntry(slot, bulkBeginEntry)) { + RunAndCheckFilters(slot, bulkBeginEntry, bulkSize); } + bulkBeginEntry += bulkSize; + bulkSize = std::min(GetCurrentBulkSize(), static_cast(end - bulkBeginEntry)); } } catch (...) { std::cerr << "RDataFrame::Run: event loop was interrupted\n"; @@ -1401,7 +1417,9 @@ void ROOT::Detail::RDF::RLoopManager::TTreeThreadTask(TTreeReader &treeReader, R if (fNewSampleNotifier.CheckFlag(slot)) { UpdateSampleInfo(slot, treeReader); } - RunAndCheckFilters(slot, count++); + std::size_t curBulkSize = std::min(GetCurrentBulkSize(), static_cast(end - start)); + RunAndCheckFilters(slot, count, curBulkSize); + count += curBulkSize; } } catch (...) { std::cerr << "RDataFrame::Run: event loop was interrupted\n"; From 31167f0f1454e7d2a7e1a291906c7b452e40eedb Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Wed, 3 Jun 2026 13:22:17 +0200 Subject: [PATCH 4/8] [df] Prepare for bulk nodes that update values define and variation nodes update their current available values via an Update method, this is now prepared to work in bulks, currently assuming always size one. --- .../inc/ROOT/RDF/RDefaultValueFor.hxx | 22 +++++++++---- tree/dataframe/inc/ROOT/RDF/RDefine.hxx | 30 ++++++++++------- .../inc/ROOT/RDF/RDefinePerSample.hxx | 21 +++++++----- tree/dataframe/inc/ROOT/RDF/RVariation.hxx | 32 ++++++++++++------- 4 files changed, 67 insertions(+), 38 deletions(-) diff --git a/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx b/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx index c7cb63af96def..f8c1dc8ef0c87 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefaultValueFor.hxx @@ -45,11 +45,12 @@ template class R__CLING_PTRCHECK(off) RDefaultValueFor final : public RDefineBase { using ColumnTypes_t = ROOT::TypeTraits::TypeList; using TypeInd_t = std::make_index_sequence; - // Avoid instantiating vector as `operator[]` returns temporaries in that case. Use std::deque instead. - using ValuesPerSlot_t = std::conditional_t::value, std::deque, std::vector>; + + using ValuesPerSlot_t = std::vector>; T fDefaultValue; - ValuesPerSlot_t fLastResults; + // Each slot accesses a cache of values for the current bulk + ValuesPerSlot_t fCachedResultsPerSlot; // One column reader per slot std::vector fValues; @@ -71,12 +72,16 @@ public: RLoopManager &lm, const std::string &variationName = "nominal") : RDefineBase(name, type, colRegister, lm, columns, variationName), fDefaultValue(defaultValue), - fLastResults(lm.GetNSlots() * RDFInternal::CacheLineStep()), + fCachedResultsPerSlot(lm.GetNSlots() * RDFInternal::CacheLineStep>()), fValues(lm.GetNSlots()) { fLoopManager->Register(this); // We suppress errors that TTreeReader prints regarding the missing branch fLoopManager->InsertSuppressErrorsForMissingBranch(fColumnNames[0]); + // Assume 1-size bulk for now + for (decltype(lm.GetNSlots()) i = 0; i < lm.GetNSlots(); ++i) { + fCachedResultsPerSlot[i * RDFInternal::CacheLineStep>()].resize(1ul); + } } RDefaultValueFor(const RDefaultValueFor &) = delete; @@ -97,10 +102,10 @@ public: fLastCheckedEntry[slot * RDFInternal::CacheLineStep()] = -1; } - /// Return the (type-erased) address of the Define'd value for the given processing slot. + /// Return the beginning of the cached results of the current bulk for the input processing slot void *GetValuePtr(unsigned int slot) final { - return static_cast(&fLastResults[slot * RDFInternal::CacheLineStep()]); + return static_cast(fCachedResultsPerSlot[slot * RDFInternal::CacheLineStep>()].data()); } /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry @@ -113,9 +118,12 @@ public: // Assume 1-size bulk for now fValues[slot]->Load(mask); const std::size_t bulkSize = fLoopManager->GetCurrentBulkSize(); + auto &result = fCachedResultsPerSlot[slot * RDFInternal::CacheLineStep>()]; + result.clear(); + result.resize(bulkSize); for (std::size_t i = 0; i < bulkSize; ++i) { if (mask[i]) - fLastResults[slot * RDFInternal::CacheLineStep()] = GetValueOrDefault(slot, i); + fCachedResultsPerSlot[slot * RDFInternal::CacheLineStep>()][i] = GetValueOrDefault(slot, i); } fLastCheckedEntry[slot * RDFInternal::CacheLineStep()] = mask.GetFirstEntry(); } diff --git a/tree/dataframe/inc/ROOT/RDF/RDefine.hxx b/tree/dataframe/inc/ROOT/RDF/RDefine.hxx index 2c2eb8cb15620..30f64e09a96e9 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefine.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefine.hxx @@ -56,12 +56,11 @@ class R__CLING_PTRCHECK(off) RDefine final : public RDefineBase { RDFInternal::RemoveFirstTwoParametersIf_t::value, ColumnTypesTmp_t>; using TypeInd_t = std::make_index_sequence; using ret_type = typename CallableTraits::ret_type; - // Avoid instantiating vector as `operator[]` returns temporaries in that case. Use std::deque instead. - using ValuesPerSlot_t = - std::conditional_t::value, std::deque, std::vector>; + using ValuesPerSlot_t = std::vector>; F fExpression; - ValuesPerSlot_t fLastResults; + // Each slot accesses a cache of values for the current bulk + ValuesPerSlot_t fCachedResultsPerSlot; /// Column readers per slot and per input column std::vector> fValues; @@ -114,10 +113,16 @@ public: RDefine(std::string_view name, std::string_view type, F expression, const ROOT::RDF::ColumnNames_t &columns, const RDFInternal::RColumnRegister &colRegister, RLoopManager &lm, const std::string &variationName = "nominal") - : RDefineBase(name, type, colRegister, lm, columns, variationName), fExpression(std::move(expression)), - fLastResults(lm.GetNSlots() * RDFInternal::CacheLineStep()), fValues(lm.GetNSlots()) + : RDefineBase(name, type, colRegister, lm, columns, variationName), + fExpression(std::move(expression)), + fCachedResultsPerSlot(lm.GetNSlots() * RDFInternal::CacheLineStep>()), + fValues(lm.GetNSlots()) { fLoopManager->Register(this); + // Assume 1-size bulk for now + for (decltype(lm.GetNSlots()) i = 0; i < lm.GetNSlots(); ++i) { + fCachedResultsPerSlot[i * RDFInternal::CacheLineStep>()].resize(1ul); + } } RDefine(const RDefine &) = delete; @@ -131,13 +136,14 @@ public: fLastCheckedEntry[slot * RDFInternal::CacheLineStep()] = -1; } - /// Return the (type-erased) address of the Define'd value for the given processing slot. + /// Return the beginning of the cached results of the current bulk for the input processing slot void *GetValuePtr(unsigned int slot) final { - return static_cast(&fLastResults[slot * RDFInternal::CacheLineStep()]); + return static_cast( + fCachedResultsPerSlot[slot * RDFInternal::CacheLineStep>()].data()); } - /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry + /// Update the values at the array returned by GetValuePtr with the content corresponding to the given mask void Update(unsigned int slot, const ROOT::Internal::RDF::RMaskedEntryRange &mask) final { if (static_cast(mask.GetFirstEntry()) == @@ -147,10 +153,12 @@ public: std::for_each(fValues[slot].begin(), fValues[slot].end(), [&mask](auto *v) { v->Load(mask); }); // Assume 1-size bulk for now const std::size_t bulkSize = 1; - auto &result = fLastResults[slot * RDFInternal::CacheLineStep()]; + auto &result = fCachedResultsPerSlot[slot * RDFInternal::CacheLineStep>()]; + result.clear(); + result.resize(bulkSize); for (std::size_t i = 0; i < bulkSize; ++i) { if (mask[i]) { - result = UpdateHelper(slot, i, mask.GetFirstEntry() + i, ColumnTypes_t{}, TypeInd_t{}, ExtraArgsTag{}); + result[i] = UpdateHelper(slot, i, mask.GetFirstEntry() + i, ColumnTypes_t{}, TypeInd_t{}, ExtraArgsTag{}); fLastCheckedEntry[slot * RDFInternal::CacheLineStep()] = mask.GetFirstEntry(); } } diff --git a/tree/dataframe/inc/ROOT/RDF/RDefinePerSample.hxx b/tree/dataframe/inc/ROOT/RDF/RDefinePerSample.hxx index afa62a9670f35..6b0e8e5d74ed2 100644 --- a/tree/dataframe/inc/ROOT/RDF/RDefinePerSample.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RDefinePerSample.hxx @@ -30,22 +30,25 @@ template class R__CLING_PTRCHECK(off) RDefinePerSample final : public RDefineBase { using RetType_t = typename CallableTraits::ret_type; - // Avoid instantiating vector as `operator[]` returns temporaries in that case. Use std::deque instead. - using ValuesPerSlot_t = - std::conditional_t::value, std::deque, std::vector>; + using ValuesPerSlot_t = std::vector>; F fExpression; - ValuesPerSlot_t fLastResults; + // Each slot accesses a cache of values for the current bulk + ValuesPerSlot_t fCachedResultsPerSlot; public: RDefinePerSample(std::string_view name, std::string_view type, F expression, RLoopManager &lm) : RDefineBase(name, type, RDFInternal::RColumnRegister{&lm}, lm, /*columnNames*/ {}), fExpression(std::move(expression)), - fLastResults(lm.GetNSlots() * RDFInternal::CacheLineStep()) + fCachedResultsPerSlot(lm.GetNSlots() * RDFInternal::CacheLineStep>()) { fLoopManager->Register(this); auto callUpdate = [this](unsigned int slot, const ROOT::RDF::RSampleInfo &id) { this->Update(slot, id); }; fLoopManager->AddSampleCallback(this, std::move(callUpdate)); + // Assume 1-size bulk for now + for (decltype(lm.GetNSlots()) i = 0; i < lm.GetNSlots(); ++i) { + fCachedResultsPerSlot[i * RDFInternal::CacheLineStep>()].resize(1ul); + } } RDefinePerSample(const RDefinePerSample &) = delete; @@ -53,10 +56,11 @@ public: ~RDefinePerSample() { fLoopManager->Deregister(this); } - /// Return the (type-erased) address of the Define'd value for the given processing slot. + /// Return the beginning of the cached results of the current bulk for the input processing slot void *GetValuePtr(unsigned int slot) final { - return static_cast(&fLastResults[slot * RDFInternal::CacheLineStep()]); + return static_cast( + fCachedResultsPerSlot[slot * RDFInternal::CacheLineStep>()].data()); } void Update(unsigned int, const ROOT::Internal::RDF::RMaskedEntryRange &) final @@ -67,7 +71,8 @@ public: /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry void Update(unsigned int slot, const ROOT::RDF::RSampleInfo &id) final { - fLastResults[slot * RDFInternal::CacheLineStep()] = fExpression(slot, id); + // Assume 1-size bulk for now + fCachedResultsPerSlot[slot * RDFInternal::CacheLineStep>()][0] = fExpression(slot, id); } const std::type_info &GetTypeId() const final { return typeid(RetType_t); } diff --git a/tree/dataframe/inc/ROOT/RDF/RVariation.hxx b/tree/dataframe/inc/ROOT/RDF/RVariation.hxx index 3b66497a69f1d..5cf18ab7709d9 100644 --- a/tree/dataframe/inc/ROOT/RDF/RVariation.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RVariation.hxx @@ -77,9 +77,9 @@ void AssignResults(ROOT::RVec &resStorage, ROOT::RVec &&tmpResults) } template -void *GetValuePtrHelper(ROOT::RVec &v, std::size_t /*colIdx*/, std::size_t varIdx) +void *GetValuePtrHelper(ROOT::RVec> &v, std::size_t /*colIdx*/, std::size_t varIdx) { - return static_cast(&v[varIdx]); + return static_cast(&v[0][varIdx]); } ///@} @@ -119,9 +119,9 @@ void AssignResults(std::vector> &resStorage, ROOT::RVec -void *GetValuePtrHelper(std::vector> &v, std::size_t colIdx, std::size_t varIdx) +void *GetValuePtrHelper(ROOT::RVec>> &v, std::size_t colIdx, std::size_t varIdx) { - return static_cast(&v[colIdx][varIdx]); + return static_cast(&v[0][colIdx][varIdx]); } ///@} @@ -151,10 +151,11 @@ class R__CLING_PTRCHECK(off) RVariation final : public RVariationBase { using Ret_t = typename CallableTraits::ret_type; using VariedCol_t = ColumnType_t; using Result_t = std::conditional_t, std::vector>>; + using ValuesPerSlot_t = std::vector>; F fExpression; - /// Per-slot storage for varied column values (for one or multiple columns depending on IsSingleColumn). - std::vector fLastResults; + // Each slot accesses a cache of values for the current bulk + ValuesPerSlot_t fCachedResultsPerSlot; /// Column readers per slot and per input column std::vector> fValues; @@ -186,7 +187,8 @@ class R__CLING_PTRCHECK(off) RVariation final : public RVariationBase { std::to_string(fVariationNames.size()) + " were expected."); } - AssignResults(fLastResults[slot * CacheLineStep()], std::move(results)); + AssignResults(fCachedResultsPerSlot[slot * RDFInternal::CacheLineStep>()][0], + std::move(results)); } public: @@ -195,13 +197,17 @@ public: RLoopManager &lm, const ColumnNames_t &inputColNames) : RVariationBase(colNames, variationName, variationTags, type, defines, lm, inputColNames), fExpression(std::move(expression)), - fLastResults(lm.GetNSlots() * CacheLineStep()), + fCachedResultsPerSlot(lm.GetNSlots() * RDFInternal::CacheLineStep>()), fValues(lm.GetNSlots()) { fLoopManager->Register(this); - for (auto i = 0u; i < lm.GetNSlots(); ++i) - ResizeResults(fLastResults[i * CacheLineStep()], colNames.size(), variationTags.size()); + // Assume 1-size bulk for now + for (decltype(lm.GetNSlots()) i = 0; i < lm.GetNSlots(); ++i) { + auto &cachedResultsForThisSlot = fCachedResultsPerSlot[i * RDFInternal::CacheLineStep>()]; + cachedResultsForThisSlot.resize(1ul); + ResizeResults(cachedResultsForThisSlot[0], colNames.size(), variationTags.size()); + } } RVariation(const RVariation &) = delete; @@ -215,7 +221,8 @@ public: fLastCheckedEntry[slot * CacheLineStep()] = -1; } - /// Return the (type-erased) address of the value for the given processing slot. + /// Return the beginning of the cached results of the current bulk for the input processing slot, column and + /// variation void *GetValuePtr(unsigned int slot, const std::string &column, const std::string &variation) final { const auto colIt = std::find(fColNames.begin(), fColNames.end(), column); @@ -226,7 +233,8 @@ public: assert(varIt != fVariationNames.end()); const auto varIdx = std::distance(fVariationNames.begin(), varIt); - return GetValuePtrHelper(fLastResults[slot * CacheLineStep()], colIdx, varIdx); + return GetValuePtrHelper(fCachedResultsPerSlot[slot * RDFInternal::CacheLineStep>()], colIdx, + varIdx); } /// Update the value at the address returned by GetValuePtr with the content corresponding to the given entry From 46b731fc86d91a54f75c26df592b3e76fbf88ce7 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Mon, 5 Oct 2026 14:18:43 +0200 Subject: [PATCH 5/8] [df] Prepare for reading TTreeReaderValue in bulks Enables reading TTree data via TTreeReaderValue into a pre-allocated memory region that will eventually contain the bulk of entries, assuming for now a bulk size of one. The changes are backwards compatible, all tests work with the new bulk loading logic, with one exception and a minor remark. The remark is that the test_ctors is modified since now the construction and destruction of class instances happens at the boundary of a bulk. Before, TTree could optimize by IO-constructing the value once and then re-assign it across entries. The exception is the test called PolymorphicTBranchObject in dataframe_regressions.cxx, which reads a TTree branch of on-disk type TObject where at each entry the concrete type of the value is different (namely a TList in the first entry and a TH1D in the second entry). In particular, this is a TBranchObject with split level zero. The approach taken is to disable the bulk loading from RDataFrame in this case and only allow reading one entry at a time for this type of branch. Co-authored-by: Philippe Canal --- roottest/root/dataframe/test_ctors.ref | 4 +- .../inc/ROOT/RDF/RColumnReaderBase.hxx | 6 +- .../inc/ROOT/RDF/RMaskedEntryRange.hxx | 9 ++ .../inc/ROOT/RDF/RTreeColumnReader.hxx | 9 +- tree/dataframe/src/RTreeColumnReader.cxx | 74 ++++++++++-- tree/tree/inc/TTree.h | 6 + tree/tree/src/TTree.cxx | 9 ++ tree/treeplayer/inc/TTreeReaderArray.h | 2 +- tree/treeplayer/inc/TTreeReaderValue.h | 13 ++- tree/treeplayer/src/TTreeReaderValue.cxx | 108 ++++++++++++++++++ 10 files changed, 225 insertions(+), 15 deletions(-) diff --git a/roottest/root/dataframe/test_ctors.ref b/roottest/root/dataframe/test_ctors.ref index e499d343d35a6..1efaf0ae5fd19 100644 --- a/roottest/root/dataframe/test_ctors.ref +++ b/roottest/root/dataframe/test_ctors.ref @@ -11,8 +11,10 @@ running chain... ctor called filter foreach: 1 +dtor called +ctor called filter foreach: 2 +dtor called done count: 2 -dtor called diff --git a/tree/dataframe/inc/ROOT/RDF/RColumnReaderBase.hxx b/tree/dataframe/inc/ROOT/RDF/RColumnReaderBase.hxx index be5f8a6847f55..0323d03b6363b 100644 --- a/tree/dataframe/inc/ROOT/RDF/RColumnReaderBase.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RColumnReaderBase.hxx @@ -43,13 +43,13 @@ public: /// The caller is responsible for checking that the returned value actually /// exists. template - T *TryGet(std::size_t idx) + T *TryGet(std::size_t entryInBulk) { - return static_cast(GetImpl(idx)); + return static_cast(GetImpl(entryInBulk)); } private: - virtual void *GetImpl(std::size_t idx) = 0; + virtual void *GetImpl(std::size_t entryInBulk) = 0; virtual void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) = 0; }; diff --git a/tree/dataframe/inc/ROOT/RDF/RMaskedEntryRange.hxx b/tree/dataframe/inc/ROOT/RDF/RMaskedEntryRange.hxx index 55da9b3caea18..227959fbe200d 100644 --- a/tree/dataframe/inc/ROOT/RDF/RMaskedEntryRange.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RMaskedEntryRange.hxx @@ -32,6 +32,15 @@ public: bool &operator[](std::size_t idx) { return fMask.at(idx); } std::uint64_t GetFirstEntry() const { return fBegin; } void SetFirstEntry(std::uint64_t e) { fBegin = e; } + ROOT::RVec GetValidIndices() const + { + ROOT::RVec validIndices{}; + for (std::size_t i = 0; i < fMask.size(); ++i) { + if (fMask[i]) + validIndices.push_back(i); + } + return validIndices; + } }; } // namespace ROOT::Internal::RDF diff --git a/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx b/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx index b750a74f89bd3..6f4ee278f768f 100644 --- a/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx @@ -58,7 +58,14 @@ public: /// RTreeColumnReader specialization for TTree values read via TTreeReaderUntypedValue class R__CLING_PTRCHECK(off) RTreeUntypedValueColumnReader final : public ROOT::Detail::RDF::RColumnReaderBase { std::unique_ptr fTreeValue; - void *fValuePtr{nullptr}; + ROOT::RVec fCachedResults{}; + ROOT::RVec fCachedResultsInvalidIndices{}; + ROOT::RVec fCachedBranchAddresses{}; + ROOT::RVec fCachedBranchAddressesOfAddresses{}; + std::function fObjDeleter; + std::size_t fValueSize{0}; + std::uint64_t fLastEntry = std::numeric_limits::max(); + void *fAddrOfTypeThatCannotBeReadInBulk{}; void *GetImpl(std::size_t) override; void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) override; diff --git a/tree/dataframe/src/RTreeColumnReader.cxx b/tree/dataframe/src/RTreeColumnReader.cxx index 6a6c35c2965c0..4a4b436279a96 100644 --- a/tree/dataframe/src/RTreeColumnReader.cxx +++ b/tree/dataframe/src/RTreeColumnReader.cxx @@ -4,6 +4,7 @@ #include #include +#include void *ROOT::Internal::RDF::RTreeOpaqueColumnReader::GetImpl(std::size_t) { @@ -12,7 +13,7 @@ void *ROOT::Internal::RDF::RTreeOpaqueColumnReader::GetImpl(std::size_t) void ROOT::Internal::RDF::RTreeOpaqueColumnReader::LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &mask) { - // Assume size-1 bulk for now + // Assume 1-size bulk for now if (mask[0]) fValuePtr = fTreeValue->GetAddress(); } @@ -24,16 +25,68 @@ ROOT::Internal::RDF::RTreeOpaqueColumnReader::RTreeOpaqueColumnReader(TTreeReade ROOT::Internal::RDF::RTreeOpaqueColumnReader::~RTreeOpaqueColumnReader() = default; -void *ROOT::Internal::RDF::RTreeUntypedValueColumnReader::GetImpl(std::size_t) +void *ROOT::Internal::RDF::RTreeUntypedValueColumnReader::GetImpl(std::size_t entryInBulk) { - return fValuePtr; + + if (fAddrOfTypeThatCannotBeReadInBulk) + return fAddrOfTypeThatCannotBeReadInBulk; + + assert(fValueSize > 0 && "Could not retrieve size of value type in RDataFrame column reader."); + if (fCachedResultsInvalidIndices.end() != + std::find(fCachedResultsInvalidIndices.begin(), fCachedResultsInvalidIndices.end(), entryInBulk)) { + // This entry was marked as invalid during loading, return nullptr to signal this to the caller + return nullptr; + } + + return fCachedResults.data() + entryInBulk * fValueSize; } void ROOT::Internal::RDF::RTreeUntypedValueColumnReader::LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &mask) { - // Assume size-1 bulk for now - if (mask[0]) - fValuePtr = fTreeValue->Get(); + // Early return for the case of TBranchObject with split level zero, which cannot be read in a bulk + if (fTreeValue->IsBranchObjectUnsplit()) { + fAddrOfTypeThatCannotBeReadInBulk = fTreeValue->Get(); + return; + } + + if (fLastEntry == mask.GetFirstEntry()) + return; + + if (fValueSize == 0) + fValueSize = fTreeValue->GetValueSize(); + assert(fValueSize > 0 && "Could not retrieve size of value type in RDataFrame column reader."); + + // Assume 1-size bulk for now + const auto validIndices = mask.GetValidIndices(); + fLastEntry = mask.GetFirstEntry(); + if (validIndices.empty()) + return; + + fCachedResultsInvalidIndices.clear(); + if (fObjDeleter) + for (auto *addr : fCachedBranchAddresses) + fObjDeleter(addr); + fCachedBranchAddressesOfAddresses.clear(); + fCachedBranchAddresses.clear(); + fCachedResults.clear(); + + fCachedResultsInvalidIndices.reserve(validIndices.size()); + fCachedBranchAddressesOfAddresses.reserve(validIndices.size() * fValueSize); + fCachedBranchAddresses.reserve(validIndices.size() * fValueSize); + fCachedResults.resize(validIndices.size() * fValueSize); + + for (auto idx : validIndices) { + fCachedBranchAddresses.push_back(fCachedResults.data() + idx * fValueSize); + // Crucial for reading non-PODs. Also crucial that this specific address + // survives until the end of the Get method which reads the actual value + // from the branch into the memory location above + fCachedBranchAddressesOfAddresses.push_back(&fCachedBranchAddresses.back()); + + fObjDeleter = fTreeValue->SetAddress(fCachedBranchAddresses.back(), fCachedBranchAddressesOfAddresses.back()); + auto *readAddress = fTreeValue->Get(); + if (!readAddress) + fCachedResultsInvalidIndices.push_back(idx); + } } ROOT::Internal::RDF::RTreeUntypedValueColumnReader::RTreeUntypedValueColumnReader(TTreeReader &r, @@ -43,7 +96,12 @@ ROOT::Internal::RDF::RTreeUntypedValueColumnReader::RTreeUntypedValueColumnReade { } -ROOT::Internal::RDF::RTreeUntypedValueColumnReader::~RTreeUntypedValueColumnReader() = default; +ROOT::Internal::RDF::RTreeUntypedValueColumnReader::~RTreeUntypedValueColumnReader() +{ + if (fObjDeleter) + for (auto *addr : fCachedBranchAddresses) + fObjDeleter(addr); +}; void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadStdArray(Long64_t entry) { @@ -176,7 +234,7 @@ void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadRVec(Long64_t entr void ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &mask) { - // Assume size-1 bulk for now + // Assume 1-size bulk for now if (mask[0]) { if (fCollectionType == ECollectionType::kStdArray) fValuePtr = LoadStdArray(mask.GetFirstEntry()); diff --git a/tree/tree/inc/TTree.h b/tree/tree/inc/TTree.h index 0a540b86142ca..5a2f0dcb0ce27 100644 --- a/tree/tree/inc/TTree.h +++ b/tree/tree/inc/TTree.h @@ -90,6 +90,9 @@ TBranch *CallBranchImp(TTree &tree, const char *branchname, TClass *ptrClass, vo // TBranchElement::GetObject() return as a string. Longptr_t GetBranchAddress(TBranch *branch); Longptr_t GetBranchElementObject(TBranchElement *branch); + +Int_t SetBranchAddress(TTree &tree, const char *bname, void *add, TBranch **ptr, TClass *realClass, EDataType datatype, + bool isptr, bool suppressMissingBranchError); } class TTree : public TNamed, public TAttLine, public TAttFill, public TAttMarker { @@ -200,6 +203,9 @@ class TTree : public TNamed, public TAttLine, public TAttFill, public TAttMarker Int_t splitlevel); friend TBranch *ROOT::Internal::TreeUtils::CallBranchImp(TTree &tree, const char *branchname, TClass *ptrClass, void *addobj, Int_t bufsize, Int_t splitlevel); + friend Int_t ROOT::Internal::TreeUtils::SetBranchAddress(TTree &tree, const char *bname, void *add, TBranch **ptr, + TClass *realClass, EDataType datatype, bool isptr, + bool suppressMissingBranchError); virtual void KeepCircular(); virtual TBranch *BranchImp(const char* branchname, const char* classname, TClass* ptrClass, void* addobj, Int_t bufsize, Int_t splitlevel); virtual TBranch *BranchImp(const char* branchname, TClass* ptrClass, void* addobj, Int_t bufsize, Int_t splitlevel); diff --git a/tree/tree/src/TTree.cxx b/tree/tree/src/TTree.cxx index 303bb3e111506..80a5481ab4699 100644 --- a/tree/tree/src/TTree.cxx +++ b/tree/tree/src/TTree.cxx @@ -10306,3 +10306,12 @@ Longptr_t ROOT::Internal::TreeUtils::GetBranchElementObject(TBranchElement *bran { return reinterpret_cast(branch->GetObject()); } + +Int_t ROOT::Internal::TreeUtils::SetBranchAddress(TTree &tree, const char *bname, void *add, TBranch **ptr, + TClass *realClass, EDataType datatype, bool isptr, + bool suppressMissingBranchError) +{ + // We unconditionally call the overload that will not print errors related to branch address, assuming that the + // caller has a different way to detect and report the issue to the user when needed. + return tree.SetBranchAddress(bname, add, ptr, realClass, datatype, isptr, suppressMissingBranchError); +} diff --git a/tree/treeplayer/inc/TTreeReaderArray.h b/tree/treeplayer/inc/TTreeReaderArray.h index e39a5cd79bd33..6139870b7c9a0 100644 --- a/tree/treeplayer/inc/TTreeReaderArray.h +++ b/tree/treeplayer/inc/TTreeReaderArray.h @@ -49,7 +49,7 @@ class TTreeReaderArrayBase : public TTreeReaderValueBase { bool IsContiguous() const { return fImpl->IsContiguous(GetProxy()); } /// Returns the `sizeof` of the collection value type. Returns 0 in case the value size could not be retrieved. - std::size_t GetValueSize() const { return fImpl ? fImpl->GetValueSize(GetProxy()) : 0; } + std::size_t GetValueSize() const override { return fImpl ? fImpl->GetValueSize(GetProxy()) : 0; } protected: void *UntypedAt(std::size_t idx) const { return fImpl->At(GetProxy(), idx); } diff --git a/tree/treeplayer/inc/TTreeReaderValue.h b/tree/treeplayer/inc/TTreeReaderValue.h index 316672c771ef0..18afe9a7b75e4 100644 --- a/tree/treeplayer/inc/TTreeReaderValue.h +++ b/tree/treeplayer/inc/TTreeReaderValue.h @@ -93,15 +93,22 @@ class TTreeReaderValueBase { ESetupStatus GetSetupStatus() const { return fSetupStatus; } virtual EReadStatus GetReadStatus() const { return fReadStatus; } + virtual std::size_t GetValueSize() const; + /// If we are reading a leaf, return the corresponding TLeaf. TLeaf *GetLeaf() { return fLeaf; } void *GetAddress(); + std::function SetAddress(void *addr, void *addrOfAddr); const char *GetBranchName() const { return fBranchName; } virtual ~TTreeReaderValueBase(); + TTreeReader *GetTreeReader() const { return fTreeReader; } + + bool IsBranchObjectUnsplit() const { return fIsBranchObjectUnsplit; } + protected: TTreeReaderValueBase(TTreeReader *reader, const char *branchname, TDictionary *dict, bool opaqueRead = false); TTreeReaderValueBase(const TTreeReaderValueBase &); @@ -136,7 +143,8 @@ class TTreeReaderValueBase { TString fBranchName; ///< Name of the branch to read data from. TString fLeafName; TTreeReader *fTreeReader; ///< Tree reader we belong to - TDictionary *fDict; ///< Type that the branch should contain + TDictionary *fDict; ///< The user requests to read branch values as this type + TDictionary *fActualDict; ///< On-disk type of the branch Detail::TBranchProxy *fProxy = nullptr; ///< Proxy for this branch, owned by TTreeReader TLeaf *fLeaf = nullptr; std::vector fStaticClassOffsets; @@ -150,6 +158,9 @@ class TTreeReaderValueBase { */ bool fOpaqueRead{false}; + // If true, the connected branch is a TBranchObject with split level zero, which cannot be read in a bulk + bool fIsBranchObjectUnsplit{false}; + // FIXME: re-introduce once we have ClassDefInline! // ClassDefOverride(TTreeReaderValueBase, 0);//Base class for accessors to data via TTreeReader diff --git a/tree/treeplayer/src/TTreeReaderValue.cxx b/tree/treeplayer/src/TTreeReaderValue.cxx index 01284f75bcfbe..65e88b71270ee 100644 --- a/tree/treeplayer/src/TTreeReaderValue.cxx +++ b/tree/treeplayer/src/TTreeReaderValue.cxx @@ -62,6 +62,7 @@ ROOT::Internal::TTreeReaderValueBase::TTreeReaderValueBase(TTreeReader *reader / fBranchName(branchname), fTreeReader(reader), fDict(dict), + fActualDict(nullptr), fOpaqueRead(opaqueRead) { auto * cl = dynamic_cast(dict); @@ -84,6 +85,7 @@ ROOT::Internal::TTreeReaderValueBase::TTreeReaderValueBase(const TTreeReaderValu fLeafName(rhs.fLeafName), fTreeReader(rhs.fTreeReader), fDict(rhs.fDict), + fActualDict(rhs.fActualDict), fProxy(rhs.fProxy), fLeaf(rhs.fLeaf), fStaticClassOffsets(rhs.fStaticClassOffsets) @@ -297,6 +299,68 @@ void *ROOT::Internal::TTreeReaderValueBase::GetAddress() return (Byte_t *)fProxy->GetWhere(); } +std::function ROOT::Internal::TTreeReaderValueBase::SetAddress(void *addr, void *addrOfAddr) +{ + // assume we are connected to a TTreeReader and in turn it is connected to a TTree and we have information about + // the data type of the branch + assert(fTreeReader); + assert(fTreeReader->fTree); + assert(fDict); + + if (!fProxy) { + // Most probably this is a case of a missing branch that has not been found yet, we can just return early + return nullptr; + } + + TClass *cl{nullptr}; + EDataType dt{EDataType::kOther_t}; + auto *selectedDict = [&] { return (fActualDict && fDict && fActualDict != fDict) ? fActualDict : fDict; }(); + + if (fHaveLeaf && fLeaf) { + // Case of a leaf data member + fLeaf->SetAddress(addr); + // prepare_proxy(); + fProxy->fRead = -1; + fProxy->fWhere = addr; + return nullptr; + } else if (auto dictAsClass = dynamic_cast(selectedDict)) { + // Case of non-POD top-level branch + cl = dictAsClass; + if (strcmp(cl->GetName(), "TClonesArray") != 0) { + cl->New(addr); + } + TBranch *brPtr{nullptr}; + TBranch **brPtrPtr{&brPtr}; + ROOT::Internal::TreeUtils::SetBranchAddress(*(GetTreeReader()->GetTree()), fBranchName, addrOfAddr, brPtrPtr, cl, + dt, + /*isPtr*/ true, /*suppressErrorsForThisBranch*/ true); + assert(brPtr); + brPtr->SetAutoDelete(false); + if (strcmp(cl->GetName(), "TClonesArray") == 0) { + auto *be = dynamic_cast(brPtr); + if (be) + new (addr) TClonesArray(be->GetClonesName()); + } + // prepare_proxy(); + fProxy->fRead = -1; + fProxy->fWhere = addr; + return [cl](void *p) { cl->Destructor(p, /*dtorOnly=*/true); }; + } else if (auto dictAsDataType = dynamic_cast(selectedDict)) { + // Case of POD top-level branch + dt = static_cast(dictAsDataType->GetType()); // returns Int_t but the data member is an EDataType + ROOT::Internal::TreeUtils::SetBranchAddress(*(GetTreeReader()->GetTree()), fBranchName, addr, + /*branchPtr*/ nullptr, cl, dt, /*isPtr*/ false, + /*suppressErrorsForThisBranch*/ true); + // prepare_proxy(); + fProxy->fRead = -1; + fProxy->fWhere = addr; + return nullptr; + } + + Error("TTreeReaderValueBase::SetAddress", "Unrecognized TTreeReaderValue branch (or leaf) setup."); + return nullptr; +} + //////////////////////////////////////////////////////////////////////////////// /// \brief Search a branch the name of which contains a "." /// \param[out] myLeaf The leaf identified by the name if found (can be untouched). @@ -639,6 +703,18 @@ void ROOT::Internal::TTreeReaderValueBase::CreateProxy() } } + // At this point we may have found a type for the branch which is different than the type stored in fDict. This + // may happen for example when the user explicitly requests a reader with a different type than the on-disk one + // hoping to read it as such. We record the information of the true on-disk type and let other code detect and use + // it appropriately. + fActualDict = branchActualType; + + // We also take the chance to record information on whether the branch is a TBranchObject with split level 0, which + // cannot be read in bulk + if (dynamic_cast(branch) && branch->GetSplitLevel() == 0) { + fIsBranchObjectUnsplit = true; + } + if (!namedProxy) { // Search for the branchname, determine what it contains, and wire the // TBranchProxy representing it to us so we can access its data. @@ -850,6 +926,38 @@ void ROOT::Internal::TTreeReaderValueBase::ErrorAboutMissingProxyIfNeeded() fBranchName.Data()); } +std::size_t ROOT::Internal::TTreeReaderValueBase::GetValueSize() const +{ + // Prioritize true on-disk type information over user-requested type information + if (fActualDict && fDict && fActualDict != fDict) { + if (auto dataType = dynamic_cast(fActualDict)) + return dataType->Size(); + if (auto cl = dynamic_cast(fActualDict)) + return cl->Size(); + } + + // Attempt querying size information from available dictionary + if (fDict) { + if (auto dataType = dynamic_cast(fDict)) + return dataType->Size(); + if (auto cl = dynamic_cast(fDict)) + return cl->Size(); + } + + // If that failed, try with TBranchProxy, needs to be initialized + if (!fProxy) { + Error("TTreeReaderValueBase::GetValueSize()", "Proxy not set for branch %s. Cannot determine value size.", + fBranchName.Data()); + return 0; + } + if (!fProxy->Read()) { + Error("TTreeReaderValueBase::GetValueSize()", + "Failed to read from proxy for branch %s. Cannot determine value size.", fBranchName.Data()); + return 0; + } + return fProxy->GetValueSize(); +} + namespace cling { // The value printers of TTreeReaderValue and TTreeReaderArray rely on the // one of TTreeReaderValueBase, from which they both inherit. From c16d3fb1d4bd91db4668279f9aeadd6802b37d06 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Tue, 27 Oct 2026 18:05:54 +0100 Subject: [PATCH 6/8] Adapt custom loader action to bulk internals --- tree/dataframe/inc/ROOT/RDF/RActionCustomLoader.hxx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tree/dataframe/inc/ROOT/RDF/RActionCustomLoader.hxx b/tree/dataframe/inc/ROOT/RDF/RActionCustomLoader.hxx index a38564319aa69..886c5d415e83e 100644 --- a/tree/dataframe/inc/ROOT/RDF/RActionCustomLoader.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RActionCustomLoader.hxx @@ -95,22 +95,23 @@ public: "FilterAvailable/FilterMissing to discard/keep entries with missing values instead."}; } - void CallExec(unsigned int slot, Long64_t entry) + void CallExec(unsigned int slot, std::size_t idx) { auto &untypedValues = fValueAddresses[slot]; auto nReaders = fValues[slot].size(); untypedValues.resize(nReaders); for (decltype(nReaders) readerIdx{}; readerIdx < nReaders; readerIdx++) - untypedValues[readerIdx] = GetValue(slot, readerIdx, entry); + untypedValues[readerIdx] = GetValue(slot, readerIdx, idx); fHelper.Exec(slot, untypedValues); } - void Run(unsigned int slot, Long64_t entry) final + void Run(unsigned int slot, Long64_t bulkBeginEntry, std::size_t bulkSize) final { - - if (fPrevNodes.front()->CheckFilters(slot, entry)) - CallExec(slot, entry); + const auto mask = fPrevNodes.front()->CheckFilters(slot, bulkBeginEntry, bulkSize); + std::for_each(fValues[slot].begin(), fValues[slot].end(), [&mask](auto *v) { v->Load(mask); }); + if (mask[0]) + CallExec(slot, /*idx=*/0u); } void TriggerChildrenCount() final From fbec2a84345b84d1743197e91d965768de69e571 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Tue, 27 Oct 2026 18:06:15 +0100 Subject: [PATCH 7/8] Adapt RNTupleProcessorDS to bulk internals --- tree/dataframe/src/RNTupleProcessorDS.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tree/dataframe/src/RNTupleProcessorDS.cxx b/tree/dataframe/src/RNTupleProcessorDS.cxx index 05a7d95313ea1..49ebdcde66425 100644 --- a/tree/dataframe/src/RNTupleProcessorDS.cxx +++ b/tree/dataframe/src/RNTupleProcessorDS.cxx @@ -36,10 +36,10 @@ class RNTupleProcessorColumnReader : public ROOT::Detail::RDF::RColumnReaderBase RNTupleProcessorColumnReader(RNTupleProcessorValuePtr &valuePtr) : fValuePtr(std::move(valuePtr)) {} ~RNTupleProcessorColumnReader() override = default; - void *GetImpl(Long64_t entry [[maybe_unused]]) final + void *GetImpl(std::size_t /*idx*/) final { return fValuePtr.GetPtr().get(); } + void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) final { - assert(entry == fLastLoadedEntry); - return fValuePtr.GetPtr().get(); + // Loading happens in RNTupleProcessorDS } void SetEntryLoaded(Long64_t entry) { fLastLoadedEntry = entry; } From 007404a3d2e37c2224ea679b9d3ef1621a949195 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Sun, 18 Oct 2026 09:25:03 +0200 Subject: [PATCH 8/8] [df] Prepare for reading TTreeReaderArray values in bulk A cache of values of the bulk is created at construction time of RTreeUntypedArrayColumnReader, since TTree requires stable addresses in memory to read branch values from. At loading time, the values are read into the corresponding location of the cache for the current bulk entry index. Currently, this assumes always a bulk size of one. --- .../inc/ROOT/RDF/RMaskedEntryRange.hxx | 1 + .../inc/ROOT/RDF/RTreeColumnReader.hxx | 23 ++- tree/dataframe/src/RLoopManager.cxx | 10 +- tree/dataframe/src/RTreeColumnReader.cxx | 136 +++++++++++------- 4 files changed, 99 insertions(+), 71 deletions(-) diff --git a/tree/dataframe/inc/ROOT/RDF/RMaskedEntryRange.hxx b/tree/dataframe/inc/ROOT/RDF/RMaskedEntryRange.hxx index 227959fbe200d..68498798635b3 100644 --- a/tree/dataframe/inc/ROOT/RDF/RMaskedEntryRange.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RMaskedEntryRange.hxx @@ -32,6 +32,7 @@ public: bool &operator[](std::size_t idx) { return fMask.at(idx); } std::uint64_t GetFirstEntry() const { return fBegin; } void SetFirstEntry(std::uint64_t e) { fBegin = e; } + std::size_t GetBulkSize() const { return fMask.size(); } ROOT::RVec GetValidIndices() const { ROOT::RVec validIndices{}; diff --git a/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx b/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx index 6f4ee278f768f..a1cedf50c8ecb 100644 --- a/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx @@ -107,29 +107,26 @@ private: std::unique_ptr fTreeArray; ECollectionType fCollectionType; - using Byte_t = std::byte; - /// We return a reference to this RVec to clients, to guarantee a stable address and contiguous memory layout. - RVec fRVec{}; - - /// When the user explicitly requests std::vector, we use this std::vector as a stable storage. - std::vector fStdVector{}; - - Long64_t fLastEntry = -1; + std::uint64_t fLastEntry{std::numeric_limits::max()}; /// The size of the collection value type. std::size_t fValueSize{}; + // We absolutely require stable addresses for all collections in the bulk, so we need to instantiate a bulk of + // collections at construction time (currrently fixed to size 1) and then always reuse those collections. + using CacheStorage_t = std::variant>, std::vector>>; + CacheStorage_t fCachedValues{}; + ROOT::RVec fCachedValuesInvalidIndices{}; + /// Whether we already printed a warning about performing a copy of the TTreeReaderArray contents bool fCopyWarningPrinted = false; - void *fValuePtr{nullptr}; - void *GetImpl(std::size_t) override; void LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &) override; - void *LoadStdArray(Long64_t entry); - void *LoadStdVector(Long64_t entry); - void *LoadRVec(Long64_t entry); + void LoadStdArray(std::size_t entryInBulk); + void LoadStdVector(std::size_t entryInBulk); + void LoadRVec(std::size_t entryInBulk); }; class R__CLING_PTRCHECK(off) RMaskedColumnReader : public ROOT::Detail::RDF::RColumnReaderBase { diff --git a/tree/dataframe/src/RLoopManager.cxx b/tree/dataframe/src/RLoopManager.cxx index 8d827dbf0f6d1..85ae237302581 100644 --- a/tree/dataframe/src/RLoopManager.cxx +++ b/tree/dataframe/src/RLoopManager.cxx @@ -1378,10 +1378,14 @@ void ROOT::Detail::RDF::RLoopManager::RNTupleThreadTask(const std::pairGetLabel(), begin - columnReaderOffset, end - columnReaderOffset, slot}); try { - for (auto entry = begin; entry < end; ++entry) { - if (fDataSource->SetEntry(slot, entry)) { - RunAndCheckFilters(slot, entry); + auto bulkBeginEntry = begin; + std::size_t bulkSize = std::min(GetCurrentBulkSize(), static_cast(end - bulkBeginEntry)); + while (bulkBeginEntry < end) { + if (fDataSource->SetEntry(slot, bulkBeginEntry)) { + RunAndCheckFilters(slot, bulkBeginEntry, bulkSize); } + bulkBeginEntry += bulkSize; + bulkSize = std::min(GetCurrentBulkSize(), static_cast(end - bulkBeginEntry)); } } catch (...) { std::cerr << "RDataFrame::Run: event loop was interrupted\n"; diff --git a/tree/dataframe/src/RTreeColumnReader.cxx b/tree/dataframe/src/RTreeColumnReader.cxx index 4a4b436279a96..d7bce64df274e 100644 --- a/tree/dataframe/src/RTreeColumnReader.cxx +++ b/tree/dataframe/src/RTreeColumnReader.cxx @@ -103,48 +103,45 @@ ROOT::Internal::RDF::RTreeUntypedValueColumnReader::~RTreeUntypedValueColumnRead fObjDeleter(addr); }; -void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadStdArray(Long64_t entry) +void ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadStdArray(std::size_t entryInBulk) { - if (entry == fLastEntry) - return fRVec.data(); // We return the RVec we already created - auto &readerArray = *fTreeArray; // GetSize is called here to trigger actual reading of the branch proxy, which also sets the appropriate read status const auto readerArraySize = readerArray.GetSize(); // The reader could not read an array, signal this back to the node requesting the value - if (R__unlikely(readerArray.GetReadStatus() == ROOT::Internal::TTreeReaderValueBase::EReadStatus::kReadError)) - return nullptr; + if (R__unlikely(readerArray.GetReadStatus() == ROOT::Internal::TTreeReaderValueBase::EReadStatus::kReadError)) { + fCachedValuesInvalidIndices.push_back(entryInBulk); + return; + } // std::array storage should always be contiguous assert(readerArray.IsContiguous() && "std::array storage should always be contiguous"); + auto &cachedValues = std::get<0>(fCachedValues); + if (readerArraySize > 0) { // trigger loading of the contents of the TTreeReaderArray // the address of the first element in the reader array is not necessarily equal to // the address returned by the GetAddress method - RVec rvec(readerArray.At(0), readerArraySize); - swap(fRVec, rvec); - } else { - fRVec.clear(); + RVec rvec(readerArray.At(0), readerArraySize); + swap(cachedValues[entryInBulk], rvec); } - - fLastEntry = entry; - return fRVec.data(); } -void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadStdVector(Long64_t entry) +void ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadStdVector(std::size_t entryInBulk) { - if (entry == fLastEntry) - return &fStdVector; // We return the std::vector we already created - auto &readerArray = *fTreeArray; // GetSize is called here to trigger actual reading of the branch proxy, which also sets the appropriate read status const auto readerArraySize = readerArray.GetSize(); // The reader could not read an array, signal this back to the node requesting the value - if (R__unlikely(readerArray.GetReadStatus() == ROOT::Internal::TTreeReaderValueBase::EReadStatus::kReadError)) - return nullptr; + if (R__unlikely(readerArray.GetReadStatus() == ROOT::Internal::TTreeReaderValueBase::EReadStatus::kReadError)) { + fCachedValuesInvalidIndices.push_back(entryInBulk); + return; + } + + auto &cachedValues = std::get<1>(fCachedValues); // There is no zero-copy constructor for std::vector, so we need to copy the data in any case. if (readerArraySize > 0) { @@ -153,42 +150,37 @@ void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadStdVector(Long64_t fValueSize = readerArray.GetValueSize(); assert(fValueSize > 0 && "Could not retrieve size of collection value type."); // Array is not contiguous, make a full copy of it. - fStdVector.clear(); - fStdVector.reserve(readerArraySize * fValueSize); + auto &dst = cachedValues[entryInBulk]; + dst.reserve(readerArraySize * fValueSize); for (std::size_t i{0}; i < readerArraySize; i++) { auto val = readerArray.At(i); - std::copy(val, val + fValueSize, std::back_inserter(fStdVector)); + std::copy(val, val + fValueSize, std::back_inserter(dst)); } - } else { - fStdVector.clear(); } - - fLastEntry = entry; - return &fStdVector; } -void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadRVec(Long64_t entry) +void ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadRVec(std::size_t entryInBulk) { - if (entry == fLastEntry) - return &fRVec; // We return the RVec we already created auto &readerArray = *fTreeArray; // GetSize is called here to trigger actual reading of the branch proxy, which also sets the appropriate read status const auto readerArraySize = readerArray.GetSize(); // The reader could not read an array, signal this back to the node requesting the value - if (R__unlikely(readerArray.GetReadStatus() == ROOT::Internal::TTreeReaderValueBase::EReadStatus::kReadError)) - return nullptr; + if (R__unlikely(readerArray.GetReadStatus() == ROOT::Internal::TTreeReaderValueBase::EReadStatus::kReadError)) { + fCachedValuesInvalidIndices.push_back(entryInBulk); + return; + } + + auto &cachedValues = std::get<0>(fCachedValues); if (readerArray.IsContiguous() && !(fCollectionType == ECollectionType::kRVecBool)) { if (readerArraySize > 0) { // trigger loading of the contents of the TTreeReaderArray // the address of the first element in the reader array is not necessarily equal to // the address returned by the GetAddress method - RVec rvec(readerArray.At(0), readerArraySize); - swap(fRVec, rvec); - } else { - fRVec.clear(); + RVec rvec(readerArray.At(0), readerArraySize); + swap(cachedValues[entryInBulk], rvec); } } else { // The storage is not contiguous: we cannot but copy into the RVec @@ -209,45 +201,74 @@ void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadRVec(Long64_t entr fValueSize = readerArray.GetValueSize(); assert(fValueSize > 0 && "Could not retrieve size of collection value type."); // Array is not contiguous, make a full copy of it. - fRVec.clear(); - fRVec.reserve(readerArraySize * fValueSize); + auto &dst = cachedValues[entryInBulk]; + dst.reserve(readerArraySize * fValueSize); for (std::size_t i{0}; i < readerArraySize; i++) { auto val = readerArray.At(i); - std::copy(val, val + fValueSize, std::back_inserter(fRVec)); + std::copy(val, val + fValueSize, std::back_inserter(dst)); } // RVec's `size()` method returns the value of the `fSize` data member, unlike std::vector's `size()` // which returns the distance between begin and end divided by the size of the collection value type. // This difference is important in this case: we reserved enough space in the RVec to fill // `readerArraySize * fValueSize` bytes, but the reader will need to read just `readerArraySize` elements // adjusted to the correct `fValueSize` bytes per element. Thus, we set the size of the RVec here to - // represent the correct size of the user-requested RVec. This leaves the RVec in an invalid + // represent the correct size of the user-requested RVec. This leaves the RVec in an invalid // state until it is cast to the correct type (by the `TryGet` call). - fRVec.set_size(readerArraySize); - } else { - fRVec.clear(); + dst.set_size(readerArraySize); } } - - fLastEntry = entry; - return &fRVec; } void ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::LoadImpl(const ROOT::Internal::RDF::RMaskedEntryRange &mask) { + + if (fLastEntry == mask.GetFirstEntry()) + return; + + // Assume 1-size bulk for now + const auto validIndices = mask.GetValidIndices(); + fLastEntry = mask.GetFirstEntry(); + if (validIndices.empty()) + return; + + fCachedValuesInvalidIndices.clear(); + fCachedValuesInvalidIndices.reserve(validIndices.size()); + + std::visit( + [](auto &&visited) { + for (auto &coll : visited) + coll.clear(); + }, + fCachedValues); + // Assume 1-size bulk for now - if (mask[0]) { - if (fCollectionType == ECollectionType::kStdArray) - fValuePtr = LoadStdArray(mask.GetFirstEntry()); - else if (fCollectionType == ECollectionType::kStdVector) - fValuePtr = LoadStdVector(mask.GetFirstEntry()); - else - fValuePtr = LoadRVec(mask.GetFirstEntry()); + for (auto idx : validIndices) { + switch (fCollectionType) { + case ECollectionType::kStdArray: LoadStdArray(idx); break; + case ECollectionType::kRVec: + case ECollectionType::kRVecBool: LoadRVec(idx); break; + case ECollectionType::kStdVector: LoadStdVector(idx); break; + default: throw std::runtime_error("RDataFrame: unrecognized value type to read a collection in a bulk."); + } } } -void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::GetImpl(std::size_t) +void *ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::GetImpl(std::size_t entryInBulk) { - return fValuePtr; + if (fCachedValuesInvalidIndices.end() != + std::find(fCachedValuesInvalidIndices.begin(), fCachedValuesInvalidIndices.end(), entryInBulk)) { + return nullptr; + } + + switch (fCollectionType) { + case ECollectionType::kStdArray: { + return std::get<0>(fCachedValues)[entryInBulk].data(); + } + case ECollectionType::kRVec: + case ECollectionType::kRVecBool: return &std::get<0>(fCachedValues)[entryInBulk]; + case ECollectionType::kStdVector: return &std::get<1>(fCachedValues)[entryInBulk]; + default: throw std::runtime_error("RDataFrame: unrecognized value type to read a collection in a bulk."); + } } ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::RTreeUntypedArrayColumnReader(TTreeReader &r, @@ -257,6 +278,11 @@ ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::RTreeUntypedArrayColumnReade : fTreeArray(std::make_unique(r, colName, valueTypeName)), fCollectionType(collType) { + // Assume 1-size bulk for now + if (fCollectionType == ECollectionType::kStdVector) + fCachedValues = std::vector>(1); + else + fCachedValues = std::vector>(1); } ROOT::Internal::RDF::RTreeUntypedArrayColumnReader::~RTreeUntypedArrayColumnReader() = default;