[DF] TSlotStack uses local maps indexed with thread ids rather than thread local

this prevents bogus behaviour if two instances of TSlotStack are used simultaneously
or even one after the other.

Author: dpiparo

tree/dataframe/inc/ROOT/RDFNodes.hxx

@@ -17,7 +17,7 @@
 #include "ROOT/RDFUtils.hxx"
 #include "ROOT/RIntegerSequence.hxx"
 #include "ROOT/RVec.hxx"
-#include "ROOT/TSpinMutex.hxx"
+#include "ROOT/TRWSpinLock.hxx"
 #include "ROOT/TypeTraits.hxx"
 #include "TError.h"
 #include "TTreeReaderArray.h"
@@ -30,6 +30,7 @@
 #include <numeric> // std::accumulate (FillReport), std::iota (TSlotStack)
 #include <stack>
 #include <string>
+#include <thread>
 #include <tuple>
 #include <vector>
 
@@ -38,31 +39,25 @@ namespace Internal {
 namespace RDF {
 class RActionBase;
 
-// This is an helper class to allow to pick a slot without resorting to a map
+// This is an helper class to allow to pick a slot resorting to a map
 // indexed by thread ids.
 // WARNING: this class does not work as a regular stack. The size is
 // fixed at construction time and no blocking is foreseen.
 class TSlotStack {
 private:
-   unsigned int &GetCount()
-   {
-      TTHREAD_TLS(unsigned int) count = 0U;
-      return count;
-   }
-   unsigned int &GetIndex()
-   {
-      TTHREAD_TLS(unsigned int) index = std::numeric_limits<unsigned int>::max();
-      return index;
-   }
+   unsigned int &GetCount();
+   unsigned int &GetIndex();
    unsigned int fCursor;
    std::vector<unsigned int> fBuf;
-   ROOT::TSpinMutex fMutex;
+   ROOT::TRWSpinLock fRWLock;
 
 public:
    TSlotStack() = delete;
    TSlotStack(unsigned int size) : fCursor(size), fBuf(size) { std::iota(fBuf.begin(), fBuf.end(), 0U); }
    void ReturnSlot(unsigned int slotNumber);
    unsigned int GetSlot();
+   std::map<std::thread::id, unsigned int> fCountMap;
+   std::map<std::thread::id, unsigned int> fIndexMap;
 };
 } // ns RDF
 } // ns Internal

tree/dataframe/src/RDFNodes.cxx

@@ -209,6 +209,37 @@ void RJittedFilter::InitNode()
    fConcreteFilter->InitNode();
 }
 
+unsigned int &TSlotStack::GetCount()
+{
+   const auto tid = std::this_thread::get_id();
+   {
+      ROOT::TRWSpinLockReadGuard rg(fRWLock);
+      auto it = fCountMap.find(tid);
+      if (fCountMap.end() != it)
+         return it->second;
+   }
+
+   {
+      ROOT::TRWSpinLockWriteGuard rg(fRWLock);
+      return (fCountMap[tid] = 0U);
+   }
+}
+unsigned int &TSlotStack::GetIndex()
+{
+   const auto tid = std::this_thread::get_id();
+
+   {
+      ROOT::TRWSpinLockReadGuard rg(fRWLock);
+      if (fIndexMap.end() != fIndexMap.find(tid))
+         return fIndexMap[tid];
+   }
+
+   {
+      ROOT::TRWSpinLockWriteGuard rg(fRWLock);
+      return (fIndexMap[tid] = std::numeric_limits<unsigned int>::max());
+   }
+}
+
 void TSlotStack::ReturnSlot(unsigned int slotNumber)
 {
    auto &index = GetIndex();
@@ -217,7 +248,7 @@ void TSlotStack::ReturnSlot(unsigned int slotNumber)
    count--;
    if (0U == count) {
       index = std::numeric_limits<unsigned int>::max();
-      std::lock_guard<ROOT::TSpinMutex> guard(fMutex);
+      ROOT::TRWSpinLockWriteGuard guard(fRWLock);
       fBuf[fCursor++] = slotNumber;
       R__ASSERT(fCursor <= fBuf.size() &&
                 "TSlotStack assumes that at most a fixed number of values can be present in the "
@@ -233,7 +264,7 @@ unsigned int TSlotStack::GetSlot()
    count++;
    if (std::numeric_limits<unsigned int>::max() != index)
       return index;
-   std::lock_guard<ROOT::TSpinMutex> guard(fMutex);
+   ROOT::TRWSpinLockWriteGuard guard(fRWLock);
    R__ASSERT(fCursor > 0 &&
              "TSlotStack assumes that a value can be always obtained. In this case fCursor is <=0 and this "
              "violates such assumption.");