Skip to content
Snippets Groups Projects
Commit 61952b5b authored by Benedikt Hegner's avatar Benedikt Hegner
Browse files

More efficient Toolhandle<T>::get() method

I would like to use ToolHandle more, but I also want to use it optimally efficiently, by which I mean I want inline access to the underlying tool pointer, with no validity checks (I always `retrieve()` my tools up front in initialize()) and true const behaviour (for thread safety). Essentially what I want is `GaudiHandle<T>::get()`, but unfortunately in the current implementation this method is hidden behind the virtual `ToolHandle<T>::get()` method, that also down casts the tool to `IAlgTool*` (so no use for direct usage).

This MR fixes both these issues. get() is now fully optimal in `ToolHandle<T>` as it simply refers back to GaudiHandle in an inline method.

The `BaseToolHandle<T>::get()` method is just a wrapper that calls a hidden (protected) virtual function (`getAsIAlgTool()`) that returns what get() previously did.

I also tightened up the const'ness, providing both const and non-const methods that return const and non-const tool pointers respectively.

I also updated a few methods to remove multiple return statements (which normally cause compilers to abort inlining) and adding some `noexcept` here and there..

The only API change is that `ToolHandle<T>::get()` now returns `T*` instead of `IAlgTool*`, but this is not a problem as the user can still choose to use it as a `IAlgTool*` if they want..

Gaudi master builds fine with this, not tested any other projects yet.

cheers Chris

See merge request !218
parents b9f002b7 cad1f2ec
No related branches found
No related tags found
1 merge request!218More efficient Toolhandle<T>::get() method
Pipeline #
......@@ -198,28 +198,27 @@ public:
return *this;
}
~GaudiHandle(){
//release();
}
/** Retrieve the component. Release existing component if needed. */
StatusCode retrieve() const { // not really const, because it updates m_pObject
if ( m_pObject && release().isFailure() ) return StatusCode::FAILURE;
if ( retrieve( m_pObject ).isFailure() ) {
StatusCode sc = StatusCode::SUCCESS;
if ( m_pObject && release().isFailure() ) { sc = StatusCode::FAILURE; }
if ( sc && retrieve( m_pObject ).isFailure() )
{
m_pObject = nullptr;
return StatusCode::FAILURE;
sc = StatusCode::FAILURE;
}
return StatusCode::SUCCESS;
return sc;
}
/** Release the component. */
StatusCode release() const { // not really const, because it updates m_pObject
if ( m_pObject ) {
StatusCode sc = release( m_pObject );
StatusCode sc = StatusCode::SUCCESS;
if ( m_pObject )
{
sc = release( m_pObject );
m_pObject = nullptr;
return sc;
}
return StatusCode::SUCCESS;
return sc;
}
/// Check if the handle is valid (try to retrive the object is not done yet).
......@@ -233,6 +232,9 @@ public:
return isValid();
}
/// Return the wrapped pointer, not calling retrieve() if null.
T * get() { return m_pObject; }
/// Return the wrapped pointer, not calling retrieve() if null.
typename std::add_const<T>::type * get() const {
return m_pObject;
......@@ -510,22 +512,25 @@ public:
return it != end() ? &*it : nullptr;
}
/** Add a handle with given type and name. Can be overridden in derived class.
Return whether addition was successful or not. */
/** Add a handle with given type and name. Can be overridden in derived class.
Return whether addition was successful or not. */
using GaudiHandleArrayBase::push_back; // avoid compiler warning
virtual bool push_back( const T& myHandle ) {
m_handleArray.push_back( myHandle );
return true;
}
/** Retrieve all tools */
StatusCode retrieve() {
for (auto& i : *this) {
// stop at first failure
if ( i.retrieve().isFailure() ) return StatusCode::FAILURE;
StatusCode retrieve()
{
StatusCode sc = StatusCode::SUCCESS;
for ( auto& i : *this )
{
// stop at first failure
if ( i.retrieve().isFailure() ) { sc = StatusCode::FAILURE; break; }
}
m_retrieved = true;
return StatusCode::SUCCESS;
if ( sc ) { m_retrieved = true; }
return sc;
}
/** Release all tools */
......
......@@ -22,8 +22,11 @@ class AlgTool;
class Service;
/** General info and helper functions for toolhandles and arrays */
class ToolHandleInfo {
class ToolHandleInfo
{
protected:
ToolHandleInfo(const IInterface* parent = nullptr, bool createIf = true )
: m_parent(parent), m_createIf(createIf)
{}
......@@ -32,25 +35,25 @@ public:
virtual ~ToolHandleInfo() = default;
bool isPublic() const { return !m_parent; }
bool isPublic() const noexcept { return !m_parent; }
bool createIf() const { return m_createIf; }
bool createIf() const noexcept { return m_createIf; }
const IInterface* parent() const { return m_parent; }
const IInterface* parent() const noexcept { return m_parent; }
//
// Some helper functions
//
static std::string toolComponentType(const IInterface* parent)
static std::string toolComponentType(const IInterface* parent)
{
return parent ? "PrivateTool" : "PublicTool";
}
static std::string toolParentName(const IInterface* parent)
static std::string toolParentName(const IInterface* parent)
{
if (!parent) return "ToolSvc";
const INamedInterface* pNamed = dynamic_cast<const INamedInterface*>(parent);
return pNamed ? pNamed->name() : "";
auto * pNamed = ( parent ? dynamic_cast<const INamedInterface*>(parent) : nullptr );
return ( !parent ? "ToolSvc" : ( pNamed ? pNamed->name() : "" ) );
}
protected:
......@@ -67,9 +70,11 @@ protected:
@author Daniel Funke <daniel.funke@cern.ch>
*/
class BaseToolHandle: public ToolHandleInfo {
class BaseToolHandle: public ToolHandleInfo
{
protected:
BaseToolHandle(const IInterface* parent = nullptr, bool createIf = true )
: ToolHandleInfo(parent, createIf)
{}
......@@ -78,14 +83,22 @@ protected:
public:
virtual ~BaseToolHandle() {}
StatusCode retrieve(IAlgTool*& tool) const {
return i_retrieve(tool);
}
virtual IAlgTool * get() const = 0;
const IAlgTool * get() const { return getAsIAlgTool(); }
IAlgTool * get() { return getAsIAlgTool(); }
virtual std::string typeAndName() const = 0;
protected:
virtual const IAlgTool * getAsIAlgTool() const = 0;
virtual IAlgTool * getAsIAlgTool() = 0;
};
/** @class ToolHandle ToolHandle.h GaudiKernel/ToolHandle.h
......@@ -99,7 +112,8 @@ public:
@author Martin.Woudstra@cern.ch
*/
template< class T >
class ToolHandle : public BaseToolHandle, public GaudiHandle<T> {
class ToolHandle : public BaseToolHandle, public GaudiHandle<T>
{
friend class Algorithm;
friend class AlgTool;
......@@ -198,14 +212,26 @@ public:
return m_pToolSvc->releaseTool( nonConst(algTool) );
}
IAlgTool * get() const override
std::string typeAndName() const override {
return GaudiHandleBase::typeAndName();
}
typename std::add_const<T>::type * get() const { return GaudiHandle<T>::get(); }
T * get() { return GaudiHandle<T>::get(); }
protected:
const IAlgTool * getAsIAlgTool() const override
{
// const cast to support T being const
return nonConst( GaudiHandle<T>::get() );
return GaudiHandle<T>::get();
}
std::string typeAndName() const override {
return GaudiHandleBase::typeAndName();
IAlgTool * getAsIAlgTool() override
{
// const cast to support T being const
return nonConst( GaudiHandle<T>::get() );
}
private:
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment