gROOTMutex -> RWLock

core/base/inc/LinkDef3.h

@@ -165,6 +165,7 @@
 #pragma link C++ class TFileCollection+;
 #pragma link C++ class TVirtualAuth;
 #pragma link C++ class TVirtualMutex;
+#pragma link C++ class TVirtualRWMutex;
 #pragma link C++ class TLockGuard;
 #pragma link C++ class TRedirectOutputGuard;
 #pragma link C++ class TVirtualPerfStats;

core/base/inc/TVirtualMutex.h

@@ -29,7 +29,7 @@ class TVirtualMutex;
 // Global mutex set in TThread::Init
 R__EXTERN TVirtualMutex *gGlobalMutex;
 
-class TVirtualMutex : public TObject {
+class TVirtualMutex {
 
 public:
    TVirtualMutex(Bool_t /* recursive */ = kFALSE) { }

core/base/inc/TVirtualRWMutex.h

@@ -0,0 +1,49 @@
+// @(#)root/base:$Id$
+// Author: Philippe Canal, 2017
+
+/*************************************************************************
+ * Copyright (C) 1995-2017, 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_TVirtualRWMutex
+#define ROOT_TVirtualRWMutex
+
+
+//////////////////////////////////////////////////////////////////////////
+//                                                                      //
+// TVirtualRWMutex                                                      //
+//                                                                      //
+// This class implements a read-write mutex interface. The actual work  //
+// is done via TRWSpinLock which is available as soon as the thread     //
+// library is loaded.                                                   //
+//                                                                      //
+//////////////////////////////////////////////////////////////////////////
+
+#include "TVirtualMutex.h"
+
+
+class TVirtualRWMutex : public TVirtualMutex  {
+
+public:
+   virtual void ReadLock() = 0;
+   virtual void ReadUnLock() = 0;
+   virtual void WriteLock() = 0;
+   virtual void WriteUnLock() = 0;
+
+   Int_t Lock() override { WriteLock(); return 1; }
+   Int_t TryLock() override { WriteLock(); return 1; }
+   Int_t UnLock() override { WriteUnLock(); return 1; }
+   Int_t CleanUp() override { WriteUnLock(); return 1; }
+
+   TVirtualRWMutex *Factory(Bool_t /*recursive*/ = kFALSE) override = 0;
+
+   ClassDefOverride(TVirtualRWMutex,0)  // Virtual mutex lock class
+};
+
+
+
+#endif

core/base/src/TROOT.cxx

@@ -497,9 +497,6 @@ namespace Internal {
    /// Enables the global mutex to make ROOT thread safe/aware.
    void EnableThreadSafety()
    {
-      // 'Insure' gROOT is created before initializing the Thread safe behavior
-      // (to make sure we do not have two attempting to create it).
-      GetROOT();
       static void (*sym)() = (void(*)())Internal::GetSymInLibImt("ROOT_TThread_Initialize");
       if (sym)
          sym();

core/cont/inc/TCollection.h

@@ -60,7 +60,7 @@ R__EXTERN TVirtualMutex *gCollectionMutex;
 class TCollection : public TObject {
 
 #ifdef R__CHECK_COLLECTION_MULTI_ACCESS
-protected:
+public:
    class TErrorLock {
       // Warn when multiple thread try to acquire the same 'lock'
       std::atomic<std::thread::id> fWriteCurrent;
@@ -81,7 +81,7 @@ class TCollection : public TObject {
                           const char *function);
 
    public:
-      TErrorLock() : fWriteCurrentRecurse(0), fReadCurrentRecurse(0) { std::atomic_flag_clear(&fSpinLockFlag); }
+      TErrorLock() : fWriteCurrent(), fWriteCurrentRecurse(0), fReadCurrentRecurse(0) { std::atomic_flag_clear(&fSpinLockFlag); }
 
       class WriteGuard {
          TErrorLock *fLock;

core/metacling/src/TCling.cxx

@@ -2981,6 +2981,12 @@ Bool_t TCling::HandleNewTransaction(const cling::Transaction &T)
 
 void TCling::RecursiveRemove(TObject* obj)
 {
+   // NOTE: When replacing the mutex by a ReadWrite mutex, we **must**
+   // put in place the Read/Write part here.  Keeping the write lock
+   // here is 'catasptrophic' for scaling as it means that ALL calls
+   // to RecursiveRemove will take the write lock and performance
+   // of many threads trying to access the write lock at the same
+   // time is relatively bad.
    R__LOCKGUARD(gInterpreterMutex);
    // Note that fgSetOfSpecials is supposed to be updated by TClingCallbacks::tryFindROOTSpecialInternal
    // (but isn't at the moment).

core/thread/CMakeLists.txt

@@ -5,7 +5,7 @@
 set(headers TAtomicCount.h TCondition.h TConditionImp.h TMutex.h TMutexImp.h
             TRWLock.h ROOT/TRWSpinLock.hxx TSemaphore.h TThread.h TThreadFactory.h
             TThreadImp.h ROOT/TThreadedObject.hxx TThreadPool.h
-            ThreadLocalStorage.h ROOT/TSpinMutex.hxx)
+            ThreadLocalStorage.h ROOT/TSpinMutex.hxx ROOT/TReentrantRWLock.hxx)
 if(NOT WIN32)
   set(headers ${headers} TPosixCondition.h TPosixMutex.h
                          TPosixThread.h TPosixThreadFactory.h PosixThreadInc.h)
@@ -18,7 +18,7 @@ endif()
 
 set(sources TCondition.cxx TConditionImp.cxx TMutex.cxx TMutexImp.cxx
             TRWLock.cxx TRWSpinLock.cxx TSemaphore.cxx TThread.cxx TThreadFactory.cxx
-            TThreadImp.cxx)
+            TThreadImp.cxx TRWMutexImp.cxx TReentrantRWLock.cxx)
 if(NOT WIN32)
   set(sources ${sources} TPosixCondition.cxx TPosixMutex.cxx
                          TPosixThread.cxx TPosixThreadFactory.cxx)
@@ -32,3 +32,7 @@ ROOT_GENERATE_DICTIONARY(G__Thread ${headers} STAGE1 MODULE Thread LINKDEF LinkD
 ROOT_OBJECT_LIBRARY(ThreadObjs ${sources} G__Thread.cxx)
 ROOT_LINKER_LIBRARY(Thread $<TARGET_OBJECTS:ThreadObjs> LIBRARIES ${CMAKE_THREAD_LIBS_INIT} DEPENDENCIES Core BUILTINS)
 ROOT_INSTALL_HEADERS(${installoptions})
+
+if(testing)
+    add_subdirectory(test)
+endif()

core/thread/Module.mk

@@ -25,7 +25,8 @@ THREADH      := $(MODDIRI)/TCondition.h $(MODDIRI)/TConditionImp.h \
                 $(MODDIRI)/TThreadImp.h $(MODDIRI)/TAtomicCount.h \
                 $(MODDIRI)/TThreadPool.h $(MODDIRI)/ThreadLocalStorage.h \
                 $(MODDIRI)/ROOT/TThreadedObject.hxx \
-                $(MODDIRI)/ROOT/TSpinMutex.hxx
+                $(MODDIRI)/ROOT/TSpinMutex.hxx \
+                $(MODDIRI)/ROOT/TReentrantRWLock.hxx
 
 ifeq ($(IMT),yes)
 THREADH      += $(MODDIRI)/ROOT/TThreadExecutor.hxx
@@ -48,7 +49,7 @@ THREADS      := $(MODDIRS)/TCondition.cxx $(MODDIRS)/TConditionImp.cxx \
                 $(MODDIRS)/TMutex.cxx $(MODDIRS)/TMutexImp.cxx \
                 $(MODDIRS)/TRWLock.cxx $(MODDIRS)/TSemaphore.cxx \
                 $(MODDIRS)/TThread.cxx $(MODDIRS)/TThreadFactory.cxx \
-                $(MODDIRS)/TThreadImp.cxx
+                $(MODDIRS)/TThreadImp.cxx $(MODDIRS)/TReentrantRWLock.cxx
 ifneq ($(ARCH),win32)
 THREADS      += $(MODDIRS)/TPosixCondition.cxx $(MODDIRS)/TPosixMutex.cxx \
                 $(MODDIRS)/TPosixThread.cxx $(MODDIRS)/TPosixThreadFactory.cxx

core/thread/inc/ROOT/TReentrantRWLock.hxx

@@ -0,0 +1,150 @@
+// @(#)root/thread:$Id$
+// Authors: Enric Tejedor CERN  12/09/2016
+//          Philippe Canal FNAL 12/09/2016
+
+/*************************************************************************
+ * Copyright (C) 1995-2016, 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_TRWSpinLock
+#define ROOT_TRWSpinLock
+
+#include "TSpinMutex.hxx"
+
+#include <atomic>
+#include <condition_variable>
+#include <thread>
+#include <unordered_map>
+
+namespace ROOT {
+namespace Internal {
+struct UniqueLockRecurseCount {
+   struct LocalCounts {
+      int fReadersCount = 0;
+      bool fIsWriter = false;
+   };
+   size_t fWriteRecurse = 0; ///<! Number of re-entry in the lock by the same thread.
+
+   UniqueLockRecurseCount();
+
+   using local_t = LocalCounts*;
+
+   local_t GetLocal() {
+      static thread_local LocalCounts gLocal;
+      return &gLocal;
+   }
+
+   void IncrementReadCount(local_t &local) { ++(local->fReadersCount); }
+
+   template <typename MutexT>
+   void IncrementReadCount(local_t &local, MutexT &) { IncrementReadCount(local); }
+
+   void DecrementReadCount(local_t &local) { --(local->fReadersCount); }
+
+   template <typename MutexT>
+   void DecrementReadCount(local_t &local, MutexT &) { DecrementReadCount(local); }
+
+   bool IsNotCurrentWriter(local_t &local) { return !local->fIsWriter; }
+
+   void SetIsWriter(local_t &local)
+   {
+      // if (fWriteRecurse == std::numeric_limits<decltype(fWriteRecurse)>::max()) {
+      //    ::Fatal("TRWSpinLock::WriteLock", "Too many recursions in TRWSpinLock!");
+      // }
+      ++fWriteRecurse;
+      local->fIsWriter = true;
+   }
+
+   void DecrementWriteCount() { --fWriteRecurse; }
+
+   void ResetIsWriter(local_t &local) { local->fIsWriter = false; }
+
+   size_t GetLocalReadersCount(local_t &local) { return local->fReadersCount; }
+};
+
+struct RecurseCounts {
+   using ReaderColl_t = std::unordered_map<std::thread::id, int>;
+   size_t fWriteRecurse; ///<! Number of re-entry in the lock by the same thread.
+
+   std::thread::id fWriterThread; ///<! Holder of the write lock
+   ReaderColl_t fReadersCount;    ///<! Set of reader thread ids
+
+   using local_t = std::thread::id;
+
+   local_t GetLocal() { return std::this_thread::get_id(); }
+
+   void IncrementReadCount(local_t &local) { ++(fReadersCount[local]); }
+
+   template <typename MutexT>
+   void IncrementReadCount(local_t &local, MutexT &mutex)
+   {
+      std::unique_lock<MutexT> lock(mutex);
+      IncrementReadCount(local);
+   }
+
+   void DecrementReadCount(local_t &local) { --(fReadersCount[local]); }
+
+   template <typename MutexT>
+   void DecrementReadCount(local_t &local, MutexT &mutex)
+   {
+      std::unique_lock<MutexT> lock(mutex);
+      DecrementReadCount(local);
+   }
+
+   bool IsNotCurrentWriter(local_t &local) { return fWriterThread != local; }
+
+   void SetIsWriter(local_t &local)
+   {
+      // if (fWriteRecurse == std::numeric_limits<decltype(fWriteRecurse)>::max()) {
+      //    ::Fatal("TRWSpinLock::WriteLock", "Too many recursions in TRWSpinLock!");
+      // }
+      ++fWriteRecurse;
+      fWriterThread = local;
+   }
+
+   void DecrementWriteCount() { --fWriteRecurse; }
+
+   void ResetIsWriter(local_t & /* local */) { fWriterThread = std::thread::id(); }
+
+   size_t GetLocalReadersCount(local_t &local) { return fReadersCount[local]; }
+
+
+};
+} // Internal
+
+template <typename MutexT = ROOT::TSpinMutex, typename RecurseCountsT = Internal::RecurseCounts>
+class TReentrantRWLock {
+private:
+
+   std::atomic<int> fReaders;           ///<! Number of readers
+   std::atomic<int> fReaderReservation; ///<! A reader wants access
+   std::atomic<int> fWriterReservation; ///<! A writer wants access
+   std::atomic<bool> fWriter;           ///<! Is there a writer?
+   MutexT fMutex;                       ///<! RWlock internal mutex
+   std::condition_variable_any fCond;   ///<! RWlock internal condition variable
+
+   RecurseCountsT fRecurseCounts;        ///<! Trackers for re-entry in the lock by the same thread.
+
+   // size_t fWriteRecurse;                ///<! Number of re-entry in the lock by the same thread.
+
+   // std::thread::id fWriterThread; ///<! Holder of the write lock
+   // ReaderColl_t fReadersCount;    ///<! Set of reader thread ids
+
+public:
+   ////////////////////////////////////////////////////////////////////////
+   /// Regular constructor.
+   TReentrantRWLock() : fReaders(0), fReaderReservation(0), fWriterReservation(0), fWriter(false) {}
+
+   void ReadLock();
+   void ReadUnLock();
+   void WriteLock();
+   void WriteUnLock();
+
+   };
+} // end of namespace ROOT
+
+#endif

core/thread/inc/TMutex.h

@@ -47,6 +47,10 @@ friend class TThread;
    Int_t  UnLock();
    Int_t  CleanUp();
 
+   // Compatibility with standard library
+   void lock() { TMutex::Lock(); }
+   void unlock() { TMutex::UnLock(); }
+
    TVirtualMutex *Factory(Bool_t recursive = kFALSE);
 
    ClassDef(TMutex,0)  // Mutex lock class

core/thread/src/TMutex.cxx

@@ -23,7 +23,7 @@
 #include "TMutex.h"
 #include "TThreadFactory.h"
 #include <errno.h>
-
+#include "TError.h"
 
 ClassImp(TMutex);
 

core/thread/src/TRWMutexImp.cxx

@@ -0,0 +1,73 @@
+// @(#)root/thread:$Id$
+// Author: Fons Rademakers   26/06/97
+
+/*************************************************************************
+ * Copyright (C) 1995-2017, Rene Brun and Fons Rademakers.               *
+ * All rights reserved.                                                  *
+ *                                                                       *
+ * For the licensing terms see $ROOTSYS/LICENSE.                         *
+ * For the list of contributors see $ROOTSYS/README/CREDITS.             *
+ *************************************************************************/
+
+//////////////////////////////////////////////////////////////////////////
+//                                                                      //
+// TRWMutexImp                                                          //
+//                                                                      //
+// This class implements the TVirtualRWMutex interface,                 //
+// based on TRWSpinLock.                                                //
+//                                                                      //
+//////////////////////////////////////////////////////////////////////////
+
+#include "TRWMutexImp.h"
+#include "ROOT/TSpinMutex.hxx"
+#include "TMutex.h"
+
+////////////////////////////////////////////////////////////////////////////////
+/// Take the Read Lock of the mutex.
+
+template <typename MutexT, typename RecurseCountsT>
+void TRWMutexImp<MutexT, RecurseCountsT>::ReadLock()
+{
+   fMutexImp.ReadLock();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+/// Take the Write Lock of the mutex.
+
+template <typename MutexT, typename RecurseCountsT>
+void TRWMutexImp<MutexT, RecurseCountsT>::WriteLock()
+{
+   fMutexImp.WriteLock();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+/// Release the read lock of the mutex
+
+template <typename MutexT, typename RecurseCountsT>
+void TRWMutexImp<MutexT, RecurseCountsT>::ReadUnLock()
+{
+   fMutexImp.ReadUnLock();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+/// Release the read lock of the mutex
+
+template <typename MutexT, typename RecurseCountsT>
+void TRWMutexImp<MutexT, RecurseCountsT>::WriteUnLock()
+{
+   fMutexImp.WriteUnLock();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+/// Create mutex and return pointer to it.
+
+template <typename MutexT, typename RecurseCountsT>
+TVirtualRWMutex *TRWMutexImp<MutexT, RecurseCountsT>::Factory(Bool_t /*recursive = kFALSE*/)
+{
+   return new TRWMutexImp();
+}
+
+template class TRWMutexImp<TMutex>;
+template class TRWMutexImp<ROOT::TSpinMutex>;
+template class TRWMutexImp<TMutex, ROOT::Internal::UniqueLockRecurseCount>;
+template class TRWMutexImp<ROOT::TSpinMutex, ROOT::Internal::UniqueLockRecurseCount>;