Skip to content

Commit

Permalink
Merge pull request #19826 from mantidproject/multi_file_prop_cache
Browse files Browse the repository at this point in the history
Add caching to multiple file property
  • Loading branch information
martyngigg authored Jun 8, 2017
2 parents cdb6870 + 0488b19 commit 0e72709
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 3 deletions.
8 changes: 8 additions & 0 deletions Framework/API/inc/MantidAPI/MultipleFileProperty.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,14 @@ class DLLExport MultipleFileProperty
/// The action type of this property
/// Load (dafault) or OptionalLoad are supported
unsigned int m_action;
/// Last value of propValue used in
/// MultipleFileProperty::setValueAsMultipleFiles
/// and MultipleFileProperty::setValueAsSingleFile
std::string m_oldPropValue;
/// Last value of the found files used in
/// MultipleFileProperty::setValueAsMultipleFiles
/// and MultipleFileProperty::setValueAsSingleFile
std::vector<std::vector<std::string>> m_oldFoundValue;
};

} // namespace API
Expand Down
29 changes: 26 additions & 3 deletions Framework/API/src/MultipleFileProperty.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,13 @@ std::string MultipleFileProperty::getDefault() const {
*/
std::string
MultipleFileProperty::setValueAsSingleFile(const std::string &propValue) {
// if value is unchanged use the cached version
if ((propValue == m_oldPropValue) && (!m_oldFoundValue.empty())) {
PropertyWithValue<std::vector<std::vector<std::string>>>::operator=(
m_oldFoundValue);
return "";
}

// Use a slave FileProperty to do the job for us.
FileProperty slaveFileProp("Slave", "", FileProperty::Load, m_exts,
Direction::Input);
Expand All @@ -190,15 +197,21 @@ MultipleFileProperty::setValueAsSingleFile(const std::string &propValue) {
return error;

// Store.
std::vector<std::vector<std::string>> foundFiles;
try {
std::vector<std::vector<std::string>> result;
toValue(slaveFileProp(), result, "", "");
PropertyWithValue<std::vector<std::vector<std::string>>>::operator=(result);
toValue(slaveFileProp(), foundFiles, "", "");
PropertyWithValue<std::vector<std::vector<std::string>>>::operator=(
foundFiles);
} catch (std::invalid_argument &except) {
g_log.debug() << "Could not set property " << name() << ": "
<< except.what();
return except.what();
}

// cache the new version of things
m_oldPropValue = propValue;
m_oldFoundValue = foundFiles;

return "";
}

Expand All @@ -216,6 +229,12 @@ MultipleFileProperty::setValueAsSingleFile(const std::string &propValue) {
*/
std::string
MultipleFileProperty::setValueAsMultipleFiles(const std::string &propValue) {
// if value is unchanged use the cached version
if ((propValue == m_oldPropValue) && (!m_oldFoundValue.empty())) {
return PropertyWithValue<std::vector<std::vector<std::string>>>::setValue(
toString(m_oldFoundValue));
}

// Return error if there are any adjacent + or , operators.
const std::string INVALID = "\\+\\+|,,|\\+,|,\\+";
boost::smatch invalid_substring;
Expand Down Expand Up @@ -395,6 +414,10 @@ MultipleFileProperty::setValueAsMultipleFiles(const std::string &propValue) {
allFullFileNames.push_back(fullFileNames);
}

// cache the new version of things
m_oldPropValue = propValue;
m_oldFoundValue = allFullFileNames;

// Now re-set the value using the full paths found.
return PropertyWithValue<std::vector<std::vector<std::string>>>::setValue(
toString(allFullFileNames));
Expand Down
5 changes: 5 additions & 0 deletions Framework/API/test/MultipleFilePropertyTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ class MultipleFilePropertyTest : public CxxTest::TestSuite {
std::string m_dirWithWhitespace;
std::unordered_set<std::string> m_tempDirs;
std::vector<std::string> m_exts;
std::string m_oldArchiveSearchSetting;

Mantid::Kernel::ConfigServiceImpl &g_config;

Expand All @@ -97,6 +98,7 @@ class MultipleFilePropertyTest : public CxxTest::TestSuite {
: m_multiFileLoadingSetting(), m_oldDataSearchDirectories(),
m_oldDefaultFacility(), m_oldDefaultInstrument(), m_dummyFilesDir(),
m_dirWithWhitespace(), m_tempDirs(), m_exts{".raw", ".nxs"},
m_oldArchiveSearchSetting(),
g_config(Mantid::Kernel::ConfigService::Instance()) {
m_dummyFilesDir =
createAbsoluteDirectory("_MultipleFilePropertyTestDummyFiles");
Expand Down Expand Up @@ -157,11 +159,13 @@ class MultipleFilePropertyTest : public CxxTest::TestSuite {
m_oldDataSearchDirectories = g_config.getString("datasearch.directories");
m_oldDefaultFacility = g_config.getString("default.facilities");
m_oldDefaultInstrument = g_config.getString("default.instrument");
m_oldArchiveSearchSetting = g_config.getString("datasearch.searcharchive");

g_config.setString("datasearch.directories",
m_dummyFilesDir + ";" + m_dirWithWhitespace + ";");
g_config.setString("default.facility", "ISIS");
g_config.setString("default.instrument", "TOSCA");
g_config.setString("datasearch.searcharchive", "Off");

// Make sure that multi file loading is enabled for each test.
m_multiFileLoadingSetting =
Expand All @@ -173,6 +177,7 @@ class MultipleFilePropertyTest : public CxxTest::TestSuite {
g_config.setString("datasearch.directories", m_oldDataSearchDirectories);
g_config.setString("default.facility", m_oldDefaultFacility);
g_config.setString("default.instrument", m_oldDefaultInstrument);
g_config.setString("datasearch.searcharchive", m_oldArchiveSearchSetting);

// Replace user's preference after the test has run.
Kernel::ConfigService::Instance().setString("loading.multifile",
Expand Down

0 comments on commit 0e72709

Please sign in to comment.