From 3e5115e9e31bd18f2d8f336075def4d8290a4f7f Mon Sep 17 00:00:00 2001
From: Adam Edward Barton <adam.edward.barton@cern.ch>
Date: Mon, 25 May 2020 10:51:54 +0000
Subject: [PATCH] Virtualize the internal code of the IdentifiableContainer and
use standard vector iteration for faster access
---
.../T_AthenaPoolCreateFuncs.h | 6 +-
Database/TPTools/TPTools/TPConverter.icc | 9 +-
Event/EventContainers/CMakeLists.txt | 5 +
.../EventContainers/IDC_WriteHandleBase.h | 6 -
.../EventContainers/IIdentifiableCont.h | 2 +-
.../EventContainers/I_InternalIDC.h | 50 +++++
.../EventContainers/IdentifiableCache.h | 4 +-
.../EventContainers/IdentifiableCacheBase.h | 55 +----
.../IdentifiableContainerBase.h | 45 ++--
.../EventContainers/IdentifiableContainerMT.h | 194 +++++-------------
.../EventContainers/InternalOffline.h | 45 ++++
.../EventContainers/InternalOnline.h | 57 +++++
.../EventContainers/SelectAllObjectMT.h | 35 ++--
.../EventContainers/EventContainers/deleter.h | 37 ++++
Event/EventContainers/share/IDBenchTest.ref | 1 +
Event/EventContainers/share/IDStressTest.ref | 10 +-
.../src/IDC_WriteHandleBase.cxx | 1 -
.../src/IdentifiableCacheBase.cxx | 58 ++----
.../src/IdentifiableContainerBase.cxx | 106 ++--------
Event/EventContainers/src/InternalOffline.cxx | 110 ++++++++++
Event/EventContainers/src/InternalOnline.cxx | 154 ++++++++++++++
Event/EventContainers/test/IDC_Benchmark.cxx | 71 +++++++
.../test/IDC_Realistic_Test.cxx | 44 +++-
.../test/IDMT_ContainerTest.cxx | 47 ++++-
24 files changed, 761 insertions(+), 391 deletions(-)
create mode 100644 Event/EventContainers/EventContainers/I_InternalIDC.h
create mode 100644 Event/EventContainers/EventContainers/InternalOffline.h
create mode 100644 Event/EventContainers/EventContainers/InternalOnline.h
create mode 100644 Event/EventContainers/EventContainers/deleter.h
create mode 100644 Event/EventContainers/share/IDBenchTest.ref
create mode 100644 Event/EventContainers/src/InternalOffline.cxx
create mode 100644 Event/EventContainers/src/InternalOnline.cxx
create mode 100644 Event/EventContainers/test/IDC_Benchmark.cxx
diff --git a/Database/AthenaPOOL/AthenaPoolCnvSvc/AthenaPoolCnvSvc/T_AthenaPoolCreateFuncs.h b/Database/AthenaPOOL/AthenaPoolCnvSvc/AthenaPoolCnvSvc/T_AthenaPoolCreateFuncs.h
index f9d4e0aaafd..72a905ee01a 100644
--- a/Database/AthenaPOOL/AthenaPoolCnvSvc/AthenaPoolCnvSvc/T_AthenaPoolCreateFuncs.h
+++ b/Database/AthenaPOOL/AthenaPoolCnvSvc/AthenaPoolCnvSvc/T_AthenaPoolCreateFuncs.h
@@ -29,8 +29,10 @@
#include "GaudiKernel/MsgStream.h"
#include <memory>
#include <type_traits>
-class IdentifiableContainerBase;
+namespace EventContainers{
+ class IdentifiableContainerBase;
+}
namespace AthenaPoolCnvSvc {
@@ -124,7 +126,7 @@ createTransient (TPCNV& cnv,
MsgStream& log)
{
typedef typename TPCNV::Trans_t Trans_t;
- if constexpr(std::is_base_of< IdentifiableContainerBase, Trans_t>::value &&
+ if constexpr(std::is_base_of< EventContainers::IdentifiableContainerBase, Trans_t>::value &&
!std::is_default_constructible<Trans_t>::value)
{
log << "IdentifiableContainerBase is not compatible with createTransient" << endmsg;
diff --git a/Database/TPTools/TPTools/TPConverter.icc b/Database/TPTools/TPTools/TPConverter.icc
index 0a2b2209aa1..90ce12ed48f 100644
--- a/Database/TPTools/TPTools/TPConverter.icc
+++ b/Database/TPTools/TPTools/TPConverter.icc
@@ -70,14 +70,15 @@ toPersistentWithKey_impl( const TRANS *trans,
}
return TPObjRef( this->m_pStorageTID, size );
}
-
+namespace EventContainers{
class IdentifiableContainerBase;
+}
class IdentifiableValueContainerBase;
template< class TRANS_BASE, class TRANS, class PERS >
TRANS* TPPolyCnvBase<TRANS_BASE, TRANS, PERS>::
createTransient ([[maybe_unused]] const PERS* persObj, MsgStream &log)
{
- if constexpr(std::is_base_of< IdentifiableContainerBase, TRANS>::value && !std::is_default_constructible<TRANS>::value) {
+ if constexpr(std::is_base_of< EventContainers::IdentifiableContainerBase, TRANS>::value && !std::is_default_constructible<TRANS>::value) {
log << "IdentifiableContainerBase is not compatable with createTransient" << std::endl;
return nullptr;
}else if constexpr(std::is_base_of< IdentifiableValueContainerBase, TRANS>::value && !std::is_default_constructible<TRANS>::value){
@@ -99,7 +100,7 @@ createTransientWithKey ([[maybe_unused]] const PERS* persObj,
const std::string& key,
MsgStream& log)
{
- if constexpr(std::is_base_of< IdentifiableContainerBase, TRANS>::value && !std::is_default_constructible<TRANS>::value) {
+ if constexpr(std::is_base_of< EventContainers::IdentifiableContainerBase, TRANS>::value && !std::is_default_constructible<TRANS>::value) {
log << "IdentifiableContainerBase is not compatable with createTransient" << std::endl;
return nullptr;
}else if constexpr(std::is_base_of< IdentifiableValueContainerBase, TRANS>::value && !std::is_default_constructible<TRANS>::value){
@@ -120,7 +121,7 @@ template< class TRANS, class PERS >
TRANS* TPConverterConstBase<TRANS, PERS>::createTransientConst
(const PERS* persObj, MsgStream& log) const
{
- if constexpr(std::is_base_of< IdentifiableContainerBase, TRANS>::value && !std::is_default_constructible<TRANS>::value) {
+ if constexpr(std::is_base_of< EventContainers::IdentifiableContainerBase, TRANS>::value && !std::is_default_constructible<TRANS>::value) {
log << "IdentifiableContainerBase is not compatable with createTransient" << std::endl;
return nullptr;
}else if constexpr(std::is_base_of< IdentifiableValueContainerBase, TRANS>::value && !std::is_default_constructible<TRANS>::value){
diff --git a/Event/EventContainers/CMakeLists.txt b/Event/EventContainers/CMakeLists.txt
index 3b2187e6cac..6a78b1d010c 100644
--- a/Event/EventContainers/CMakeLists.txt
+++ b/Event/EventContainers/CMakeLists.txt
@@ -38,3 +38,8 @@ atlas_add_test( IDCValueTest SOURCES test/IDCValueTest.cxx
INCLUDE_DIRS src test EventContainers
LINK_LIBRARIES Identifier AthenaKernel GaudiKernel EventContainers
)
+atlas_add_test( IDBenchTest SOURCES test/IDC_Benchmark.cxx
+ INCLUDE_DIRS src test EventContainers
+ LINK_LIBRARIES Identifier AthenaKernel GaudiKernel EventContainers
+ LOG_IGNORE_PATTERN "time"
+ )
\ No newline at end of file
diff --git a/Event/EventContainers/EventContainers/IDC_WriteHandleBase.h b/Event/EventContainers/EventContainers/IDC_WriteHandleBase.h
index 719404a7c1d..1e25cf03262 100644
--- a/Event/EventContainers/EventContainers/IDC_WriteHandleBase.h
+++ b/Event/EventContainers/EventContainers/IDC_WriteHandleBase.h
@@ -14,13 +14,7 @@ namespace EventContainers {
struct mutexPair{
std::condition_variable condition;
std::mutex mutex;
-#ifndef NDEBUG
- std::atomic<int> counter;
-#endif
mutexPair() : condition(), mutex() {
-#ifndef NDEBUG
- counter.store(0);
-#endif
}
};
diff --git a/Event/EventContainers/EventContainers/IIdentifiableCont.h b/Event/EventContainers/EventContainers/IIdentifiableCont.h
index 1d9e18de2fd..1d644f5e094 100644
--- a/Event/EventContainers/EventContainers/IIdentifiableCont.h
+++ b/Event/EventContainers/EventContainers/IIdentifiableCont.h
@@ -5,7 +5,7 @@
#ifndef EVENTCONTAINERS_IIDENTIFIABLECONT_H
#define EVENTCONTAINERS_IIDENTIFIABLECONT_H
-#include "Identifier/Identifier.h"
+#include "Identifier/IdentifierHash.h"
#include <memory>
namespace EventContainers {
diff --git a/Event/EventContainers/EventContainers/I_InternalIDC.h b/Event/EventContainers/EventContainers/I_InternalIDC.h
new file mode 100644
index 00000000000..2300f952670
--- /dev/null
+++ b/Event/EventContainers/EventContainers/I_InternalIDC.h
@@ -0,0 +1,50 @@
+/*
+ Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
+*/
+
+#ifndef EVENTCONTAINERS_I_INTERNALIDC_H
+#define EVENTCONTAINERS_I_INTERNALIDC_H
+
+#include "Identifier/IdentifierHash.h"
+#include "GaudiKernel/StatusCode.h"
+#include <vector>
+#include <utility>
+
+namespace EventContainers{
+class IDC_WriteHandleBase;
+/*
+The intention of the IdentifiableContainer is to provide a key-value map
+for collection pointers. To increase memory and cpu efficiency the online trigger
+system different “views†can share collection that are created concurrently.
+To efficiently support these different uses while not imposing overhead on the
+offline case and to maintain a consistent interface the internals of the class are
+virtualised.
+
+A standard iterator is provided for fast iteration in a sequential order.
+*/
+class I_InternalIDC{
+public:
+ typedef std::pair < IdentifierHash::value_type, const void* > hashPair;
+ typedef std::vector < hashPair >::const_iterator InternalConstItr;
+ #include "EventContainers/deleter.h"
+ virtual InternalConstItr cbegin() const=0;
+ virtual InternalConstItr cend() const=0;
+ virtual InternalConstItr indexFind( IdentifierHash hashId ) const =0;
+ virtual ~I_InternalIDC() = default;
+ virtual void Wait() const = 0;
+ virtual bool tryAddFromCache(IdentifierHash hashId, EventContainers::IDC_WriteHandleBase &lock) = 0;
+ virtual bool tryAddFromCache(IdentifierHash hashId) = 0;
+ virtual std::vector<IdentifierHash> GetAllCurrentHashes() const =0;
+ virtual const std::vector < hashPair >& GetAllHashPtrPair() const = 0;
+ virtual size_t numberOfCollections() const = 0;
+ virtual size_t fullSize() const noexcept =0;
+ virtual StatusCode fetchOrCreate(IdentifierHash hashId) =0;
+ virtual StatusCode fetchOrCreate(const std::vector<IdentifierHash> &hashIds) =0;
+ virtual bool insert(IdentifierHash hashId, const void* ptr) =0;
+ virtual const void* FindIndexPtr(IdentifierHash hashId) const noexcept = 0;
+ virtual StatusCode addLock(IdentifierHash hashId, const void* ptr) = 0;
+ virtual void* removeCollection( IdentifierHash hashId ) =0;
+};
+
+}
+#endif
\ No newline at end of file
diff --git a/Event/EventContainers/EventContainers/IdentifiableCache.h b/Event/EventContainers/EventContainers/IdentifiableCache.h
index 0c0d3aa7d22..e95325dfeda 100644
--- a/Event/EventContainers/EventContainers/IdentifiableCache.h
+++ b/Event/EventContainers/EventContainers/IdentifiableCache.h
@@ -71,12 +71,12 @@ public:
return reinterpret_cast<const T*> (IdentifiableCacheBase::get (hash));
}
- bool add (IdentifierHash hash, const T* p)
+ std::pair<bool, const void*> add (IdentifierHash hash, const T* p)
{
return IdentifiableCacheBase::add (hash, p);
}
- bool add (IdentifierHash hash, std::unique_ptr<T> p)
+ std::pair<bool, const void*> add (IdentifierHash hash, std::unique_ptr<T> p)
{
return IdentifiableCacheBase::add (hash, void_unique_ptr(std::move(p)));
}
diff --git a/Event/EventContainers/EventContainers/IdentifiableCacheBase.h b/Event/EventContainers/EventContainers/IdentifiableCacheBase.h
index 92a12a85b8f..be173fe0913 100644
--- a/Event/EventContainers/EventContainers/IdentifiableCacheBase.h
+++ b/Event/EventContainers/EventContainers/IdentifiableCacheBase.h
@@ -5,12 +5,7 @@
*/
// $Id: IdentifiableCacheBase.h 791541 2017-01-09 10:43:53Z smh $
-/**
- * @file IdentifiableCacheBase.h
- * @author scott snyder <snyder@bnl.gov>
- * @date Mar, 2016
- * @brief
- */
+
#ifndef EVENTCONTAINERS_IDENTIFIABLECACHEBASE_H
@@ -37,43 +32,13 @@ public:
//here for access from other classes
static constexpr uintptr_t INVALIDflag = UINTPTR_MAX;
static constexpr uintptr_t ABORTEDflag = UINTPTR_MAX-1;
-static constexpr size_t s_defaultBucketSize =6;
+static constexpr size_t s_defaultBucketSize =2;
typedef std::true_type thread_safe;
typedef std::set<IdentifierHash> idset_t;
-#if 0
- struct deleter
- {
- void operator() (const void* p);
- };
-#endif
-
- typedef void deleter_f (const void* p);
+#include "EventContainers/deleter.h"
- class void_unique_ptr
- : public std::unique_ptr<const void, deleter_f*>
- {
- public:
- using std::unique_ptr<const void, deleter_f*>::unique_ptr;
-
- template <class T>
- struct Deleter
- {
- static void deleter (const void* p)
- {
- delete reinterpret_cast<const T*>(p);
- }
- };
-
- template <class T>
- void_unique_ptr(std::unique_ptr<T> p)
- : std::unique_ptr<const void, deleter_f*> (p.release(),
- Deleter<T>::deleter)
- {
- }
- };
-
struct IMaker
{
bool m_IsReEntrant = false;
@@ -92,13 +57,13 @@ typedef std::set<IdentifierHash> idset_t;
///In a threaded situation this collection will be valid but will not container hashes later added
std::vector<IdentifierHash> ids();
- bool add (IdentifierHash hash, const void* p) noexcept;
+ std::pair<bool, const void*> add (IdentifierHash hash, const void* p) noexcept;
// addLock is same as method above except we check for invalid state first,
// more optimal for calling using writehandle lock method
- bool addLock (IdentifierHash hash, const void* p) noexcept;
- bool addLock (IdentifierHash hash, void_unique_ptr p) noexcept;
- bool add (IdentifierHash hash, void_unique_ptr p) noexcept;
+ std::pair<bool, const void*> addLock (IdentifierHash hash, const void* p) noexcept;
+ std::pair<bool, const void*> addLock (IdentifierHash hash, void_unique_ptr p) noexcept;
+ std::pair<bool, const void*> add (IdentifierHash hash, void_unique_ptr p) noexcept;
bool IMakerPresent() const { return m_maker!=nullptr; }
@@ -128,9 +93,7 @@ typedef std::set<IdentifierHash> idset_t;
size_t fullSize() const { return m_vec.size(); }
///In a concurrent situation this number isn't necessarily perfectly synchronised with ids().size()
size_t numberOfHashes();
-#ifndef NDEBUG
- void cancelWait(IdentifierHash hash);
-#endif
+
protected:
IdentifiableCacheBase (IdentifierHash maxHash, const IMaker* maker, size_t lockBucketSize);
IdentifiableCacheBase (IdentifierHash maxHash, const IMaker* maker);
@@ -140,7 +103,7 @@ protected:
void notifyHash(IdentifierHash hash);
private:
std::vector<std::atomic<const void*> > m_vec;
-
+ friend class InternalOnline;
const IMaker* m_maker;
typedef std::mutex mutex_t;
diff --git a/Event/EventContainers/EventContainers/IdentifiableContainerBase.h b/Event/EventContainers/EventContainers/IdentifiableContainerBase.h
index 86d697f8226..3e6d14b424f 100644
--- a/Event/EventContainers/EventContainers/IdentifiableContainerBase.h
+++ b/Event/EventContainers/EventContainers/IdentifiableContainerBase.h
@@ -1,39 +1,42 @@
/*
- Copyright (C) 2002-2018 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
*/
#ifndef EVENTCONTAINERS_IDENTIFIABLECONTAINERBASE_H
#define EVENTCONTAINERS_IDENTIFIABLECONTAINERBASE_H
-
-#include "EventContainers/IdentifiableCacheBase.h"
+#include "EventContainers/I_InternalIDC.h"
#include "CxxUtils/checker_macros.h"
-class IdentifiableContainerBase{
+namespace EventContainers{ class IdentifiableCacheBase;
+
+class IdentifiableContainerBase{
public:
+#include "EventContainers/deleter.h"
typedef std::vector<IdentifierHash> Hash_Container;
- IdentifiableContainerBase(EventContainers::IdentifiableCacheBase *cache, bool online);
- static Hash_Container::const_iterator findHash(Hash_Container*, IdentifierHash);
- ~IdentifiableContainerBase();
+ IdentifiableContainerBase(EventContainers::IdentifiableCacheBase *cache);
+ IdentifiableContainerBase(size_t maxsize);
+ ~IdentifiableContainerBase() = default;
protected:
- EventContainers::IdentifiableCacheBase *m_cacheLink;
bool m_OnlineMode;
- mutable std::atomic<bool> m_waitNeeded ATLAS_THREAD_SAFE; //These mutables are carefully thought out, do not change
typedef EventContainers::IdentifiableCacheBase IdentifiableCacheBase;
- mutable std::vector< IdentifierHash > m_waitlist ATLAS_THREAD_SAFE;
- mutable std::mutex m_waitMutex ATLAS_THREAD_SAFE;
- mutable std::vector<bool> m_mask ATLAS_THREAD_SAFE;
- std::vector<IdentifierHash> GetAllCurrentHashes() const;
-
- void Wait() const;
- bool tryAddFromCache(IdentifierHash hashId, EventContainers::IDC_WriteHandleBase &lock);
- bool tryAddFromCache(IdentifierHash hashId);
- void cleanup();
- size_t numberOfCollections() const;
+ std::unique_ptr<I_InternalIDC> m_link;
+ std::vector<IdentifierHash> GetAllCurrentHashes() const { return m_link->GetAllCurrentHashes(); }
+ bool insert(IdentifierHash hashId, const void* ptr) { return m_link->insert(hashId, ptr); }
+ void Wait() const { m_link->Wait(); }
+ bool tryAddFromCache(IdentifierHash hashId, EventContainers::IDC_WriteHandleBase &lock) {
+ return m_link->tryAddFromCache(hashId, lock); }
+ bool tryAddFromCache(IdentifierHash hashId){ return m_link->tryAddFromCache(hashId); }
+ void cleanup(deleter_f* deleter);
+ size_t numberOfCollections() const { return m_link->numberOfCollections(); }
void ResetMask();
- StatusCode fetchOrCreate(IdentifierHash hashId);
- StatusCode fetchOrCreate(const std::vector<IdentifierHash> &hashIds);
+ StatusCode fetchOrCreate(IdentifierHash hashId){ return m_link->fetchOrCreate(hashId); }
+ StatusCode fetchOrCreate(const std::vector<IdentifierHash> &hashIds) { return m_link->fetchOrCreate(hashIds); }
+ const void* indexFindPtr( IdentifierHash hashId ) const { return m_link->FindIndexPtr(hashId); }
};
+}
+
+
#endif
diff --git a/Event/EventContainers/EventContainers/IdentifiableContainerMT.h b/Event/EventContainers/EventContainers/IdentifiableContainerMT.h
index cb1e7bb2fb3..054d29a9360 100644
--- a/Event/EventContainers/EventContainers/IdentifiableContainerMT.h
+++ b/Event/EventContainers/EventContainers/IdentifiableContainerMT.h
@@ -16,48 +16,34 @@
*/
#include "Identifier/Identifier.h"
-#include "EventContainers/IdentifiableCache.h"
#include "EventContainers/IIdentifiableCont.h"
#include <memory>
#include "GaudiKernel/DataObject.h"
#include "EventContainers/IdentifiableContainerBase.h"
#include "EventContainers/IDC_WriteHandleBase.h"
#include "CxxUtils/AthUnlikelyMacros.h"
-//const_iterator and indexFind are provided for backwards compatability. they are not optimal
-/*
-IT is faster to iterate over the container with this method:
- auto hashes= m_container->GetAllCurrentHashes();
- for (auto hash : hashes) {
- T* coll = m_container->indexFindPtr(hash);
- //Use coll here
- }
-*/
+#include "EventContainers/IdentifiableCache.h"
+#include "EventContainers/InternalOffline.h"
template < class T>
-class IdentifiableContainerMT : public DataObject, public IdentifiableContainerBase, public EventContainers::IIdentifiableCont<T>
+class IdentifiableContainerMT : public DataObject, public EventContainers::IdentifiableContainerBase, public EventContainers::IIdentifiableCont<T>
{
-
public:
class IDC_WriteHandle : EventContainers::IDC_WriteHandleBase {
friend class IdentifiableContainerMT<T>;
IdentifiableContainerMT<T> *m_IDC_ptr;
IdentifierHash m_hashId;
- bool m_alreadyPresent;
public:
- IDC_WriteHandle() : IDC_WriteHandleBase(), m_IDC_ptr(0), m_hashId(0), m_alreadyPresent(false) { }
+ IDC_WriteHandle() : IDC_WriteHandleBase(), m_IDC_ptr(0), m_hashId(0) { }
IDC_WriteHandle& operator=(const IDC_WriteHandle& other) = delete;
IDC_WriteHandle(const IDC_WriteHandle& other) = delete;
- IDC_WriteHandle& operator=(IDC_WriteHandle&& other) noexcept{
- Swap(*this, other);
- return *this;
- }
- void Swap(IDC_WriteHandle& a, IDC_WriteHandle& b) noexcept {
+ IDC_WriteHandle& operator=(IDC_WriteHandle&& other) noexcept = delete;
+ static void Swap(IDC_WriteHandle& a, IDC_WriteHandle& b) noexcept {
if(&a == &b) return;
std::swap(a.m_IDC_ptr, b.m_IDC_ptr);
std::swap(a.m_hashId, b.m_hashId);
- std::swap(a.m_alreadyPresent, b.m_alreadyPresent);
std::swap(a.m_atomic, b.m_atomic);
std::swap(a.m_mut, b.m_mut);
}
@@ -69,10 +55,11 @@ public:
if(ATH_UNLIKELY(m_IDC_ptr==nullptr)) return StatusCode::FAILURE;
StatusCode sc = m_IDC_ptr->addLock(std::move(ptr), m_hashId);
IDC_WriteHandleBase::ReleaseLock();
- m_alreadyPresent = true;
return sc;
}
- bool alreadyPresent() const { return m_alreadyPresent; }
+ bool alreadyPresent() { return m_IDC_ptr->m_link->tryAddFromCache(m_hashId, *this); }
+ ///This method is to avoid calling an expensive operation in the offline case
+ bool OnlineAndPresentInAnotherView() { return m_IDC_ptr->m_OnlineMode && m_IDC_ptr->m_link->tryAddFromCache(m_hashId, *this); }
};
@@ -89,16 +76,13 @@ public:
typedef T base_value_type;
- ICACHE* castCache() { return static_cast<ICACHE*>(m_cacheLink); }
- ICACHE* castCache() const { return static_cast<ICACHE*>(m_cacheLink); }
-
class const_iterator
{
public:
/// iterator constructor
- const_iterator() : m_sptr(), m_current(nullptr), m_hash(0), m_idContainer(nullptr), m_end(false) { }
+ const_iterator() : m_itr() {}
const_iterator(const_iterator&&) = default;
const_iterator(const const_iterator&) = default;
const_iterator& operator = ( const const_iterator & ) = default; //move operators may be useful for shared_ptr
@@ -108,41 +92,8 @@ public:
/// increment operator
const_iterator& operator ++ () {
- if(m_end) return *this;
- //If called on iterator created by "fast" iterator method const_iterator( const MyType* idC, IdentifierHash hash )
- if(!m_sptr) {
- auto ids = m_idContainer->GetAllCurrentHashes();
- m_sptr = std::shared_ptr<Hash_Container> (new Hash_Container());
- m_sptr->swap(ids);
- m_hashItr = IdentifiableContainerBase::findHash(m_sptr.get(), m_hash);
- if(m_hashItr==m_sptr->end()) {
- m_end = true;
- m_sptr.reset();
- return *this;
- }
- }
-
-#ifdef IdentifiableCacheBaseRemove
- do {
- ++m_hashItr;
- if(m_hashItr==m_sptr->end()) {
- m_end = true;
- m_sptr.reset();
- return *this;
- }
- m_current = m_idContainer->indexFindPtr(*m_hashItr);
- } while(m_current==nullptr); //If we implement remove, this can happen
-
-#else
- ++m_hashItr;
- if(m_hashItr==m_sptr->end()) {
- m_end = true;
- m_sptr.reset();
- return *this;
- }
- m_current = m_idContainer->indexFindPtr(*m_hashItr);
-#endif
- return *this;
+ ++m_itr;
+ return *this;
}
/// increment operator
@@ -153,74 +104,41 @@ public:
}
const T* cptr () const {
- return m_current;
+ return reinterpret_cast<const T*>( m_itr->second );
}
const T* operator * () const {
- return m_current;
+ return reinterpret_cast<const T*>( m_itr->second );
}
const T* operator ->() const { return (operator*()); }
/// comparison operator
bool operator != ( const const_iterator &it ) const {
- if(m_end || it.m_end) return m_end!= it.m_end;
- return m_current!= it.m_current;
+ return m_itr!= it.m_itr;
}
/// comparison operator
bool operator == ( const const_iterator &it ) const {
- if(m_end || it.m_end) return m_end == it.m_end;
- return m_current == it.m_current;
+ return m_itr == it.m_itr;
}
/// hashId of the pointed-to element
//Calling this on an end iterator is undefined behaviour
IdentifierHash hashId() const {
- return !m_sptr ? m_hash : *m_hashItr;
+ return m_itr->first;
}
protected:
friend class IdentifiableContainerMT<T>;
- const_iterator(const MyType* idC, bool end) : m_sptr(), m_current(nullptr), m_hash(0),
- m_idContainer(idC), m_end(end)
- {
- if(!m_end) {
- auto ids = m_idContainer->GetAllCurrentHashes();
- if(ids.empty()) {//For empty containers
- m_end = true;
- } else {
- m_sptr = std::shared_ptr<Hash_Container> (new Hash_Container());
- m_sptr->swap(ids);
- m_hashItr = m_sptr->begin();
- m_current = m_idContainer->indexFindPtr(*m_hashItr);
-#ifdef IdentifiableCacheBaseRemove
- if(m_current==nullptr){ ++(*this); } //If we implement remove, this can happen
-#endif
- }
- }
- }
-
+ const_iterator(EventContainers::I_InternalIDC::InternalConstItr itr) : m_itr(std::move(itr)) {}
- //Alot of existing code creates a lot of iterators but doesn't interator over it
- //This constructor delays creation of hash vector until ++ is called
- const_iterator( const MyType* idC, IdentifierHash hash ) :m_sptr(),
- m_hash(hash), m_idContainer(idC) {
- m_current = m_idContainer->indexFindPtr(hash);
- m_end = m_current==nullptr;
- }
-
- Hash_Container::const_iterator m_hashItr;
- std::shared_ptr<Hash_Container> m_sptr;
- const T* m_current;
- IdentifierHash m_hash;
- const MyType *m_idContainer;
- bool m_end;
+ EventContainers::I_InternalIDC::InternalConstItr m_itr;
};
-
+ friend class IDC_WriteHandle;
/// constructor initializes the collection the hashmax, OFFLINE usages pattern
IdentifiableContainerMT(IdentifierHash hashMax);
@@ -228,7 +146,7 @@ public:
/// constructor initializes with a link to a cache, ONLINE usage pattern
IdentifiableContainerMT(ICACHE *cache);
- ~IdentifiableContainerMT() { if(!m_OnlineMode) delete castCache(); }
+ ~IdentifiableContainerMT() { if(!m_OnlineMode) static_cast<EventContainers::InternalOffline*>(m_link.get())->cleanUp(void_unique_ptr::Deleter<T>::deleter); }
virtual bool hasExternalCache() const override final { return m_OnlineMode; }
@@ -239,8 +157,7 @@ public:
virtual const T* indexFindPtr( IdentifierHash hashId ) const override final;
const_iterator indexFind( IdentifierHash hashId ) const {
- const_iterator it(this, hashId);
- return it;
+ return m_link->indexFind(hashId);
}
/// insert collection into container with id hash
@@ -282,12 +199,12 @@ public:
/// return full size of container
virtual size_t fullSize() const override final {
- return m_mask.size();
+ return m_link->fullSize();
}
///Duplicate of fullSize for backwards compatability
size_t size() const {
- return m_mask.size();
+ return m_link->fullSize();
}
/// return number of collections
@@ -295,6 +212,11 @@ public:
return IdentifiableContainerBase::numberOfCollections();
}
+ const std::vector < std::pair<IdentifierHash::value_type, const T*> >& GetAllHashPtrPair() const{
+ static_assert(sizeof(const T*) == sizeof(const void*) && std::is_pointer<const T*>::value);
+ return reinterpret_cast<const std::vector < std::pair<IdentifierHash::value_type, const T*> >&>
+ (m_link->GetAllHashPtrPair());
+ }
///Returns a collection of all hashes availiable in this IDC.
///If this is an "offline" mode IDC then this is identical to the cache
@@ -306,14 +228,13 @@ public:
/// return const_iterator for first entry
const_iterator begin() const {
- const_iterator it(this, false);
- return it;
+ return const_iterator(m_link->cbegin());
}
/// return const_iterator for end of container
const_iterator end() const {
- return const_iterator(nullptr, true);
+ return const_iterator(m_link->cend());
}
[[nodiscard]] IDC_WriteHandle getWriteHandle(IdentifierHash hash)
@@ -321,7 +242,6 @@ public:
IDC_WriteHandle lock;
lock.m_hashId = hash;
lock.m_IDC_ptr = this;
- lock.m_alreadyPresent = IdentifiableContainerBase::tryAddFromCache(hash, lock);
return lock;
}
};
@@ -331,11 +251,7 @@ template < class T>
T* //Please don't do this we want to get rid of this
IdentifiableContainerMT<T>::removeCollection( IdentifierHash hashId )
{
- using namespace EventContainers;
- const T* ptr = reinterpret_cast<const T*> (castCache()->find (hashId));
- castCache()->remove(hashId);
- m_mask[hashId] = false;
- return const_cast<T*>(ptr);
+ return reinterpret_cast<T*>(m_link->removeCollection(hashId));
}
#endif
@@ -343,13 +259,13 @@ IdentifiableContainerMT<T>::removeCollection( IdentifierHash hashId )
// Constructor for OFFLINE style IDC
template < class T>
-IdentifiableContainerMT<T>::IdentifiableContainerMT(IdentifierHash maxHash) : IdentifiableContainerBase(new ICACHE(maxHash, nullptr), false)
+IdentifiableContainerMT<T>::IdentifiableContainerMT(IdentifierHash maxHash) : IdentifiableContainerBase(maxHash)
{
}
// Constructor for ONLINE style IDC
template < class T>
-IdentifiableContainerMT<T>::IdentifiableContainerMT(ICACHE *cache) : IdentifiableContainerBase(cache, true)
+IdentifiableContainerMT<T>::IdentifiableContainerMT(ICACHE *cache) : IdentifiableContainerBase(cache)
{
}
@@ -360,8 +276,7 @@ template < class T>
const T*
IdentifiableContainerMT<T>::indexFindPtr( IdentifierHash hashId ) const
{
- if((hashId < m_mask.size()) and m_mask[hashId]) return castCache()->findWait(hashId);
- else return nullptr;
+ return reinterpret_cast<const T* > (IdentifiableContainerBase::indexFindPtr(hashId));
}
// insert collection into container with id hash
@@ -370,8 +285,7 @@ StatusCode
IdentifiableContainerMT<T>::addCollection(const T* coll, IdentifierHash hashId)
{
// update m_hashids
- if (ATH_UNLIKELY(! castCache()->add(hashId, coll))) return StatusCode::FAILURE;
- m_mask[hashId] = true;
+ if (ATH_UNLIKELY(! IdentifiableContainerBase::insert(hashId, coll))) return StatusCode::FAILURE;
return StatusCode::SUCCESS;
}
@@ -382,8 +296,7 @@ template < class T>
void
IdentifiableContainerMT<T>::cleanup()
{
- if(m_OnlineMode) { IdentifiableContainerBase::ResetMask(); }
- else { castCache()->clearCache(); }
+ IdentifiableContainerBase::cleanup(void_unique_ptr::Deleter<T>::deleter);
}
template < class T>
@@ -414,18 +327,20 @@ IdentifiableContainerMT<T>::naughtyRetrieve(IdentifierHash hashId, T* &collToRet
{
if(ATH_UNLIKELY(m_OnlineMode)) return StatusCode::FAILURE;//NEVER ALLOW FOR EXTERNAL CACHE
else {
- collToRetrieve = const_cast< T* > (castCache()->find(hashId));//collToRetrieve can be null on success
+ auto p = reinterpret_cast<const T* > (static_cast<EventContainers::InternalOffline*>(m_link.get())->FindIndexPtr(hashId));//collToRetrieve can be null on success
+ collToRetrieve = const_cast<T*>(p);
return StatusCode::SUCCESS;
}
}
template < class T>
StatusCode
-IdentifiableContainerMT<T>::addOrDelete(std::unique_ptr<T> ptr, IdentifierHash hashId)
+IdentifiableContainerMT<T>::addOrDelete(std::unique_ptr<T> uptr, IdentifierHash hashId)
{
- if(ATH_UNLIKELY(hashId >= m_mask.size())) return StatusCode::FAILURE;
- castCache()->add(hashId, std::move(ptr));
- m_mask[hashId] = true; //it wasn't added it is already present therefore mask could be true
+ if(ATH_UNLIKELY(hashId >= m_link->fullSize())) return StatusCode::FAILURE;
+ auto ptr = uptr.release();
+ bool b = IdentifiableContainerBase::insert(hashId, ptr);
+ if(!b) delete ptr;
return StatusCode::SUCCESS;
}
@@ -433,25 +348,18 @@ template < class T>
StatusCode
IdentifiableContainerMT<T>::addLock(std::unique_ptr<T> ptr, IdentifierHash hashId)
{
- if(ATH_UNLIKELY(hashId >= m_mask.size())) return StatusCode::FAILURE;
- [[maybe_unused]] bool deleted = !castCache()->addLock(hashId, std::move(ptr));
-#ifndef NDEBUG
- if(deleted){
- std::cout << "IDC WARNING Deletion shouldn't occur in addLock paradigm" << std::endl;
- }
-#endif
- m_mask[hashId] = true; //it wasn't added it is already present therefore mask could be true
- return StatusCode::SUCCESS;
+ return m_link->addLock(hashId, ptr.release());
}
template < class T>
StatusCode
-IdentifiableContainerMT<T>::addOrDelete(std::unique_ptr<T> ptr, IdentifierHash hashId, bool &deleted)
+IdentifiableContainerMT<T>::addOrDelete(std::unique_ptr<T> uptr, IdentifierHash hashId, bool &deleted)
{
- if(ATH_UNLIKELY(hashId >= m_mask.size())) return StatusCode::FAILURE;
- bool added = castCache()->add(hashId, std::move(ptr));
- m_mask[hashId] = true;
- deleted = !added;
+ if(ATH_UNLIKELY(hashId >= m_link->fullSize())) return StatusCode::FAILURE;
+ auto ptr = uptr.release();
+ bool b = IdentifiableContainerBase::insert(hashId, ptr);
+ if(!b) delete ptr;
+ deleted = !b;
return StatusCode::SUCCESS;
}
diff --git a/Event/EventContainers/EventContainers/InternalOffline.h b/Event/EventContainers/EventContainers/InternalOffline.h
new file mode 100644
index 00000000000..1bb03226ce2
--- /dev/null
+++ b/Event/EventContainers/EventContainers/InternalOffline.h
@@ -0,0 +1,45 @@
+/*
+ Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
+*/
+
+#ifndef EVENTCONTAINERS_INTERNALOFFLINE_H
+#define EVENTCONTAINERS_INTERNALOFFLINE_H
+#include "EventContainers/I_InternalIDC.h"
+
+namespace EventContainers{
+/*
+This class implements the IdentifiableContainer code for the offline case.
+To avoid excess memory usage just the key-value pair is stored and can be directly
+iterated upon.
+
+Faster consecutive iteration, slower random access.
+Prefer iteration over FindIndexPtr and indexFind
+*/
+class InternalOffline final : public I_InternalIDC {
+public:
+ InternalOffline(size_t max);
+ virtual ~InternalOffline()=default;
+ virtual InternalConstItr cbegin() const override;
+ virtual InternalConstItr cend() const override;
+ virtual InternalConstItr indexFind( IdentifierHash hashId ) const override;
+ virtual const std::vector < hashPair >& GetAllHashPtrPair() const override;
+ std::vector<std::pair<IdentifierHash::value_type, const void*>> m_map;
+ size_t m_maximumSize;
+ virtual bool tryAddFromCache(IdentifierHash hashId, EventContainers::IDC_WriteHandleBase &lock) override;
+ virtual bool tryAddFromCache(IdentifierHash hashId) override;
+ virtual void Wait() const override;
+ virtual std::vector<IdentifierHash> GetAllCurrentHashes() const override;
+ virtual size_t numberOfCollections() const override;
+ void cleanUp(deleter_f* deleter) noexcept;
+ virtual size_t fullSize() const noexcept override {return m_maximumSize;}
+ virtual StatusCode fetchOrCreate(IdentifierHash hashId) override;
+ virtual StatusCode fetchOrCreate(const std::vector<IdentifierHash> &hashIds) override;
+ virtual bool insert(IdentifierHash hashId, const void* ptr) override;
+ virtual const void* FindIndexPtr(IdentifierHash hashId) const noexcept override;
+ virtual StatusCode addLock(IdentifierHash hashId, const void* ptr) override;
+ virtual void* removeCollection( IdentifierHash hashId ) override;
+
+};
+
+}
+#endif
diff --git a/Event/EventContainers/EventContainers/InternalOnline.h b/Event/EventContainers/EventContainers/InternalOnline.h
new file mode 100644
index 00000000000..eb78185ff9c
--- /dev/null
+++ b/Event/EventContainers/EventContainers/InternalOnline.h
@@ -0,0 +1,57 @@
+/*
+ Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
+*/
+
+#ifndef EVENTCONTAINERS_INTERNALONLINE_H
+#define EVENTCONTAINERS_INTERNALONLINE_H
+
+#include "EventContainers/I_InternalIDC.h"
+#include "CxxUtils/checker_macros.h"
+#include <atomic>
+#include <mutex>
+
+namespace EventContainers{
+class IdentifiableCacheBase;
+/*
+The online implementation keeps a pointer to an external cache which owns the collections
+and can be filled concurrently. The container itself maintains a bit mask to identify which
+collections can be accessed from this container. It also maintains a wait list which records
+uncompleted objects that were added to the container. When a complete collection is
+requested the wait-list is processed, locking the thread until all elements are completed.
+The intention of this is to delay thread locking until the last possible moment and thus
+hopefully avoid it altogether.
+Random access is fast since it takes a direct index to the external cache element.
+Ordered iteration is also fast after the lazily initialised collection is constructed.
+*/
+class InternalOnline final : public I_InternalIDC {
+public:
+ InternalOnline(EventContainers::IdentifiableCacheBase *cache);
+ virtual InternalConstItr cbegin() const override;
+ virtual InternalConstItr cend() const override;
+ virtual InternalConstItr indexFind( IdentifierHash hashId ) const override;
+ virtual const std::vector < hashPair >& GetAllHashPtrPair() const override;
+ virtual ~InternalOnline()=default;
+ virtual void Wait() const override;
+ virtual bool tryAddFromCache(IdentifierHash hashId, EventContainers::IDC_WriteHandleBase &lock) override;
+ virtual bool tryAddFromCache(IdentifierHash hashId) override;
+ virtual std::vector<IdentifierHash> GetAllCurrentHashes() const override;
+ virtual size_t numberOfCollections() const override;
+ void ResetMask();
+ virtual size_t fullSize() const noexcept override { return m_mask.size(); }
+ virtual StatusCode fetchOrCreate(IdentifierHash hashId) override;
+ virtual StatusCode fetchOrCreate(const std::vector<IdentifierHash> &hashIds) override;
+ virtual bool insert(IdentifierHash hashId, const void* ptr) override;
+ virtual const void* FindIndexPtr(IdentifierHash hashId) const noexcept override;
+ virtual StatusCode addLock(IdentifierHash hashId, const void* ptr) override;
+ virtual void* removeCollection( IdentifierHash hashId ) override;
+private:
+ EventContainers::IdentifiableCacheBase *m_cacheLink;
+ mutable std::vector< IdentifierHash > m_waitlist ATLAS_THREAD_SAFE;
+ mutable std::vector<std::pair<IdentifierHash::value_type, const void*>> m_map ATLAS_THREAD_SAFE;
+ mutable std::mutex m_waitMutex ATLAS_THREAD_SAFE;
+ mutable std::vector<bool> m_mask ATLAS_THREAD_SAFE;
+ mutable std::atomic<bool> m_waitNeeded ATLAS_THREAD_SAFE; //These mutables are carefully thought out, do not change
+};
+
+}
+#endif
diff --git a/Event/EventContainers/EventContainers/SelectAllObjectMT.h b/Event/EventContainers/EventContainers/SelectAllObjectMT.h
index 42a25c0f7c5..3adb448a5c1 100644
--- a/Event/EventContainers/EventContainers/SelectAllObjectMT.h
+++ b/Event/EventContainers/EventContainers/SelectAllObjectMT.h
@@ -15,11 +15,12 @@ public:
// typedef OBJECT OBJECT;
typedef SelectAllObjectMT<DCC,OBJECT> MyType;
typedef typename DC::const_iterator Object_const_iterator;
- typedef typename DCC::Hash_Container::const_iterator Hash_iterator;
+ typedef typename DCC::const_iterator Cont_iterator;
+// typedef typename DCC::Hash_Container::const_iterator Hash_iterator;
class const_iterator
{
- Hash_iterator m_hash_itr;
+ Cont_iterator m_cont_itr;
const SelectAllObjectMT<DCC,OBJECT>* m_Parent;
Object_const_iterator m_digit_it;
const DC* m_dc; // cache DC
@@ -29,12 +30,9 @@ public:
SKIPNEXT:
- ++m_hash_itr;
- if(m_hash_itr != m_Parent->m_hashes.cend()) {
- m_dc = m_Parent->m_dcc->indexFindPtr(*m_hash_itr);
-#ifdef IdentifiableCacheBaseRemove
- if(m_dc == nullptr) goto SKIPNEXT;//In case we implement remove
-#endif
+ ++m_cont_itr;
+ if(m_cont_itr != m_Parent->m_dcc->end()) {
+ m_dc = *m_cont_itr;
m_digit_it = m_dc->begin();
if(m_digit_it == m_dc->end()) goto SKIPNEXT;
} else {
@@ -47,16 +45,16 @@ SKIPNEXT:
const_iterator()
:
- m_hash_itr(),
+ m_cont_itr(),
m_Parent(nullptr),
m_digit_it(),
m_dc(nullptr)
{}
- const_iterator(const Hash_iterator &itr, const SelectAllObjectMT<DCC,OBJECT>* parent,
+ const_iterator(const Cont_iterator &itr, const SelectAllObjectMT<DCC,OBJECT>* parent,
Object_const_iterator digitit, const DC* dc )
:
- m_hash_itr(itr),
+ m_cont_itr(itr),
m_Parent(parent),
m_digit_it(digitit),
m_dc(dc)
@@ -94,7 +92,7 @@ SKIPNEXT:
bool operator != ( const const_iterator it ) const
{
- if( it.m_hash_itr != m_hash_itr || it.m_digit_it != m_digit_it )
+ if( it.m_cont_itr != m_cont_itr || it.m_digit_it != m_digit_it )
return true;
return false;
@@ -102,25 +100,25 @@ SKIPNEXT:
bool operator == ( const const_iterator it ) const
{
- return( it.m_hash_itr == m_hash_itr && it.m_digit_it == m_digit_it );
+ return( it.m_cont_itr == m_cont_itr && it.m_digit_it == m_digit_it );
}
};
- SelectAllObjectMT(const DCC* dcc) : m_dcc(dcc), m_hashes(dcc->GetAllCurrentHashes())
+ SelectAllObjectMT(const DCC* dcc) : m_dcc(dcc)
{ }
SelectAllObjectMT( ) = delete;
const_iterator begin() {
- Hash_iterator b = m_hashes.cbegin();
+ auto b = m_dcc->begin();
NEXT:
const DC* dc = nullptr;
Object_const_iterator digit_it;
- if(b!= m_hashes.cend()) {
- dc = m_dcc->indexFindPtr(*b);
+ if(b!= m_dcc->end()) {
+ dc = *b;
digit_it = dc->begin();
if(digit_it == dc->end()){
++b;
@@ -131,7 +129,7 @@ NEXT:
}
const_iterator end() {
- Hash_iterator b = m_hashes.cend();
+ auto b = m_dcc->end();
const DC* dc = nullptr;
Object_const_iterator digit_it;
return const_iterator(b, this, digit_it, dc);
@@ -149,7 +147,6 @@ NEXT:
private:
// pointer to the container it is processing.
const DCC* m_dcc;
- const typename DCC::Hash_Container m_hashes;
};
diff --git a/Event/EventContainers/EventContainers/deleter.h b/Event/EventContainers/EventContainers/deleter.h
new file mode 100644
index 00000000000..6d43c66ed56
--- /dev/null
+++ b/Event/EventContainers/EventContainers/deleter.h
@@ -0,0 +1,37 @@
+#ifndef EVENTCONTAINERS_DELETER_H
+//#define EVENTCONTAINERS_DELETER_H
+
+#if 0
+ struct deleter
+ {
+ void operator() (const void* p);
+ };
+#endif
+
+
+ typedef void deleter_f (const void* p);
+
+ class void_unique_ptr
+ : public std::unique_ptr<const void, deleter_f*>
+ {
+ public:
+ using std::unique_ptr<const void, deleter_f*>::unique_ptr;
+
+ template <class T>
+ struct Deleter
+ {
+ static void deleter (const void* p)
+ {
+ delete reinterpret_cast<const T*>(p);
+ }
+ };
+
+ template <class T>
+ void_unique_ptr(std::unique_ptr<T> p)
+ : std::unique_ptr<const void, deleter_f*> (p.release(),
+ Deleter<T>::deleter)
+ {
+ }
+ };
+
+#endif
diff --git a/Event/EventContainers/share/IDBenchTest.ref b/Event/EventContainers/share/IDBenchTest.ref
new file mode 100644
index 00000000000..bc19b99b74c
--- /dev/null
+++ b/Event/EventContainers/share/IDBenchTest.ref
@@ -0,0 +1 @@
+Test Successful
diff --git a/Event/EventContainers/share/IDStressTest.ref b/Event/EventContainers/share/IDStressTest.ref
index ddc13f633d9..252a5f9afba 100644
--- a/Event/EventContainers/share/IDStressTest.ref
+++ b/Event/EventContainers/share/IDStressTest.ref
@@ -14,10 +14,11 @@ range 450 to 1450
30/50
40/50
NoLock
-deleted 163825
-countHit 256975
+deleted 144501
+countHit 276299
fills 500000
reads 492300
+itreads 492300
aborted 7700
0/50
10/50
@@ -29,8 +30,9 @@ deleted 0
countHit 427500
fills 500000
reads 492300
+itreads 492300
aborted 7700
Total elements 500000
MyDigits left undeleted 0
-no lock time 2.60611
-lock time 1.12183
+no lock time 3.7096
+lock time 0.983746
diff --git a/Event/EventContainers/src/IDC_WriteHandleBase.cxx b/Event/EventContainers/src/IDC_WriteHandleBase.cxx
index f555cbc4669..83710778c12 100644
--- a/Event/EventContainers/src/IDC_WriteHandleBase.cxx
+++ b/Event/EventContainers/src/IDC_WriteHandleBase.cxx
@@ -23,7 +23,6 @@ void IDC_WriteHandleBase::ReleaseLock(){
//Running code
assert(m_atomic->load() != ABORTstate);
- assert(m_mut->counter >= 0);
lockguard lk(m_mut->mutex);
m_atomic->compare_exchange_strong(waitstate, ABORTstate);
m_mut->condition.notify_all();
diff --git a/Event/EventContainers/src/IdentifiableCacheBase.cxx b/Event/EventContainers/src/IdentifiableCacheBase.cxx
index 3298c38115f..9d1f1130dac 100644
--- a/Event/EventContainers/src/IdentifiableCacheBase.cxx
+++ b/Event/EventContainers/src/IdentifiableCacheBase.cxx
@@ -58,10 +58,6 @@ int IdentifiableCacheBase::tryLock(IdentifierHash hash, IDC_WriteHandleBase &loc
if(ptr1 == INVALID){
//Second call while not finished
-#ifndef NDEBUG
- size_t slot = hash % m_NMutexes;
- m_HoldingMutexes[slot].counter++;
-#endif
wait.emplace_back(hash);
return 1;
}
@@ -84,12 +80,6 @@ void IdentifiableCacheBase::clear (deleter_f* deleter)
}else{
for (size_t i=0; i<s ;i++) m_vec[i].store(nullptr, std::memory_order_relaxed);//Need to clear incase of aborts
}
-#ifndef NDEBUG
- for(size_t i =0; i<m_NMutexes; i++){
- if(m_HoldingMutexes[i].counter!=0) std::cout << " counter is " << m_HoldingMutexes[i].counter << std::endl;
- assert(m_HoldingMutexes[i].counter==0);
- }
-#endif
}
@@ -103,12 +93,6 @@ void IdentifiableCacheBase::cleanUp (deleter_f* deleter)
if(p && p < ABORTED) deleter (p);
}
}
-#ifndef NDEBUG
- for(size_t i =0; i<m_NMutexes; i++){
- if(m_HoldingMutexes[i].counter!=0) std::cout << "IdentifiableCacheBase counter is " << m_HoldingMutexes[i].counter << std::endl;
- assert(m_HoldingMutexes[i].counter==0);
- }
-#endif
}
int IdentifiableCacheBase::itemAborted (IdentifierHash hash){
@@ -146,13 +130,7 @@ const void* IdentifiableCacheBase::waitFor(IdentifierHash hash)
}
return item;
}
-#ifndef NDEBUG
-void IdentifiableCacheBase::cancelWait(IdentifierHash hash){
- assert(m_NMutexes > 0);
- size_t slot = hash % m_NMutexes;
- m_HoldingMutexes[slot].counter--;
-}
-#endif
+
const void* IdentifiableCacheBase::findWait (IdentifierHash hash)
{
if (ATH_UNLIKELY(hash >= m_vec.size())) return nullptr;
@@ -248,56 +226,56 @@ std::vector<IdentifierHash> IdentifiableCacheBase::ids()
}
-bool IdentifiableCacheBase::add (IdentifierHash hash, const void* p) noexcept
+std::pair<bool, const void*> IdentifiableCacheBase::add (IdentifierHash hash, const void* p) noexcept
{
- if (ATH_UNLIKELY(hash >= m_vec.size())) return false;
- if(p==nullptr) return false;
+ if (ATH_UNLIKELY(hash >= m_vec.size())) return std::make_pair(false, nullptr);
+ if(p==nullptr) return std::make_pair(false, nullptr);
const void* nul=nullptr;
if(m_vec[hash].compare_exchange_strong(nul, p)){
m_currentHashes++;
- return true;
+ return std::make_pair(true, p);
}
const void* invalid = INVALID;
if(m_vec[hash].compare_exchange_strong(invalid, p)){
m_currentHashes++;
- return true;
+ return std::make_pair(true, p);
}
- return false;
+ return std::make_pair(false, invalid);
}
-bool IdentifiableCacheBase::addLock (IdentifierHash hash, const void* p) noexcept
+std::pair<bool, const void*> IdentifiableCacheBase::addLock (IdentifierHash hash, const void* p) noexcept
{ //Same as method above except we check for invalid state first,
// more optimal for calling using writehandle lock method
assert(hash < m_vec.size());
- if(p==nullptr) return false;
+ if(p==nullptr) return std::make_pair(false, nullptr);
const void* invalid = INVALID;
if(m_vec[hash].compare_exchange_strong(invalid, p)){
m_currentHashes++;
- return true;
+ return std::make_pair(true, p);
}
const void* nul=nullptr;
if(m_vec[hash].compare_exchange_strong(nul, p)){
m_currentHashes++;
- return true;
+ return std::make_pair(true, p);
}
- return false;
+ return std::make_pair(false, nul);
}
-bool IdentifiableCacheBase::addLock (IdentifierHash hash,
+std::pair<bool, const void*> IdentifiableCacheBase::addLock (IdentifierHash hash,
void_unique_ptr p) noexcept
{
- bool b = addLock(hash, p.get());
- if(b) p.release();
+ std::pair<bool, const void*> b = addLock(hash, p.get());
+ if(b.first) p.release();
return b;
}
-bool IdentifiableCacheBase::add (IdentifierHash hash,
+std::pair<bool, const void*> IdentifiableCacheBase::add (IdentifierHash hash,
void_unique_ptr p) noexcept
{
- bool b = add(hash, p.get());
- if(b) p.release();
+ std::pair<bool, const void*> b = add(hash, p.get());
+ if(b.first) p.release();
return b;
}
diff --git a/Event/EventContainers/src/IdentifiableContainerBase.cxx b/Event/EventContainers/src/IdentifiableContainerBase.cxx
index da1eb7af930..b01279096f3 100644
--- a/Event/EventContainers/src/IdentifiableContainerBase.cxx
+++ b/Event/EventContainers/src/IdentifiableContainerBase.cxx
@@ -5,106 +5,24 @@
#include <algorithm>
#include "EventContainers/IDC_WriteHandleBase.h"
#include "CxxUtils/AthUnlikelyMacros.h"
+#include "EventContainers/InternalOnline.h"
+#include "EventContainers/InternalOffline.h"
- IdentifiableContainerBase::IdentifiableContainerBase(EventContainers::IdentifiableCacheBase *cache, bool online)
- {
- m_cacheLink = cache;
- m_OnlineMode = online;
- m_mask.assign(cache->fullSize(), !online);
- m_waitNeeded.store(false, std::memory_order_relaxed);
- }
-
- IdentifiableContainerBase::Hash_Container::const_iterator IdentifiableContainerBase::findHash(IdentifiableContainerBase::Hash_Container* container, IdentifierHash hash){
- return std::find(container->begin(), container->end(), hash);
- }
-
- IdentifiableContainerBase::~IdentifiableContainerBase(){
-#ifndef NDEBUG
- //Deregister unused interest
- while(!m_waitlist.empty()){
- IdentifierHash hash = m_waitlist.back();
- m_cacheLink->cancelWait(hash);
- m_waitlist.pop_back();
- }
-#endif
- }
-
- void IdentifiableContainerBase::Wait() const{
- //lockguard to protect m_waitlist from multiple wait calls
- typedef std::lock_guard<decltype(m_waitMutex)> lockguard;
- lockguard lock(m_waitMutex);
- const void* ABORTstate = reinterpret_cast<const void*>(IdentifiableCacheBase::ABORTEDflag);
- while(!m_waitlist.empty()){
- IdentifierHash hash = m_waitlist.back();
- const void* ptr = m_cacheLink->waitFor(hash);
-#ifndef NDEBUG
- m_cacheLink->cancelWait(hash);
-#endif
- if(ptr == ABORTstate) m_mask[hash] = false;//reset flag
- m_waitlist.pop_back();
-
- }
- m_waitNeeded.store(false);
- }
+using namespace EventContainers;
- bool IdentifiableContainerBase::tryAddFromCache(IdentifierHash hashId, EventContainers::IDC_WriteHandleBase &lock)
+ IdentifiableContainerBase::IdentifiableContainerBase(EventContainers::IdentifiableCacheBase *cache)
{
- if(!m_OnlineMode){
- return tryAddFromCache(hashId);//No point calling expensive lock method
- }
- int flag = m_cacheLink->tryLock(hashId, lock, m_waitlist);
- //Relaxed since this should not be running in threaded situation.
- if(!m_waitlist.empty()) m_waitNeeded.store(true, std::memory_order_relaxed);
- if(flag > 0) {
- m_mask[hashId] = flag!=3;
- return true;
- }
- return false;
+ m_link = std::make_unique<EventContainers::InternalOnline>(cache);
+ m_OnlineMode = true;
}
- bool IdentifiableContainerBase::tryAddFromCache(IdentifierHash hashId)
- {
- auto ptr = m_cacheLink->find(hashId);
- if(ptr==nullptr){
- return false;
- }
- m_mask[hashId] = true;
- return true;
+ IdentifiableContainerBase::IdentifiableContainerBase(size_t max){
+ m_OnlineMode = false;
+ m_link = std::make_unique<EventContainers::InternalOffline>(max);
}
- std::vector<IdentifierHash> IdentifiableContainerBase::GetAllCurrentHashes() const {
- if(not m_OnlineMode) return m_cacheLink->ids();
- else{
- if(m_waitNeeded) Wait();
- std::vector<IdentifierHash> ids;
- for(size_t i =0 ; i < m_mask.size(); ++i) if(m_mask[i]) ids.emplace_back(i);
- return ids;
- }
+ void IdentifiableContainerBase::cleanup(deleter_f* deleter){
+ if(m_OnlineMode) throw std::runtime_error("Not implemented in online mode");
+ static_cast<EventContainers::InternalOffline*>(m_link.get())->cleanUp(deleter);
}
-
- size_t IdentifiableContainerBase::numberOfCollections() const{
- if(!m_OnlineMode) return m_cacheLink->numberOfHashes();
- if(m_waitNeeded) Wait();
- size_t count =std::accumulate(m_mask.begin(), m_mask.end(), 0);
- return count;
- }
-
-
- void IdentifiableContainerBase::ResetMask(){
- m_mask.assign(m_cacheLink->fullSize(), false);
- }
-
- StatusCode IdentifiableContainerBase::fetchOrCreate(IdentifierHash hashId){
- if(ATH_UNLIKELY(!m_cacheLink->IMakerPresent())) return StatusCode::FAILURE;
- auto ptr = m_cacheLink->get(hashId);
- m_mask[hashId] = ptr !=nullptr;
- return StatusCode::SUCCESS;
- }
-
- StatusCode IdentifiableContainerBase::fetchOrCreate(const std::vector<IdentifierHash> &hashIds){
- if(ATH_UNLIKELY(!m_cacheLink->IMakerPresent())) return StatusCode::FAILURE;
- m_cacheLink->createSet(hashIds, m_mask);
- return StatusCode::SUCCESS;
- }
-
diff --git a/Event/EventContainers/src/InternalOffline.cxx b/Event/EventContainers/src/InternalOffline.cxx
new file mode 100644
index 00000000000..2eb5bb12e35
--- /dev/null
+++ b/Event/EventContainers/src/InternalOffline.cxx
@@ -0,0 +1,110 @@
+/*
+ Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
+*/
+
+#include "EventContainers/InternalOffline.h"
+#include <algorithm>
+#include "EventContainers/IDC_WriteHandleBase.h"
+
+using namespace EventContainers;
+typedef I_InternalIDC::InternalConstItr InternalConstItr;
+InternalOffline::InternalOffline(size_t max) : m_maximumSize(max) {}
+
+
+bool InternalOffline::tryAddFromCache(IdentifierHash hash, EventContainers::IDC_WriteHandleBase&) {
+ return indexFind(hash) != m_map.cend();//No Cache in offline mode
+}
+
+bool InternalOffline::tryAddFromCache(IdentifierHash hash)
+{
+ return indexFind(hash) != m_map.cend();//No Cache in offline mode
+}
+
+void InternalOffline::Wait() const {
+ //No need to wait in offline, deliberately empty method
+}
+
+std::vector<IdentifierHash> InternalOffline::GetAllCurrentHashes() const {
+ std::vector<IdentifierHash> ids;
+ ids.reserve(m_map.size());
+ for(auto &x : m_map) {
+ ids.emplace_back(x.first);
+ }
+ return ids;
+}
+
+InternalConstItr
+ InternalOffline::cend() const {
+ return m_map.cend();
+}
+
+const std::vector < I_InternalIDC::hashPair >& InternalOffline::GetAllHashPtrPair() const{ return m_map; }
+
+InternalConstItr
+ InternalOffline::cbegin() const {
+ return m_map.cbegin();
+}
+
+InternalConstItr InternalOffline::indexFind( IdentifierHash hashId ) const{
+ auto itr = std::lower_bound( m_map.cbegin(), m_map.cend(), hashId.value(), [](const hashPair &lhs, IdentifierHash::value_type rhs) -> bool { return lhs.first < rhs; } );
+ if(itr!= m_map.cend() && itr->first==hashId) return itr;
+ return m_map.cend();
+// return std::find_if(m_map.begin(), m_map.end(), [hashId](const hashPair &lhs) -> bool { return lhs.first == hashId; });
+}
+
+size_t InternalOffline::numberOfCollections() const {
+ return m_map.size();
+}
+
+void InternalOffline::cleanUp(deleter_f* deleter) noexcept {
+ for(const auto& x : m_map) deleter(x.second);
+ m_map.clear();
+}
+
+bool InternalOffline::insert(IdentifierHash hashId, const void* ptr) {
+ if(m_map.empty() || m_map.back().first < hashId){
+ m_map.emplace_back(hashId, ptr);
+ return true;
+ }
+ auto itr = std::lower_bound( m_map.begin(), m_map.end(), hashId.value(), [](const hashPair &lhs, IdentifierHash::value_type rhs) -> bool { return lhs.first < rhs; } );
+ if(itr == std::end(m_map) || itr->first != hashId)
+ {
+ m_map.emplace(itr, hashId, ptr);
+ return true;
+ }
+ return false;
+}
+
+const void* InternalOffline::FindIndexPtr(IdentifierHash hashId) const noexcept{
+ auto itr = indexFind(hashId);
+ if(itr != m_map.end()) return itr->second;
+ return nullptr;
+}
+
+StatusCode InternalOffline::addLock(IdentifierHash hashId, const void* ptr) {
+ bool added = insert(hashId, ptr);
+ if(!added) {
+#ifndef NDEBUG
+ std::cout << "IDC WARNING Deletion shouldn't occur in addLock paradigm" << std::endl;
+#endif
+ return StatusCode::FAILURE;
+ }
+ return StatusCode::SUCCESS;
+}
+
+void* InternalOffline::removeCollection( IdentifierHash hashId ) {
+ auto itr = std::lower_bound( m_map.begin(), m_map.end(), hashId.value(), [](hashPair &lhs, IdentifierHash::value_type rhs) -> bool { return lhs.first < rhs; } );
+ if(itr== m_map.end() || itr->first!=hashId) return nullptr;
+ void* ptr = const_cast< void* > (itr->second);
+ m_map.erase(itr);
+ return ptr;
+}
+
+StatusCode InternalOffline::fetchOrCreate(IdentifierHash) {
+ throw std::runtime_error("Not implemented in offline mode");
+}
+StatusCode InternalOffline::fetchOrCreate(const std::vector<IdentifierHash>&)
+{
+ throw std::runtime_error("Not implemented in offline mode");
+}
+
diff --git a/Event/EventContainers/src/InternalOnline.cxx b/Event/EventContainers/src/InternalOnline.cxx
new file mode 100644
index 00000000000..7c721d1f003
--- /dev/null
+++ b/Event/EventContainers/src/InternalOnline.cxx
@@ -0,0 +1,154 @@
+/*
+ Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
+*/
+
+#include "EventContainers/InternalOnline.h"
+#include <algorithm>
+#include "EventContainers/IDC_WriteHandleBase.h"
+#include "CxxUtils/AthUnlikelyMacros.h"
+#include "EventContainers/IdentifiableCacheBase.h"
+
+
+using namespace EventContainers;
+typedef I_InternalIDC::InternalConstItr InternalConstItr;
+
+
+InternalOnline::InternalOnline(EventContainers::IdentifiableCacheBase *cache) : m_cacheLink(cache),
+ m_mask(cache->fullSize(), false), m_waitNeeded(false) {}
+
+const std::vector < I_InternalIDC::hashPair >& InternalOnline::GetAllHashPtrPair() const{
+ if(m_waitNeeded) Wait();
+ return m_map;
+}
+
+
+InternalConstItr
+ InternalOnline::cend() const {
+ if(m_waitNeeded) Wait();
+ return m_map.cend();
+}
+
+InternalConstItr
+ InternalOnline::cbegin() const {
+ if(m_waitNeeded) Wait();
+ return m_map.cbegin();
+}
+
+InternalConstItr InternalOnline::indexFind( IdentifierHash hashId ) const{
+ if(m_waitNeeded) Wait();
+ auto itr = std::lower_bound( m_map.begin(), m_map.end(), hashId.value(), [](hashPair &lhs, IdentifierHash::value_type rhs) -> bool { return lhs.first < rhs; } );
+ if(itr!= m_map.end() && itr->first==hashId) return itr;
+ return m_map.end();
+}
+
+void InternalOnline::Wait() const {
+ //lockguard to protect m_waitlist from multiple wait calls
+ std::lock_guard lock (m_waitMutex);
+ if(m_waitNeeded == false) return;
+ using namespace EventContainers;
+ const void* ABORTstate = reinterpret_cast<const void*>(IdentifiableCacheBase::ABORTEDflag);
+ while(!m_waitlist.empty()) {
+ IdentifierHash hash = m_waitlist.back();
+ const void* ptr = m_cacheLink->waitFor(hash);
+ if(ptr == ABORTstate) {
+ m_mask[hash] = false;
+ }
+ m_waitlist.pop_back();
+ }
+ m_map.clear();
+ for(size_t i =0;i<m_mask.size();i++){
+ if(m_mask[i]) m_map.emplace_back(i, m_cacheLink->m_vec[i].load());
+ }
+ m_waitNeeded.store(false);
+}
+
+bool InternalOnline::tryAddFromCache(IdentifierHash hashId, EventContainers::IDC_WriteHandleBase &lock) {
+ int flag = m_cacheLink->tryLock(hashId, lock, m_waitlist);
+ //Relaxed since this should not be running in threaded situation.
+ if(!m_waitlist.empty()) m_waitNeeded.store(true, std::memory_order_relaxed);
+ if(flag > 0) {
+ if(flag!=3){
+ m_mask[hashId] = true;
+ m_waitNeeded.store(true, std::memory_order_relaxed);
+ }
+ return true;
+ }
+ return false;
+}
+
+bool InternalOnline::tryAddFromCache(IdentifierHash hashId)
+{
+ auto ptr = m_cacheLink->find(hashId);
+ if(ptr==nullptr) {
+ return false;
+ }
+ m_mask[hashId] = true;
+ m_waitNeeded.store(true, std::memory_order_relaxed);
+ return true;
+}
+
+std::vector<IdentifierHash> InternalOnline::GetAllCurrentHashes() const {
+ if(m_waitNeeded) Wait();
+ std::vector<IdentifierHash> ids;
+ ids.reserve(m_map.size());
+ for(auto &x : m_map) {
+ ids.emplace_back(x.first);
+ }
+ return ids;
+}
+
+size_t InternalOnline::numberOfCollections() const {
+ if(m_waitNeeded) Wait();
+ return m_map.size();
+}
+
+void InternalOnline::ResetMask() {
+ if(m_waitNeeded) Wait();
+ m_mask.assign(m_cacheLink->fullSize(), false);
+ m_map.clear();
+ m_waitNeeded.store(true, std::memory_order_relaxed);
+}
+
+StatusCode InternalOnline::fetchOrCreate(IdentifierHash hashId) {
+ if(ATH_UNLIKELY(!m_cacheLink->IMakerPresent())) return StatusCode::FAILURE;
+ auto ptr = m_cacheLink->get(hashId);
+ if(ptr) { m_mask[hashId] =true; m_waitNeeded.store(true, std::memory_order_relaxed); }
+ return StatusCode::SUCCESS;
+}
+
+StatusCode InternalOnline::fetchOrCreate(const std::vector<IdentifierHash> &/*hashIds*/) {
+ throw std::runtime_error("Not implemented");
+// if(ATH_UNLIKELY(!m_cacheLink->IMakerPresent())) return StatusCode::FAILURE;
+// m_cacheLink->createSet(hashIds, m_mask);
+// return StatusCode::SUCCESS;
+}
+
+
+bool InternalOnline::insert(IdentifierHash hashId, const void* ptr) {
+ std::pair<bool, const void*> cacheinserted = m_cacheLink->add(hashId, ptr);
+ m_mask[hashId] = true; //it wasn't added it is already present therefore mask could be true
+ m_waitNeeded.store(true, std::memory_order_relaxed);
+ return ptr == cacheinserted.second;
+}
+
+const void* InternalOnline::FindIndexPtr(IdentifierHash hashId) const noexcept {
+ if(hashId < m_mask.size() and m_mask[hashId]) return m_cacheLink->findWait(hashId);
+ return nullptr;
+}
+
+StatusCode InternalOnline::addLock(IdentifierHash hashId, const void* ptr) {
+ if(ATH_UNLIKELY(hashId >= m_mask.size())) return StatusCode::FAILURE;
+ [[maybe_unused]] std::pair<bool, const void*> added = m_cacheLink->addLock(hashId, ptr);
+#ifndef NDEBUG
+ if(!added.first) {
+ std::cout << "IDC WARNING Deletion shouldn't occur in addLock paradigm" << std::endl;
+ }
+#endif
+ m_mask[hashId] = true; //it wasn't added it is already present therefore mask could be true
+ m_waitNeeded.store(true, std::memory_order_relaxed);
+ return StatusCode::SUCCESS;
+}
+
+void* InternalOnline::removeCollection( IdentifierHash ) {
+ throw std::runtime_error("Do not remove things from an online IDC");
+}
diff --git a/Event/EventContainers/test/IDC_Benchmark.cxx b/Event/EventContainers/test/IDC_Benchmark.cxx
new file mode 100644
index 00000000000..5c577317ff2
--- /dev/null
+++ b/Event/EventContainers/test/IDC_Benchmark.cxx
@@ -0,0 +1,71 @@
+/*
+ Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
+*/
+#include "EventContainers/IdentifiableContainerMT.h"
+#include "EventContainers/IdentifiableCache.h"
+#include <chrono>
+
+using namespace EventContainers;
+
+
+template<typename T>
+void timedelete(std::string name, T* ptr){
+ auto start1 = std::chrono::steady_clock::now();
+ delete ptr;
+ auto end1 = std::chrono::steady_clock::now();
+ std::chrono::duration<double> time = end1-start1;
+ std::cout << name << " delete time " << time.count() << std::endl;
+}
+
+void accessTime(std::string name, IdentifiableContainerMT<long unsigned int>& container){
+
+ auto start3 = std::chrono::steady_clock::now();
+ auto offlinecnt = container.GetAllCurrentHashes();
+ for(auto hash : offlinecnt) if(hash != *container.indexFindPtr(hash) ) std::abort();
+ auto end3 = std::chrono::steady_clock::now();
+ std::chrono::duration<double> offlinerandom = end3-start3;
+ std::cout << name << " random time " << offlinerandom.count() << std::endl;
+
+ auto start4 = std::chrono::steady_clock::now();
+ size_t i=0;
+ for(auto hash : container) {if(*hash != offlinecnt[i].value() ) std::abort(); i++; }
+ auto end4 = std::chrono::steady_clock::now();
+ std::chrono::duration<double> offlineit = end4-start4;
+ std::cout << name << " iteration time " << offlineit.count() << std::endl;
+
+ auto start5 = std::chrono::steady_clock::now();
+ const auto& directaccess = container.GetAllHashPtrPair();
+ for(const auto &[hashId, ptr] : directaccess) {if(hashId != *ptr ) std::abort(); i++; }
+ auto end5 = std::chrono::steady_clock::now();
+ std::chrono::duration<double> offlinedir = end5-start5;
+ std::cout << name << " direct time " << offlinedir.count() << std::endl;
+}
+
+int main(){
+
+ auto start1 = std::chrono::steady_clock::now();
+ auto offline = new IdentifiableContainerMT<long unsigned int>(50000);
+ for(size_t i =3;i<50000;i+=3){
+ offline->addCollection(new long unsigned int(i) ,i).ignore();
+ }
+ auto end1 = std::chrono::steady_clock::now();
+ std::chrono::duration<double> offlinefill = end1-start1;
+ std::cout << "offline fill time " << offlinefill.count() << std::endl;
+ auto start2 = std::chrono::steady_clock::now();
+ auto cache = new IdentifiableCache<long unsigned int>(50000, nullptr);
+ auto online = new IdentifiableContainerMT<long unsigned int>(cache);
+
+ for(size_t i =3;i<50000;i+=3){
+ online->addCollection(new long unsigned int(i) , i).ignore();
+ }
+ auto end2 = std::chrono::steady_clock::now();
+ std::chrono::duration<double> onlinefill = end2-start2;
+ std::cout << "online fill time " << onlinefill.count() << std::endl;
+ accessTime("online ", *online);
+ accessTime("offline ", *offline);
+ timedelete("onlineCont ", online);
+ timedelete("onlineCache ", cache);
+ timedelete("offline ", offline);
+ std::cout << "Test Successful" << std::endl;
+ return 0;
+}
diff --git a/Event/EventContainers/test/IDC_Realistic_Test.cxx b/Event/EventContainers/test/IDC_Realistic_Test.cxx
index f84599a82e8..37497319ac0 100644
--- a/Event/EventContainers/test/IDC_Realistic_Test.cxx
+++ b/Event/EventContainers/test/IDC_Realistic_Test.cxx
@@ -134,12 +134,14 @@ struct counters {
int cachehit=0;
int fills =0;
int reads =0;
+ int itreads =0;
int aborted =0;
void Add(const counters &rh) {
deletedcount+= rh.deletedcount;
cachehit+= rh.cachehit;
fills+= rh.fills;
reads+= rh.reads;
+ itreads+= rh.itreads;
aborted += rh.aborted;
}
void Print() const {
@@ -147,6 +149,7 @@ struct counters {
std::cout << "countHit " << cachehit << '\n';
std::cout << "fills " << fills << '\n';
std::cout << "reads " << reads << '\n';
+ std::cout << "itreads " << itreads << '\n';
std::cout << "aborted " << aborted << '\n';
}
counters() {
@@ -154,6 +157,7 @@ struct counters {
cachehit=0;
fills =0;
reads =0;
+ itreads=0;
aborted=0;
}
};
@@ -176,10 +180,16 @@ public:
int wrong = 0;
auto hashes = container.GetAllCurrentHashes();
+ auto countall = container.numberOfCollections();
+ if(hashes.size() != countall){
+ std::cout << "Counts don't match" << std::endl;
+ std::abort();
+ }
if(hashes.size()!=(size_t) 1000-c.aborted) {
std::cout << "Error container is " << hashes.size() << " not " << 1000-c.aborted << std::endl;
std::abort();
}
+ //Check random access method
for(const auto x : hashes) {
auto p = container.indexFindPtr(x);
int j =0;
@@ -192,6 +202,36 @@ public:
}
c.reads++;
}
+ //check iterator method
+ int orig=0;
+ for(auto p : container){
+ int j =0;
+ for(auto q : *p) {
+ if(q->val() != (initialdata[hashes[orig]]->at(j++))->val()) wrong++;
+ }
+ if(j!=ndigits) {
+ std::cout << "n digits wrong"<<std::endl;
+ std::abort();
+ }
+ c.itreads++;
+ orig++;
+ }
+ int orig2=0;
+ const auto& directaccess = container.GetAllHashPtrPair();
+ for(const auto &[hashId, ptr] : directaccess){
+ int j =0;
+ if(hashes[orig2] != hashId){
+ std::cout << "directaccess broke " << std::endl;
+ std::abort();
+ }
+ for(auto q : *ptr) {
+ if(q->val() != (initialdata[hashes[orig2]]->at(j++))->val()) {
+ std::cout << "directaccess broke " << std::endl;
+ std::abort();
+ }
+ }
+ orig2++;
+ }
if(wrong > 0) {
std::cout << "Thread " << threads << " found wrong data " << wrong << std::endl;
std::abort();
@@ -361,7 +401,7 @@ int main() {
}
std::cout << "NoLock\n";
c.Print();
- if(c.fills!=c.reads+c.aborted) {
+ if(c.fills!=c.reads+c.aborted || c.fills!=c.itreads+c.aborted ) {
std::cout << "Fills do not equal reads " << std::endl;
std::abort();
}
@@ -378,7 +418,7 @@ int main() {
}
std::cout << "Lock\n";
c.Print();
- if(c.fills!=c.reads+c.aborted) {
+ if(c.fills!=c.reads+c.aborted || c.fills!=c.itreads+c.aborted ) {
std::cout << "Fills do not equal reads " << std::endl;
std::abort();
}
diff --git a/Event/EventContainers/test/IDMT_ContainerTest.cxx b/Event/EventContainers/test/IDMT_ContainerTest.cxx
index df8ec1d3517..17207fdb4df 100644
--- a/Event/EventContainers/test/IDMT_ContainerTest.cxx
+++ b/Event/EventContainers/test/IDMT_ContainerTest.cxx
@@ -5,7 +5,7 @@
// This is a test cxx file for IdentifiableContainerMT.
//
#include "EventContainers/IdentifiableContainerMT.h"
-#include "EventContainers/SelectAllObjectMT.h"
+#include "EventContainers/SelectAllObject.h"
#include "EventContainers/IdentifiableContTemp.h"
#include "ID_ContainerTest.h"
#include "GaudiKernel/System.h"
@@ -154,7 +154,7 @@ return 0;}
int ID_ContainerTest::execute(){
- typedef SelectAllObjectMT<MyCollectionContainer,MyDigit> SELECTOR ;
+ typedef SelectAllObject<MyCollectionContainer,MyDigit> SELECTOR ;
typedef SELECTOR::const_iterator digit_const_iterator;
// typedef MyCollectionContainer::const_iterator collection_iterator;
@@ -604,14 +604,49 @@ int ID_ContainerTest::execute(){
}
{
MyCollectionContainer::IDC_WriteHandle lock;
- lock = containerOnline->getWriteHandle(IdentifierHash(50));
- lock = containerOnline->getWriteHandle(IdentifierHash(50));//Try to break the locks
- lock = containerOnline->getWriteHandle(IdentifierHash(60));
- lock = containerOnline->getWriteHandle(IdentifierHash(70));
+ MyCollectionContainer::IDC_WriteHandle lock2 = containerOnline->getWriteHandle(IdentifierHash(50));
+ MyCollectionContainer::IDC_WriteHandle::Swap(lock, lock2);
+ //lock = containerOnline->getWriteHandle(IdentifierHash(50));//Try to break the locks
+ //lock = containerOnline->getWriteHandle(IdentifierHash(60));
+ //lock = containerOnline->getWriteHandle(IdentifierHash(70));
}
delete containerOnline;
delete cache;
+
+
+ auto containerordertest = new MyCollectionContainer(500);
+ containerordertest->addCollection(nullptr, 4).ignore();
+ containerordertest->addCollection(nullptr, 3).ignore();
+ containerordertest->addCollection(nullptr, 2).ignore();
+ containerordertest->addCollection(nullptr, 1).ignore();
+ containerordertest->addCollection(nullptr, 5).ignore();
+ containerordertest->addCollection(nullptr, 4).ignore(); //Deliberate duplicate
+ containerordertest->addCollection(nullptr, 7).ignore();
+
+ auto hashes = containerordertest->GetAllCurrentHashes();
+ size_t last =0;
+ for(auto i : hashes){
+ if(last >= i) {
+ std::cout << "Ordering error" <<std::endl;
+ std::abort();
+ }
+ last =i;
+ }
+ last =0;
+ for(auto [hash,ptr] : containerordertest->GetAllHashPtrPair() ){
+ if(last >= hash) {
+ std::cout << "Ordering error" <<std::endl;
+ std::abort();
+ }
+ last =hash;
+ }
+ if(containerordertest->numberOfCollections() != 6){
+ std::cout << "Duplicate got added " << std::endl;
+ std::abort();
+ }
+
+ delete containerordertest;
std::cout << "MyDigits left undeleted " << MyDigit::s_total << std::endl;
}
return 0;
--
GitLab