[DF] Remove measures to cope with task interleaving in RSLotStack

tree/dataframe/inc/ROOT/RDF/RSlotStack.hxx

@@ -13,9 +13,7 @@
 
 #include "ROOT/TRWSpinLock.hxx"
 
-#include <map>
-#include <thread>
-#include <vector>
+#include <stack>
 
 namespace ROOT {
 namespace Internal {
@@ -27,10 +25,8 @@ namespace RDF {
 /// fixed at construction time and no blocking is foreseen.
 class RSlotStack {
 private:
-   unsigned int &GetCount();
-   unsigned int &GetIndex();
-   unsigned int fCursor;
-   std::vector<unsigned int> fBuf;
+   const unsigned int fSize;
+   std::stack<unsigned int> fStack;
    ROOT::TRWSpinLock fRWLock;
 
 public:
@@ -38,8 +34,6 @@ public:
    RSlotStack(unsigned int size);
    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/RSlotStack.cxx

@@ -8,77 +8,27 @@
  * For the list of contributors see $ROOTSYS/README/CREDITS.             *
  *************************************************************************/
 
+#include <ROOT/TSeq.hxx>
 #include <ROOT/RDF/RSlotStack.hxx>
 #include <TError.h> // R__ASSERT
 
-#include <limits>
-#include <numeric>
-
-ROOT::Internal::RDF::RSlotStack::RSlotStack(unsigned int size) : fCursor(size), fBuf(size)
-{
-   std::iota(fBuf.begin(), fBuf.end(), 0U);
-}
-
-unsigned int &ROOT::Internal::RDF::RSlotStack::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 &ROOT::Internal::RDF::RSlotStack::GetIndex()
+ROOT::Internal::RDF::RSlotStack::RSlotStack(unsigned int size) : fSize(size)
 {
-   const auto tid = std::this_thread::get_id();
-
-   {
-      ROOT::TRWSpinLockReadGuard rg(fRWLock);
-      auto it = fIndexMap.find(tid);
-      if (fIndexMap.end() != it)
-         return it->second;
-   }
-
-   {
-      ROOT::TRWSpinLockWriteGuard rg(fRWLock);
-      return (fIndexMap[tid] = std::numeric_limits<unsigned int>::max());
-   }
+   for (auto i : ROOT::TSeqU(size)) fStack.push(i);
 }
 
-void ROOT::Internal::RDF::RSlotStack::ReturnSlot(unsigned int slotNumber)
+void ROOT::Internal::RDF::RSlotStack::ReturnSlot(unsigned int slot)
 {
-   auto &index = GetIndex();
-   auto &count = GetCount();
-   R__ASSERT(count > 0U && "RSlotStack has a reference count relative to an index which will become negative.");
-   count--;
-   if (0U == count) {
-      index = std::numeric_limits<unsigned int>::max();
-      ROOT::TRWSpinLockWriteGuard guard(fRWLock);
-      fBuf[fCursor++] = slotNumber;
-      R__ASSERT(fCursor <= fBuf.size() &&
-                "RSlotStack assumes that at most a fixed number of values can be present in the "
-                "stack. fCursor is greater than the size of the internal buffer. This violates "
-                "such assumption.");
-   }
+   ROOT::TRWSpinLockWriteGuard guard(fRWLock);
+   R__ASSERT(fStack.size() < fSize && "Trying to put back a slot to a full stack!");
+   fStack.push(slot);
 }
 
 unsigned int ROOT::Internal::RDF::RSlotStack::GetSlot()
 {
-   auto &index = GetIndex();
-   auto &count = GetCount();
-   count++;
-   if (std::numeric_limits<unsigned int>::max() != index)
-      return index;
    ROOT::TRWSpinLockWriteGuard guard(fRWLock);
-   R__ASSERT(fCursor > 0 &&
-             "RSlotStack assumes that a value can be always obtained. In this case fCursor is <=0 and this "
-             "violates such assumption.");
-   index = fBuf[--fCursor];
-   return index;
+   R__ASSERT(!fStack.empty() && "Trying to pop a slot from an empty stack!");
+   const auto slot = fStack.top();
+   fStack.pop();
+   return slot;
 }

tree/dataframe/test/dataframe_nodes.cxx

@@ -11,7 +11,7 @@ TEST(RDataFrameNodes, RSlotStackCheckSameThreadSameSlot)
 {
    unsigned int n(7);
    ROOT::Internal::RDF::RSlotStack s(n);
-   EXPECT_EQ(s.GetSlot(), s.GetSlot());
+   EXPECT_EQ(s.GetSlot()-1, s.GetSlot());
 }
 
 #ifndef NDEBUG
@@ -32,7 +32,7 @@ TEST(RDataFrameNodes, RSlotStackGetOneTooMuch)
          t.join();
    };
 
-   EXPECT_DEATH(theTest(), "RSlotStack assumes that a value can be always obtained.");
+   EXPECT_DEATH(theTest(), "Trying to pop a slot from an empty stack!");
 }
 
 TEST(RDataFrameNodes, RSlotStackPutBackTooMany)
@@ -42,7 +42,7 @@ TEST(RDataFrameNodes, RSlotStackPutBackTooMany)
       s.ReturnSlot(0);
    };
 
-   EXPECT_DEATH(theTest(), "RSlotStack has a reference count relative to an index which will become negative");
+   EXPECT_DEATH(theTest(), "Trying to put back a slot to a full stack!");
 }
 
 #endif