From 1b9b15b1ca5b0c3da45cf09c16f0cf7cc9658722 Mon Sep 17 00:00:00 2001
From: Enrico Guiraud <enrico.guiraud@cern.ch>
Date: Mon, 18 Feb 2019 22:45:43 +0100
Subject: [PATCH] [DF] Prefer passing RBookedCustomColumns by rvalue-reference
---
.../dataframe/inc/ROOT/RDF/InterfaceUtils.hxx | 71 +++++++++----------
tree/dataframe/inc/ROOT/RDF/RAction.hxx | 16 ++---
tree/dataframe/inc/ROOT/RDF/RActionBase.hxx | 2 +-
tree/dataframe/inc/ROOT/RDF/RInterface.hxx | 30 ++++----
tree/dataframe/src/RActionBase.cxx | 2 +-
tree/dataframe/src/RDFInterfaceUtils.cxx | 2 +-
6 files changed, 63 insertions(+), 60 deletions(-)
diff --git a/tree/dataframe/inc/ROOT/RDF/InterfaceUtils.hxx b/tree/dataframe/inc/ROOT/RDF/InterfaceUtils.hxx
index 4a971041ef8..30e76afa798 100644
--- a/tree/dataframe/inc/ROOT/RDF/InterfaceUtils.hxx
+++ b/tree/dataframe/inc/ROOT/RDF/InterfaceUtils.hxx
@@ -87,10 +87,8 @@ HeadNode_t CreateSnaphotRDF(const ColumnNames_t &validCols,
std::string DemangleTypeIdName(const std::type_info &typeInfo);
-ColumnNames_t ConvertRegexToColumns(RDFInternal::RBookedCustomColumns & customColumns,
- TTree *tree,
- ROOT::RDF::RDataSource *dataSource,
- std::string_view columnNameRegexp,
+ColumnNames_t ConvertRegexToColumns(const RDFInternal::RBookedCustomColumns &customColumns, TTree *tree,
+ ROOT::RDF::RDataSource *dataSource, std::string_view columnNameRegexp,
std::string_view callerName);
/// An helper object that sets and resets gErrorIgnoreLevel via RAII.
@@ -157,106 +155,106 @@ struct HistoUtils<T, true> {
template <typename... BranchTypes, typename ActionTag, typename ActionResultType, typename PrevNodeType>
std::unique_ptr<RActionBase>
BuildAction(const ColumnNames_t &bl, const std::shared_ptr<ActionResultType> &h, const unsigned int nSlots,
- std::shared_ptr<PrevNodeType> prevNode, ActionTag, RDFInternal::RBookedCustomColumns customColumns)
+ std::shared_ptr<PrevNodeType> prevNode, ActionTag, RDFInternal::RBookedCustomColumns &&customColumns)
{
using Helper_t = FillParHelper<ActionResultType>;
using Action_t = RAction<Helper_t, PrevNodeType, TTraits::TypeList<BranchTypes...>>;
- return std::make_unique<Action_t>(Helper_t(h, nSlots), bl, std::move(prevNode), customColumns);
+ return std::make_unique<Action_t>(Helper_t(h, nSlots), bl, std::move(prevNode), std::move(customColumns));
}
// Histo1D filling (must handle the special case of distinguishing FillParHelper and FillHelper
template <typename... BranchTypes, typename PrevNodeType>
std::unique_ptr<RActionBase> BuildAction(const ColumnNames_t &bl, const std::shared_ptr<::TH1D> &h,
const unsigned int nSlots, std::shared_ptr<PrevNodeType> prevNode,
- ActionTags::Histo1D, RDFInternal::RBookedCustomColumns customColumns)
+ ActionTags::Histo1D, RDFInternal::RBookedCustomColumns &&customColumns)
{
auto hasAxisLimits = HistoUtils<::TH1D>::HasAxisLimits(*h);
if (hasAxisLimits) {
using Helper_t = FillParHelper<::TH1D>;
using Action_t = RAction<Helper_t, PrevNodeType, TTraits::TypeList<BranchTypes...>>;
- return std::make_unique<Action_t>(Helper_t(h, nSlots), bl, std::move(prevNode), customColumns);
+ return std::make_unique<Action_t>(Helper_t(h, nSlots), bl, std::move(prevNode), std::move(customColumns));
} else {
using Helper_t = FillHelper;
using Action_t = RAction<Helper_t, PrevNodeType, TTraits::TypeList<BranchTypes...>>;
- return std::make_unique<Action_t>(Helper_t(h, nSlots), bl, std::move(prevNode), customColumns);
+ return std::make_unique<Action_t>(Helper_t(h, nSlots), bl, std::move(prevNode), std::move(customColumns));
}
}
template <typename... BranchTypes, typename PrevNodeType>
-std::unique_ptr<RActionBase>
-BuildAction(const ColumnNames_t &bl, const std::shared_ptr<TGraph> &g, const unsigned int nSlots,
- std::shared_ptr<PrevNodeType> prevNode, ActionTags::Graph, RDFInternal::RBookedCustomColumns customColumns)
+std::unique_ptr<RActionBase> BuildAction(const ColumnNames_t &bl, const std::shared_ptr<TGraph> &g,
+ const unsigned int nSlots, std::shared_ptr<PrevNodeType> prevNode,
+ ActionTags::Graph, RDFInternal::RBookedCustomColumns &&customColumns)
{
using Helper_t = FillTGraphHelper;
using Action_t = RAction<Helper_t, PrevNodeType, TTraits::TypeList<BranchTypes...>>;
- return std::make_unique<Action_t>(Helper_t(g, nSlots), bl, std::move(prevNode), customColumns);
+ return std::make_unique<Action_t>(Helper_t(g, nSlots), bl, std::move(prevNode), std::move(customColumns));
}
// Min action
template <typename BranchType, typename PrevNodeType, typename ActionResultType>
-std::unique_ptr<RActionBase>
-BuildAction(const ColumnNames_t &bl, const std::shared_ptr<ActionResultType> &minV, const unsigned int nSlots,
- std::shared_ptr<PrevNodeType> prevNode, ActionTags::Min, RDFInternal::RBookedCustomColumns customColumns)
+std::unique_ptr<RActionBase> BuildAction(const ColumnNames_t &bl, const std::shared_ptr<ActionResultType> &minV,
+ const unsigned int nSlots, std::shared_ptr<PrevNodeType> prevNode,
+ ActionTags::Min, RDFInternal::RBookedCustomColumns &&customColumns)
{
using Helper_t = MinHelper<ActionResultType>;
using Action_t = RAction<Helper_t, PrevNodeType, TTraits::TypeList<BranchType>>;
- return std::make_unique<Action_t>(Helper_t(minV, nSlots), bl, std::move(prevNode), customColumns);
+ return std::make_unique<Action_t>(Helper_t(minV, nSlots), bl, std::move(prevNode), std::move(customColumns));
}
// Max action
template <typename BranchType, typename PrevNodeType, typename ActionResultType>
-std::unique_ptr<RActionBase>
-BuildAction(const ColumnNames_t &bl, const std::shared_ptr<ActionResultType> &maxV, const unsigned int nSlots,
- std::shared_ptr<PrevNodeType> prevNode, ActionTags::Max, RDFInternal::RBookedCustomColumns customColumns)
+std::unique_ptr<RActionBase> BuildAction(const ColumnNames_t &bl, const std::shared_ptr<ActionResultType> &maxV,
+ const unsigned int nSlots, std::shared_ptr<PrevNodeType> prevNode,
+ ActionTags::Max, RDFInternal::RBookedCustomColumns &&customColumns)
{
using Helper_t = MaxHelper<ActionResultType>;
using Action_t = RAction<Helper_t, PrevNodeType, TTraits::TypeList<BranchType>>;
- return std::make_unique<Action_t>(Helper_t(maxV, nSlots), bl, std::move(prevNode), customColumns);
+ return std::make_unique<Action_t>(Helper_t(maxV, nSlots), bl, std::move(prevNode), std::move(customColumns));
}
// Sum action
template <typename BranchType, typename PrevNodeType, typename ActionResultType>
-std::unique_ptr<RActionBase>
-BuildAction(const ColumnNames_t &bl, const std::shared_ptr<ActionResultType> &sumV, const unsigned int nSlots,
- std::shared_ptr<PrevNodeType> prevNode, ActionTags::Sum, RDFInternal::RBookedCustomColumns customColumns)
+std::unique_ptr<RActionBase> BuildAction(const ColumnNames_t &bl, const std::shared_ptr<ActionResultType> &sumV,
+ const unsigned int nSlots, std::shared_ptr<PrevNodeType> prevNode,
+ ActionTags::Sum, RDFInternal::RBookedCustomColumns &&customColumns)
{
using Helper_t = SumHelper<ActionResultType>;
using Action_t = RAction<Helper_t, PrevNodeType, TTraits::TypeList<BranchType>>;
- return std::make_unique<Action_t>(Helper_t(sumV, nSlots), bl, std::move(prevNode), customColumns);
+ return std::make_unique<Action_t>(Helper_t(sumV, nSlots), bl, std::move(prevNode), std::move(customColumns));
}
// Mean action
template <typename BranchType, typename PrevNodeType>
-std::unique_ptr<RActionBase>
-BuildAction(const ColumnNames_t &bl, const std::shared_ptr<double> &meanV, const unsigned int nSlots,
- std::shared_ptr<PrevNodeType> prevNode, ActionTags::Mean, RDFInternal::RBookedCustomColumns customColumns)
+std::unique_ptr<RActionBase> BuildAction(const ColumnNames_t &bl, const std::shared_ptr<double> &meanV,
+ const unsigned int nSlots, std::shared_ptr<PrevNodeType> prevNode,
+ ActionTags::Mean, RDFInternal::RBookedCustomColumns &&customColumns)
{
using Helper_t = MeanHelper;
using Action_t = RAction<Helper_t, PrevNodeType, TTraits::TypeList<BranchType>>;
- return std::make_unique<Action_t>(Helper_t(meanV, nSlots), bl, std::move(prevNode), customColumns);
+ return std::make_unique<Action_t>(Helper_t(meanV, nSlots), bl, std::move(prevNode), std::move(customColumns));
}
// Standard Deviation action
template <typename BranchType, typename PrevNodeType>
-std::unique_ptr<RActionBase>
-BuildAction(const ColumnNames_t &bl, const std::shared_ptr<double> &stdDeviationV, const unsigned int nSlots,
- std::shared_ptr<PrevNodeType> prevNode, ActionTags::StdDev, RDFInternal::RBookedCustomColumns customColumns)
+std::unique_ptr<RActionBase> BuildAction(const ColumnNames_t &bl, const std::shared_ptr<double> &stdDeviationV,
+ const unsigned int nSlots, std::shared_ptr<PrevNodeType> prevNode,
+ ActionTags::StdDev, RDFInternal::RBookedCustomColumns &&customColumns)
{
using Helper_t = StdDevHelper;
using Action_t = RAction<Helper_t, PrevNodeType, TTraits::TypeList<BranchType>>;
- return std::make_unique<Action_t>(Helper_t(stdDeviationV, nSlots), bl, prevNode, customColumns);
+ return std::make_unique<Action_t>(Helper_t(stdDeviationV, nSlots), bl, prevNode, std::move(customColumns));
}
// Display action
template <typename... BranchTypes, typename PrevNodeType>
std::unique_ptr<RActionBase> BuildAction(const ColumnNames_t &bl, const std::shared_ptr<RDisplay> &d,
const unsigned int, std::shared_ptr<PrevNodeType> prevNode,
- ActionTags::Display, RDFInternal::RBookedCustomColumns customColumns)
+ ActionTags::Display, RDFInternal::RBookedCustomColumns &&customColumns)
{
using Helper_t = DisplayHelper<PrevNodeType>;
using Action_t = RAction<Helper_t, PrevNodeType, TTraits::TypeList<BranchTypes...>>;
- return std::make_unique<Action_t>(Helper_t(d, prevNode), bl, prevNode, customColumns);
+ return std::make_unique<Action_t>(Helper_t(d, prevNode), bl, prevNode, std::move(customColumns));
}
/****** end BuildAndBook ******/
@@ -420,7 +418,8 @@ void CallBuildAction(std::shared_ptr<PrevNodeType> *prevNodeOnHeap, const Column
std::make_index_sequence<nColumns>(), ColTypes_t())
: *customColumns;
- auto actionPtr = BuildAction<BranchTypes...>(bl, *rOnHeap, nSlots, std::move(prevNodePtr), ActionTag{}, newColumns);
+ auto actionPtr =
+ BuildAction<BranchTypes...>(bl, *rOnHeap, nSlots, std::move(prevNodePtr), ActionTag{}, std::move(newColumns));
(*jittedActionOnHeap)->SetAction(std::move(actionPtr));
// customColumns points to the columns structure in the heap, created before the jitted call so that the jitter can
diff --git a/tree/dataframe/inc/ROOT/RDF/RAction.hxx b/tree/dataframe/inc/ROOT/RDF/RAction.hxx
index a4bc04cf7a8..22d923e4ab1 100644
--- a/tree/dataframe/inc/ROOT/RDF/RAction.hxx
+++ b/tree/dataframe/inc/ROOT/RDF/RAction.hxx
@@ -112,8 +112,8 @@ public:
using TypeInd_t = std::make_index_sequence<ColumnTypes_t::list_size>;
RActionCRTP(Helper &&h, const ColumnNames_t &bl, std::shared_ptr<PrevDataFrame> pd,
- const RBookedCustomColumns &customColumns)
- : RActionBase(pd->GetLoopManagerUnchecked(), bl, customColumns), fHelper(std::forward<Helper>(h)),
+ RBookedCustomColumns &&customColumns)
+ : RActionBase(pd->GetLoopManagerUnchecked(), bl, std::move(customColumns)), fHelper(std::forward<Helper>(h)),
fPrevDataPtr(std::move(pd)), fPrevData(*fPrevDataPtr) { }
RActionCRTP(const RActionCRTP &) = delete;
@@ -213,8 +213,8 @@ public:
using ActionCRTP_t = RActionCRTP<RAction<Helper, PrevDataFrame, ColumnTypes_t>>;
RAction(Helper &&h, const ColumnNames_t &bl, std::shared_ptr<PrevDataFrame> pd,
- const RBookedCustomColumns &customColumns)
- : ActionCRTP_t(std::forward<Helper>(h), bl, std::move(pd), customColumns), fValues(GetNSlots()) { }
+ RBookedCustomColumns &&customColumns)
+ : ActionCRTP_t(std::forward<Helper>(h), bl, std::move(pd), std::move(customColumns)), fValues(GetNSlots()) { }
void InitColumnValues(TTreeReader *r, unsigned int slot)
{
@@ -261,8 +261,8 @@ class RAction<SnapshotHelper<ColTypes...>, PrevDataFrame, ROOT::TypeTraits::Type
public:
RAction(SnapshotHelper<ColTypes...> &&h, const ColumnNames_t &bl, std::shared_ptr<PrevDataFrame> pd,
- const RBookedCustomColumns &customColumns)
- : ActionCRTP_t(std::forward<SnapshotHelper<ColTypes...>>(h), bl, std::move(pd), customColumns),
+ RBookedCustomColumns &&customColumns)
+ : ActionCRTP_t(std::forward<SnapshotHelper<ColTypes...>>(h), bl, std::move(pd), std::move(customColumns)),
fValues(GetNSlots())
{
}
@@ -301,8 +301,8 @@ class RAction<SnapshotHelperMT<ColTypes...>, PrevDataFrame, ROOT::TypeTraits::Ty
public:
RAction(SnapshotHelperMT<ColTypes...> &&h, const ColumnNames_t &bl, std::shared_ptr<PrevDataFrame> pd,
- const RBookedCustomColumns &customColumns)
- : ActionCRTP_t(std::forward<SnapshotHelperMT<ColTypes...>>(h), bl, std::move(pd), customColumns),
+ RBookedCustomColumns &&customColumns)
+ : ActionCRTP_t(std::forward<SnapshotHelperMT<ColTypes...>>(h), bl, std::move(pd), std::move(customColumns)),
fValues(GetNSlots())
{
}
diff --git a/tree/dataframe/inc/ROOT/RDF/RActionBase.hxx b/tree/dataframe/inc/ROOT/RDF/RActionBase.hxx
index ed5217ef5a7..232efbce1e9 100644
--- a/tree/dataframe/inc/ROOT/RDF/RActionBase.hxx
+++ b/tree/dataframe/inc/ROOT/RDF/RActionBase.hxx
@@ -55,7 +55,7 @@ private:
RBookedCustomColumns fCustomColumns;
public:
- RActionBase(RLoopManager *lm, const ColumnNames_t &colNames, const RBookedCustomColumns &customColumns);
+ RActionBase(RLoopManager *lm, const ColumnNames_t &colNames, RBookedCustomColumns &&customColumns);
RActionBase(const RActionBase &) = delete;
RActionBase &operator=(const RActionBase &) = delete;
virtual ~RActionBase();
diff --git a/tree/dataframe/inc/ROOT/RDF/RInterface.hxx b/tree/dataframe/inc/ROOT/RDF/RInterface.hxx
index 3f376006d09..b9226249bf8 100644
--- a/tree/dataframe/inc/ROOT/RDF/RInterface.hxx
+++ b/tree/dataframe/inc/ROOT/RDF/RInterface.hxx
@@ -764,7 +764,8 @@ public:
using Helper_t = RDFInternal::ForeachSlotHelper<F>;
using Action_t = RDFInternal::RAction<Helper_t, Proxied>;
- auto action = std::make_unique<Action_t>(Helper_t(std::move(f)), validColumnNames, fProxiedPtr, newColumns);
+ auto action =
+ std::make_unique<Action_t>(Helper_t(std::move(f)), validColumnNames, fProxiedPtr, std::move(newColumns));
fLoopManager->Book(action.get());
fLoopManager->Run();
@@ -833,7 +834,8 @@ public:
auto cSPtr = std::make_shared<ULong64_t>(0);
using Helper_t = RDFInternal::CountHelper;
using Action_t = RDFInternal::RAction<Helper_t, Proxied>;
- auto action = std::make_unique<Action_t>(Helper_t(cSPtr, nSlots), ColumnNames_t({}), fProxiedPtr, fCustomColumns);
+ auto action =
+ std::make_unique<Action_t>(Helper_t(cSPtr, nSlots), ColumnNames_t({}), fProxiedPtr, std::move(fCustomColumns));
fLoopManager->Book(action.get());
return MakeResultPtr(cSPtr, *fLoopManager, std::move(action));
}
@@ -872,7 +874,8 @@ public:
auto valuesPtr = std::make_shared<COLL>();
const auto nSlots = fLoopManager->GetNSlots();
- auto action = std::make_unique<Action_t>(Helper_t(valuesPtr, nSlots), validColumnNames, fProxiedPtr, newColumns);
+ auto action =
+ std::make_unique<Action_t>(Helper_t(valuesPtr, nSlots), validColumnNames, fProxiedPtr, std::move(newColumns));
fLoopManager->Book(action.get());
return MakeResultPtr(valuesPtr, *fLoopManager, std::move(action));
}
@@ -1514,7 +1517,7 @@ public:
using Action_t = RDFInternal::RAction<Helper_t, Proxied>;
auto action = std::make_unique<Action_t>(Helper_t(rep, fProxiedPtr, returnEmptyReport), ColumnNames_t({}),
- fProxiedPtr, fCustomColumns);
+ fProxiedPtr, RDFInternal::RBookedCustomColumns(fCustomColumns));
fLoopManager->Book(action.get());
return MakeResultPtr(rep, *fLoopManager, std::move(action));
@@ -1694,7 +1697,7 @@ public:
using Action_t = typename RDFInternal::RAction<Helper_t, Proxied>;
auto action = std::make_unique<Action_t>(
Helper_t(std::move(aggregator), std::move(merger), accObjPtr, fLoopManager->GetNSlots()), validColumnNames,
- fProxiedPtr, newColumns);
+ fProxiedPtr, std::move(newColumns));
fLoopManager->Book(action.get());
return MakeResultPtr(accObjPtr, *fLoopManager, std::move(action));
}
@@ -1774,7 +1777,7 @@ public:
auto resPtr = helper.GetResultPtr();
auto action = std::make_unique<Action_t>(Helper(std::forward<Helper>(helper)), validColumnNames, fProxiedPtr,
- fCustomColumns);
+ RDFInternal::RBookedCustomColumns(fCustomColumns));
fLoopManager->Book(action.get());
return MakeResultPtr(resPtr, *fLoopManager, std::move(action));
}
@@ -1928,8 +1931,8 @@ private:
const auto nSlots = fLoopManager->GetNSlots();
- auto action =
- RDFInternal::BuildAction<BranchTypes...>(validColumnNames, r, nSlots, fProxiedPtr, ActionTag{}, newColumns);
+ auto action = RDFInternal::BuildAction<BranchTypes...>(validColumnNames, r, nSlots, fProxiedPtr, ActionTag{},
+ std::move(newColumns));
fLoopManager->Book(action.get());
return MakeResultPtr(r, *fLoopManager, std::move(action));
}
@@ -2068,19 +2071,20 @@ private:
using Helper_t = RDFInternal::SnapshotHelper<ColumnTypes...>;
using Action_t = RDFInternal::RAction<Helper_t, Proxied>;
actionPtr.reset(new Action_t(Helper_t(filename, dirname, treename, validCols, columnList, options), validCols,
- fProxiedPtr, newColumns));
+ fProxiedPtr, std::move(newColumns)));
} else {
// multi-thread snapshot
using Helper_t = RDFInternal::SnapshotHelperMT<ColumnTypes...>;
using Action_t = RDFInternal::RAction<Helper_t, Proxied>;
actionPtr.reset(new Action_t(
Helper_t(fLoopManager->GetNSlots(), filename, dirname, treename, validCols, columnList, options), validCols,
- fProxiedPtr, newColumns));
+ fProxiedPtr, std::move(newColumns)));
}
fLoopManager->Book(actionPtr.get());
- return RDFInternal::CreateSnaphotRDF(validCols, fullTreename, filename, options.fLazy, *fLoopManager, std::move(actionPtr));
+ return RDFInternal::CreateSnaphotRDF(validCols, fullTreename, filename, options.fLazy, *fLoopManager,
+ std::move(actionPtr));
}
////////////////////////////////////////////////////////////////////////////
@@ -2108,8 +2112,8 @@ private:
}
protected:
- RInterface(const std::shared_ptr<Proxied> &proxied, RLoopManager &lm, RDFInternal::RBookedCustomColumns columns,
- RDataSource *ds)
+ RInterface(const std::shared_ptr<Proxied> &proxied, RLoopManager &lm,
+ const RDFInternal::RBookedCustomColumns &columns, RDataSource *ds)
: fProxiedPtr(proxied), fLoopManager(&lm), fDataSource(ds), fCustomColumns(columns)
{
}
diff --git a/tree/dataframe/src/RActionBase.cxx b/tree/dataframe/src/RActionBase.cxx
index 9af61f7e3c3..1d4607d3f65 100644
--- a/tree/dataframe/src/RActionBase.cxx
+++ b/tree/dataframe/src/RActionBase.cxx
@@ -13,7 +13,7 @@
using namespace ROOT::Internal::RDF;
-RActionBase::RActionBase(RLoopManager *lm, const ColumnNames_t &colNames, const RBookedCustomColumns &customColumns)
+RActionBase::RActionBase(RLoopManager *lm, const ColumnNames_t &colNames, RBookedCustomColumns &&customColumns)
: fLoopManager(lm), fNSlots(lm->GetNSlots()), fColumnNames(colNames), fCustomColumns(customColumns) { }
// outlined to pin virtual table
diff --git a/tree/dataframe/src/RDFInterfaceUtils.cxx b/tree/dataframe/src/RDFInterfaceUtils.cxx
index ddb6458b135..481bb22c73c 100644
--- a/tree/dataframe/src/RDFInterfaceUtils.cxx
+++ b/tree/dataframe/src/RDFInterfaceUtils.cxx
@@ -154,7 +154,7 @@ std::string DemangleTypeIdName(const std::type_info &typeInfo)
return TClassEdit::DemangleTypeIdName(typeInfo, dummy);
}
-ColumnNames_t ConvertRegexToColumns(RDFInternal::RBookedCustomColumns & customColumns,
+ColumnNames_t ConvertRegexToColumns(const RDFInternal::RBookedCustomColumns & customColumns,
TTree *tree,
ROOT::RDF::RDataSource *dataSource,
std::string_view columnNameRegexp,
--
GitLab