Skip to content

Commit

Permalink
Re #8634. Add caching of names in FunctionFactory::getFunctionNames.
Browse files Browse the repository at this point in the history
This method was fairly expensive because it had to create an instance of
every registered function every time to find out if it's the requested
type. This method is called to create validators in various algorithm
init methods, which can be called a lot it - for example - the algorithm
is used as a sub-algorithm in a loop within another algorithm.

It's now cached in a map that's a member of FunctionFactoryImpl. The caching
had to be made thread-safe with a mutex. On the plus side this allows the
resulting vector to be returned by reference-to-const.
  • Loading branch information
RussellTaylor committed Dec 18, 2013
1 parent 0651b06 commit 4c089bf
Showing 1 changed file with 22 additions and 10 deletions.
32 changes: 22 additions & 10 deletions Code/Mantid/Framework/API/inc/MantidAPI/FunctionFactory.h
Expand Up @@ -8,9 +8,11 @@
#include "MantidAPI/DllConfig.h"
#include "MantidKernel/DynamicFactory.h"
#include "MantidKernel/SingletonHolder.h"
#include "MantidKernel/MultiThreaded.h"

#include <boost/shared_ptr.hpp>


namespace Mantid
{

Expand Down Expand Up @@ -76,7 +78,7 @@ namespace API

/// Query available functions based on the template type
template<typename FunctionType>
std::vector<std::string> getFunctionNames() const;
const std::vector<std::string>& getFunctionNames() const;

private:
friend struct Mantid::Kernel::CreateUsingNew<FunctionFactoryImpl>;
Expand Down Expand Up @@ -116,9 +118,11 @@ namespace API
/// Add a tie to the created function
void addTie(boost::shared_ptr<IFunction> fun,const Expression& expr)const;

///static reference to the logger class
/// Reference to the logger class
Kernel::Logger& g_log;

mutable std::map<std::string,std::vector<std::string>> m_cachedFunctionNames;
mutable Kernel::Mutex m_mutex;
};

/**
Expand All @@ -127,13 +131,20 @@ namespace API
* @returns A vector of the names of the functions matching the template type
*/
template<typename FunctionType>
std::vector<std::string> FunctionFactoryImpl::getFunctionNames() const
const std::vector<std::string>& FunctionFactoryImpl::getFunctionNames() const
{
Kernel::Mutex::ScopedLock _lock(m_mutex);

const std::string soughtType(typeid(FunctionType).name());
if ( m_cachedFunctionNames.find( soughtType ) != m_cachedFunctionNames.end() )
{
return m_cachedFunctionNames[soughtType];
}

// Create the entry in the cache and work with it directly
std::vector<std::string>& typeNames = m_cachedFunctionNames[soughtType];
const std::vector<std::string> names = this->getKeys();
std::vector<std::string> typeNames;
typeNames.reserve(names.size());
for( std::vector<std::string>::const_iterator it = names.begin();
it != names.end(); ++it )
for( auto it = names.begin(); it != names.end(); ++it )
{
boost::shared_ptr<IFunction> func = this->createFunction(*it);
if ( func && dynamic_cast<FunctionType*>(func.get()) )
Expand All @@ -143,12 +154,13 @@ namespace API
}
return typeNames;
}
///Forward declaration of a specialisation of SingletonHolder for AlgorithmFactoryImpl (needed for dllexport/dllimport) and a typedef for it.

///Forward declaration of a specialisation of SingletonHolder for AlgorithmFactoryImpl (needed for dllexport/dllimport) and a typedef for it.
#ifdef _WIN32
// this breaks new namespace declaraion rules; need to find a better fix
template class MANTID_API_DLL Mantid::Kernel::SingletonHolder<FunctionFactoryImpl>;
template class MANTID_API_DLL Mantid::Kernel::SingletonHolder<FunctionFactoryImpl>;
#endif /* _WIN32 */
typedef MANTID_API_DLL Mantid::Kernel::SingletonHolder<FunctionFactoryImpl> FunctionFactory;
typedef MANTID_API_DLL Mantid::Kernel::SingletonHolder<FunctionFactoryImpl> FunctionFactory;

/// Convenient typedef for an UpdateNotification
typedef FunctionFactoryImpl::UpdateNotification FunctionFactoryUpdateNotification;
Expand Down

0 comments on commit 4c089bf

Please sign in to comment.