Ticket #8634 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

Add caching of results in FunctionFactory::getFunctionNames

Reported by: Russell Taylor Owned by: Russell Taylor
Priority: major Milestone: Release 3.1
Component: Framework Keywords:
Cc: Blocked By:
Blocking: Tester: Peter Peterson

Description

This method is quite expensive as it creates an instance of every registered fitting function every time it's called. It's been found to be called from algorithm init methods where those algorithms are called as child algorithms within a loop in another algorithm.

The cache could be that will have a map<string, vector<string>> where the keys will be type_info.name strings and values are names of functions.

Change History

comment:1 Changed 7 years ago by Russell Taylor

  • Status changed from new to inprogress

Re #8634. Add caching of names in FunctionFactory::getFunctionNames.

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.

Changeset: 4c089bf0fa8e5995b6d590485d5c29488791d35e

comment:2 Changed 7 years ago by Russell Taylor

Re #8634. Add some kind of testing for getFunctionNames.

Changeset: 08194357a74b28c0ed12c424b7d649f47bbdd87d

comment:3 Changed 7 years ago by Russell Taylor

Re #8634. Take advantage of reference return type.

Changeset: b74c6ea87f28e06e6015765a4d69461c2fd30a63

comment:4 Changed 7 years ago by Russell Taylor

Re #8634. Clear cache if functions are added or removed after startup.

Can be done from python, and of course it invalidates the cache.

Changeset: 22085b00607f2b715e5e0c132d9112bc3764dbf0

comment:5 Changed 7 years ago by Russell Taylor

Re #8634. Make sure the FrameworkManager is started in tests.

...before the first time getFunctionNames is called. Otherwise the cache will be populated before the functions have been registered.

Changeset: 24d035fb488aa6f2ddfdb41f562eed55ebefd4ef

comment:6 Changed 7 years ago by Russell Taylor

  • Status changed from inprogress to verify
  • Resolution set to fixed

Testing should be largely by inspection, as the external behaviour should be unchanged (which is verified by seeing that all tests are passing).

Not the speed improvement in https://builds.sns.gov/view/Tests/job/mantid_performance_tests_develop/Develop_branch_performance_tests/AlgorithmsTest.GetDetOffsetsMultiPeaksTestPerformance.test_performance.htm, which is what prompted this work.

comment:7 Changed 7 years ago by Peter Peterson

  • Status changed from verify to verifying
  • Tester set to Peter Peterson

comment:8 Changed 7 years ago by Peter Peterson

  • Status changed from verifying to closed

Merge remote branch 'origin/feature/8634_functionfactory_getfunctionnames_caching'

Full changeset: 8aadb09a47b4f6a657c3bac60d5e6ed05310fb81

comment:9 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 9478

Note: See TracTickets for help on using tickets.