Merge branch 'isValidID.IOVSvc-20201111' into 'master'

IOVSvc: Remove locking from CondSvc::isValidID.

See merge request atlas/athena!38117

Control/IOVSvc/src/CondInputLoader.cxx

@@ -24,6 +24,7 @@
 
 #include "xAODEventInfo/EventInfo.h"
 #include "AthenaKernel/BaseInfo.h"
+#include "ICondSvcSetupDone.h"
 
 #include "TClass.h"
 
@@ -265,6 +266,12 @@ CondInputLoader::start()
     return StatusCode::FAILURE;
   }
 
+  // Let the conditions service know that we've finished creating
+  // conditions containers.
+  ServiceHandle<ICondSvcSetupDone> condSvcDone ("CondSvc", name());
+  ATH_CHECK( condSvcDone.retrieve() );
+  ATH_CHECK( condSvcDone->setupDone() );
+
   return StatusCode::SUCCESS;
 
 }

Control/IOVSvc/src/CondSvc.cxx

@@ -130,6 +130,15 @@ CondSvc::finalize() {
 }
 
 
+StatusCode
+CondSvc::start()
+{
+  // Call this now, in case there's no CondInputLoader.
+  ATH_CHECK( setupDone() );
+  return StatusCode::SUCCESS;
+}
+
+
 StatusCode
 CondSvc::stop() {
 
@@ -342,7 +351,10 @@ CondSvc::getValidIDs(const EventContext& ctx, DataObjIDColl& validIDs) {
 
 bool
 CondSvc::isValidID(const EventContext& ctx, const DataObjID& id) const {
-  std::lock_guard<mutex_t> lock(m_lock);
+  // Don't take out the lock here.
+  // In many-thread jobs, a lock here becomes heavily contended.
+  // The only potential conflict is with startConditionSetup(),
+  // which should only be called during START.
 
   EventIDBase now(ctx.eventID());
 
@@ -352,13 +364,15 @@ CondSvc::isValidID(const EventContext& ctx, const DataObjID& id) const {
     sk.erase(0,15);
   }
 
-  if (m_sgs->contains<CondContBase>( sk ) ) {
-    CondContBase *cib;
-    if (m_sgs->retrieve(cib, sk).isSuccess()) {
-      ATH_MSG_VERBOSE("CondSvc::isValidID:  now: " << ctx.eventID() << "  id : " 
-                    << id << (cib->valid(now) ? ": T" : ": F") );
-      return cib->valid(now);
-    }
+  auto it = m_condConts.find (sk);
+  if (it != m_condConts.end()) {
+    bool valid = it->second->valid (now);
+    ATH_MSG_VERBOSE("CondSvc::isValidID:  now: " << ctx.eventID() << "  id : " 
+                    << id << (valid ? ": T" : ": F") );
+    return valid;
+  }
+  else {
+    ATH_MSG_ERROR( "Cannot find CondCont " << id );
   }
 
   ATH_MSG_DEBUG("CondSvc::isValidID:  now: " << ctx.eventID() << "  id: " 
@@ -396,3 +410,20 @@ CondSvc::conditionIDs() const {
 
 }
 
+//---------------------------------------------------------------------------
+
+
+StatusCode CondSvc::setupDone()
+{
+  std::lock_guard<mutex_t> lock(m_lock);
+
+  SG::ConstIterator<CondContBase> cib, cie;
+  if (m_sgs->retrieve(cib,cie).isSuccess()) {
+    while(cib != cie) {
+      m_condConts[cib.key()] = &*cib;
+      ++cib;      
+    }
+  }
+
+  return StatusCode::SUCCESS;
+}

Control/IOVSvc/src/CondSvc.h

 /*
-  Copyright (C) 2002-2017 CERN for the benefit of the ATLAS collaboration
+  Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
 */
 
 #ifndef IOVSVC_CONDSVC_H
@@ -10,7 +10,9 @@
 #include "AthenaBaseComps/AthService.h"
 #include "StoreGate/StoreGateSvc.h"
 #include "AthenaBaseComps/AthService.h"
+#include "ICondSvcSetupDone.h"
 
+#include <unordered_map>
 #include <map>
 #include <set>
 #include <vector>
@@ -19,38 +21,45 @@
 class ConditionSlotFuture;
 class ICondtionIOSvc;
 
-class ATLAS_CHECK_THREAD_SAFETY CondSvc: public extends1<AthService, ICondSvc> {
+class ATLAS_CHECK_THREAD_SAFETY CondSvc: public extends<AthService, ICondSvc, ICondSvcSetupDone> {
 public:
 
   CondSvc(const std::string& name, ISvcLocator* svc);
   ~CondSvc();
 
-  virtual StatusCode initialize();
-  virtual StatusCode finalize();
-  virtual StatusCode stop();
+  virtual StatusCode initialize() override;
+  virtual StatusCode finalize() override;
+  virtual StatusCode start() override;
+  virtual StatusCode stop() override;
 
   // from ICondSvc
 public:
-  virtual StatusCode regHandle(IAlgorithm* alg, const Gaudi::DataHandle& id);
+  virtual StatusCode regHandle(IAlgorithm* alg, const Gaudi::DataHandle& id) override;
 
   virtual bool getInvalidIDs(const EventContext&, DataObjIDColl& ids);
   virtual bool getValidIDs(const EventContext&, DataObjIDColl& ids);
   virtual bool getIDValidity(const EventContext&, DataObjIDColl& validIDs,
                              DataObjIDColl& invalidIDs);
-  virtual bool isValidID(const EventContext&, const DataObjID&) const;
+  virtual bool isValidID(const EventContext&, const DataObjID&) const override;
 
-  virtual const std::set<IAlgorithm*>& condAlgs() const { return m_condAlgs; }
+  virtual const std::set<IAlgorithm*>& condAlgs() const override { return m_condAlgs; }
 
-  virtual bool isRegistered(const DataObjID&) const;
-  virtual bool isRegistered(IAlgorithm*) const;
+  virtual bool isRegistered(const DataObjID&) const override;
+  virtual bool isRegistered(IAlgorithm*) const override;
   
-  virtual const DataObjIDColl& conditionIDs() const;
+  virtual const DataObjIDColl& conditionIDs() const override;
 
   virtual StatusCode validRanges( std::vector<EventIDRange>& ranges,
-                                  const DataObjID& id ) const;
+                                  const DataObjID& id ) const override;
 
   //  virtual void dump() const;
-  virtual void dump(std::ostream&) const;
+  virtual void dump(std::ostream&) const override;
+
+  /// To be called after changes to the set of conditions containers
+  /// in the conditions store.
+  /// May not be called concurrently with any other methods of this class.
+  virtual StatusCode setupDone() override;
+
 
 public:
   // unimplemented interfaces
@@ -62,7 +71,7 @@ public:
 
   /// register an IConditionIOSvc (alternative to Algorithm processing of 
   /// Conditions)
-  virtual StatusCode registerConditionIOSvc(IConditionIOSvc*) {
+  virtual StatusCode registerConditionIOSvc(IConditionIOSvc*)  override {
     return StatusCode::FAILURE;
   }
 
@@ -103,6 +112,11 @@ private:
 
   DataObjIDColl m_condIDs;
 
+  // Map from keys to CondContBase instances.
+  // Populated by startConditionSetup().
+  typedef std::unordered_map<std::string, const CondContBase*> CondContMap_t;
+  CondContMap_t m_condConts;
+
   typedef std::mutex mutex_t;
   mutable mutex_t m_lock;
 

Control/IOVSvc/src/ICondSvcSetupDone.h

+// This file's extension implies that it's C, but it's really -*- C++ -*-.
+/*
+ * Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration.
+ */
+/**
+ * @file IOVSvc/src/ICondSvcSetupDone.h
+ * @author scott snyder <snyder@bnl.gov>
+ * @date Nov, 2020
+ * @brief Interface to tell CondSvc to cache conditions containers.
+ */
+
+
+#ifndef IOVSVC_ICONDSVCSETUPDONE_H
+#define IOVSVC_ICONDSVCSETUPDONE_H
+
+
+#include "GaudiKernel/IInterface.h"
+
+
+class ICondSvcSetupDone
+  : virtual public IInterface
+{
+public:
+  DeclareInterfaceID( ICondSvcSetupDone, 1, 0 );
+
+  /// To be called after creating conditions containers
+  /// in the conditions store.
+  virtual StatusCode setupDone() = 0;
+};
+
+
+#endif // not IOVSVC_ICONDSVCSETUPDONE_H