From 4928916dcec39fea4fb83fd85c7ba1cbb7746ba8 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Sun, 17 May 2026 12:05:36 +0200 Subject: [PATCH 1/5] [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 0302113345774..6568f3b3d1ef2 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 82c72ee81102e..0e4ed6ef0e1df 100644 --- a/tree/dataframe/src/RNTupleDS.cxx +++ b/tree/dataframe/src/RNTupleDS.cxx @@ -270,13 +270,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 33ab00ab0100791e9e34fdf08d1ea856ee5222fa Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Tue, 2 Jun 2026 11:37:15 +0200 Subject: [PATCH 2/5] [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 01ddbc3e3774e..abb05a3714f98 100644 --- a/tree/dataframe/CMakeLists.txt +++ b/tree/dataframe/CMakeLists.txt @@ -82,6 +82,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 6568f3b3d1ef2..ec2513d30fb9f 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 6b6af72f84331..1c9511b40e344 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 3764a447627ca..70ddb08191598 100644 --- a/tree/dataframe/src/RDFSnapshotHelpers.cxx +++ b/tree/dataframe/src/RDFSnapshotHelpers.cxx @@ -1222,9 +1222,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++) { @@ -1234,14 +1236,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 67e1e9e4157b4..dd98a642014e2 100644 --- a/tree/dataframe/src/RLoopManager.cxx +++ b/tree/dataframe/src/RLoopManager.cxx @@ -1058,9 +1058,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 0e4ed6ef0e1df..517737eee1a01 100644 --- a/tree/dataframe/src/RNTupleDS.cxx +++ b/tree/dataframe/src/RNTupleDS.cxx @@ -204,7 +204,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. @@ -217,7 +216,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; @@ -267,16 +265,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 b503ca01ff6a26c3d12ce8117db5a5721a395a36 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Wed, 3 Jun 2026 17:30:13 +0200 Subject: [PATCH 3/5] [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 1c9511b40e344..b66a279866f25 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 @@ -336,6 +339,8 @@ public: /// The task run by every thread on an entry range (known by the input TTreeReader), for the TTree data source. void TTreeThreadTask(TTreeReader &treeReader, ROOT::Internal::RSlotStack &slotStack, std::atomic &entryCount); + + 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 dd98a642014e2..55d69612d2fb2 100644 --- a/tree/dataframe/src/RLoopManager.cxx +++ b/tree/dataframe/src/RLoopManager.cxx @@ -535,8 +535,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 @@ -560,9 +564,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"; @@ -656,11 +664,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 (...) { @@ -709,7 +721,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)) { @@ -719,9 +731,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); } @@ -1058,10 +1070,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 @@ -1359,10 +1371,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"; @@ -1398,7 +1414,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 ffded4e6279dae517522b4970cbb125b1aab29f6 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Wed, 3 Jun 2026 13:22:17 +0200 Subject: [PATCH 4/5] [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 ec2513d30fb9f..1b1a432fec2d7 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 cda277f5855816d86493b72241bcad9aed4803e0 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Fri, 5 Jun 2026 10:08:37 +0200 Subject: [PATCH 5/5] [df] Prepare for reading TTreeReaderValue in bulks This is a partial implementation, serving as a starting point. It only considers the case of reading from TTreeReaderValue. It also highlights already a few pre-existing features which may not be supported anymore: * Reading polymorphic TBranch with different concrete types stored across different events * Reading polymorphic TBranch of concrete types read as their base type --- .../inc/ROOT/RDF/RColumnReaderBase.hxx | 6 +-- .../inc/ROOT/RDF/RMaskedEntryRange.hxx | 9 ++++ .../inc/ROOT/RDF/RTreeColumnReader.hxx | 5 ++- tree/dataframe/src/RTreeColumnReader.cxx | 42 ++++++++++++++++--- tree/dataframe/test/dataframe_nodes.cxx | 7 +++- tree/dataframe/test/dataframe_regression.cxx | 6 +++ tree/treeplayer/inc/TTreeReaderArray.h | 2 +- tree/treeplayer/inc/TTreeReaderValue.h | 2 + tree/treeplayer/src/TTreeReaderValue.cxx | 24 +++++++++++ 9 files changed, 92 insertions(+), 11 deletions(-) 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..5d7cd50ca5ee4 100644 --- a/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx +++ b/tree/dataframe/inc/ROOT/RDF/RTreeColumnReader.hxx @@ -58,7 +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}; + ROOT::RVec fCachedResults{}; + ROOT::RVec fCachedResultsInvalidIndices{}; + std::size_t fValueSize{0}; + std::uint64_t fLastEntry = std::numeric_limits::max(); 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..44c71e82047cc 100644 --- a/tree/dataframe/src/RTreeColumnReader.cxx +++ b/tree/dataframe/src/RTreeColumnReader.cxx @@ -24,16 +24,48 @@ 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; + 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(); + 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(); + fCachedResultsInvalidIndices.reserve(validIndices.size()); + + fCachedResults.clear(); + fCachedResults.reserve(validIndices.size() * fValueSize); + for (auto idx : validIndices) { + // TODO: go back/forth to the correct valid index in the TTreeReaderValue + auto val = reinterpret_cast(fTreeValue->Get()); + if (!val) { + fCachedResultsInvalidIndices.push_back(idx); + } else { + std::copy(val, val + fValueSize, std::back_inserter(fCachedResults)); + } + } } ROOT::Internal::RDF::RTreeUntypedValueColumnReader::RTreeUntypedValueColumnReader(TTreeReader &r, diff --git a/tree/dataframe/test/dataframe_nodes.cxx b/tree/dataframe/test/dataframe_nodes.cxx index aca6529b25350..29fd7fdc4613f 100644 --- a/tree/dataframe/test/dataframe_nodes.cxx +++ b/tree/dataframe/test/dataframe_nodes.cxx @@ -134,7 +134,12 @@ TEST(RDataFrameNodes, DoubleEvtLoop) gSystem->Unlink(f.c_str()); } +/* // ROOT-9736 +DF-BULK-DISABLED: we do not support anymore reading of polymorphic objects +with a base class type when on-disk the object is stored as a derived class type. +This was a regression test for https://its.cern.ch/jira/browse/ROOT-9736 which +was submitted by ATLAS, so we may consider re-adding support for it. TEST(RDataFrameNodes, InheritanceOfDefines) { ROOT::RDataFrame df(1); @@ -157,7 +162,7 @@ TEST(RDataFrameNodes, InheritanceOfDefines) ROOT::RDataFrame(1).Define("x", createStat).Snapshot("t", ofileName, {"x"})->Foreach(checkStat, {"x"}); gSystem->Unlink(ofileName); } - +*/ TEST(RDataFrameNodes, InvalidLoopType) { ROOT::Detail::RDF::RLoopManager lm{}; diff --git a/tree/dataframe/test/dataframe_regression.cxx b/tree/dataframe/test/dataframe_regression.cxx index 8ff3035e5f74c..e1f0d7fd4cf66 100644 --- a/tree/dataframe/test/dataframe_regression.cxx +++ b/tree/dataframe/test/dataframe_regression.cxx @@ -195,6 +195,11 @@ TEST_P(RDFRegressionTests, ReadWriteVector3) gSystem->Unlink(filename.c_str()); } +/* +DF-BULK-DISABLED: we do not support anymore reading of polymorphic objects +of different concrete types from TBranch. Test was originally added by +https://github.com/root-project/root/pull/6087 as a regression test for +https://its.cern.ch/jira/browse/ROOT-10022 TEST_P(RDFRegressionTests, PolymorphicTBranchObject) { const std::string filename = "polymorphictbranchobject.root"; @@ -251,6 +256,7 @@ TEST_P(RDFRegressionTests, PolymorphicTBranchObject) gSystem->Unlink(snap_fname.c_str()); gSystem->Unlink(filename.c_str()); } +*/ // #11222 TEST_P(RDFRegressionTests, UseAfterDeleteOfSampleCallbacks) 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..7030716931681 100644 --- a/tree/treeplayer/inc/TTreeReaderValue.h +++ b/tree/treeplayer/inc/TTreeReaderValue.h @@ -93,6 +93,8 @@ 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; } diff --git a/tree/treeplayer/src/TTreeReaderValue.cxx b/tree/treeplayer/src/TTreeReaderValue.cxx index 01284f75bcfbe..c3a5829cdeaa4 100644 --- a/tree/treeplayer/src/TTreeReaderValue.cxx +++ b/tree/treeplayer/src/TTreeReaderValue.cxx @@ -850,6 +850,30 @@ void ROOT::Internal::TTreeReaderValueBase::ErrorAboutMissingProxyIfNeeded() fBranchName.Data()); } +std::size_t ROOT::Internal::TTreeReaderValueBase::GetValueSize() const +{ + // Try with available dictionary first + 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.