Skip to content

Commit

Permalink
Code review feedback and some other changes I noticed
Browse files Browse the repository at this point in the history
  • Loading branch information
mpartio committed Oct 17, 2018
1 parent 446f644 commit 75cf86f
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 24 deletions.
6 changes: 4 additions & 2 deletions doc/plugin-cache.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
# Summary

Cache plugin is one of the infrastructure plugins. It stores all read and written fields to memory for further use. It has the ability to evict data with an LRU algorithm when the cache has grown over a specified limit. The default behavior is to store everything without eviction. When plugins are requesting data, cache is the first place they will check. Cache is immutable.
Cache plugin is one of the infrastructure plugins. It stores all read and written fields to memory for further use. It has the ability to evict data with an LRU algorithm when the cache has grown over a specified limit. The default behavior is to store everything without eviction. When plugins are requesting data, cache is the first place they will check.

The fields are stored as shared_ptr's, and each field will have a label associated which will be matched to incoming cache requests. When a field is returned from cache, it is copied to the caller so that cache contents are immutable.
The fields are stored as shared pointers, and each field will have a label associated which will be matched to incoming data requests.

The cache has the ability to store data in different data types, for example float or double. If data is found from cache in a different data type than what was requested, plugin will do a conversion *unless* strict mode has been enabled. In the latter case cache plugin will return nil and other data sources must be searched.

# Configuration options

Expand Down
13 changes: 10 additions & 3 deletions himan-plugins/include/cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ class cache : public auxiliary_plugin
void Insert(std::shared_ptr<info<double>> anInfo, bool pin = false);

template <typename T>
std::vector<std::shared_ptr<info<T>>> GetInfo(search_options& options);
std::vector<std::shared_ptr<info<T>>> GetInfo(search_options& options, bool strict = false);

std::vector<std::shared_ptr<info<double>>> GetInfo(search_options& options);
std::vector<std::shared_ptr<info<double>>> GetInfo(search_options& options, bool strict = false);

void Clean();

Expand Down Expand Up @@ -99,8 +99,15 @@ class cache_pool : public auxiliary_plugin
template <typename T>
void Insert(const std::string& uniqueName, std::shared_ptr<info<T>> info, bool pin);

/**
* @brief Get info from cache
*
* @param uniqueName unique label that identifies a cache element
* @param strict define whether cache is allowed to do data type conversion (--> strict=false)
*/

template <typename T>
std::shared_ptr<info<T>> GetInfo(const std::string& uniqueName);
std::shared_ptr<info<T>> GetInfo(const std::string& uniqueName, bool strict);

void Clean();

Expand Down
53 changes: 34 additions & 19 deletions himan-plugins/source/cache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ string cache::UniqueName(const info<T>& info)
}

template string cache::UniqueName(const info<double>&);
template string cache::UniqueName(const info<float>&);

string cache::UniqueNameFromOptions(search_options& options)
{
Expand Down Expand Up @@ -78,6 +79,7 @@ void cache::Insert(shared_ptr<info<T>> anInfo, bool pin)

if (cache_pool::Instance()->Exists(uniqueName))
{
// TODO: should we replace existing item?
itsLogger.Trace("Data with key " + uniqueName + " already exists at cache");

// Update timestamp of this cache item
Expand Down Expand Up @@ -111,22 +113,22 @@ void cache::Insert(shared_ptr<info<T>> anInfo, bool pin)
cache_pool::Instance()->Insert<T>(uniqueName, localInfo, pin);
}

template void cache::Insert<double>(shared_ptr<info<double>> anInfo, bool pin);
template void cache::Insert<float>(shared_ptr<info<float>> anInfo, bool pin);
template void cache::Insert<double>(shared_ptr<info<double>>, bool);
template void cache::Insert<float>(shared_ptr<info<float>>, bool);

vector<shared_ptr<himan::info<double>>> cache::GetInfo(search_options& options)
vector<shared_ptr<himan::info<double>>> cache::GetInfo(search_options& options, bool strict)
{
return GetInfo<double>(options);
return GetInfo<double>(options, strict);
}

template <typename T>
vector<shared_ptr<himan::info<T>>> cache::GetInfo(search_options& options)
vector<shared_ptr<himan::info<T>>> cache::GetInfo(search_options& options, bool strict)
{
string uniqueName = UniqueNameFromOptions(options);

vector<shared_ptr<himan::info<T>>> infos;

auto foundInfo = cache_pool::Instance()->GetInfo<T>(uniqueName);
auto foundInfo = cache_pool::Instance()->GetInfo<T>(uniqueName, strict);
if (foundInfo)
{
infos.push_back(foundInfo);
Expand All @@ -137,8 +139,8 @@ vector<shared_ptr<himan::info<T>>> cache::GetInfo(search_options& options)
return infos;
}

template vector<shared_ptr<himan::info<double>>> cache::GetInfo<double>(search_options&);
template vector<shared_ptr<himan::info<float>>> cache::GetInfo<float>(search_options&);
template vector<shared_ptr<himan::info<double>>> cache::GetInfo<double>(search_options&, bool);
template vector<shared_ptr<himan::info<float>>> cache::GetInfo<float>(search_options&, bool);

void cache::Clean()
{
Expand All @@ -149,19 +151,25 @@ size_t cache::Size() const
return cache_pool::Instance()->Size();
}

void cache::Replace(info_t anInfo, bool pin)
void cache::Replace(shared_ptr<info<double>> anInfo, bool pin)
{
auto localInfo = make_shared<info<double>>(*anInfo);
return Replace<double>(anInfo, pin);
}

template <typename T>
void cache::Replace(shared_ptr<info<T>> anInfo, bool pin)
{
auto localInfo = make_shared<info<T>>(*anInfo);

if (localInfo->DimensionSize() > 1)
{
auto newInfo = make_shared<info<double>>(localInfo->ForecastType(), localInfo->Time(), localInfo->Level(),
localInfo->Param());
auto newInfo =
make_shared<info<T>>(localInfo->ForecastType(), localInfo->Time(), localInfo->Level(), localInfo->Param());
newInfo->Base(localInfo->Base());
localInfo = newInfo;
}

cache_pool::Instance()->Replace(UniqueName(*localInfo), localInfo, pin);
cache_pool::Instance()->Replace<T>(UniqueName(*localInfo), localInfo, pin);
}

cache_pool* cache_pool::itsInstance = NULL;
Expand Down Expand Up @@ -243,6 +251,7 @@ void cache_pool::Replace(const string& uniqueName, shared_ptr<himan::info<T>> an
}

template void cache_pool::Replace<double>(const string&, shared_ptr<himan::info<double>>, bool);
template void cache_pool::Replace<float>(const string&, shared_ptr<himan::info<float>>, bool);

void cache_pool::UpdateTime(const std::string& uniqueName)
{
Expand Down Expand Up @@ -297,9 +306,9 @@ shared_ptr<himan::info<U>> Convert(const shared_ptr<himan::info<T>>& fnd)
const auto& src = VEC(fnd);
auto& dst = b->data.Values();

copy(src.begin(), src.end(), dst.begin());
replace_copy_if(src.begin(), src.end(), dst.begin(), [](const U& val) { return ::isnan(val); },
himan::MissingValue<U>());

replace_if(dst.begin(), dst.end(), [](const U& val) { return ::isnan(val); }, himan::MissingValue<U>());
ret->Create(b);

return ret;
Expand All @@ -321,7 +330,7 @@ struct cache_visitor : public boost::static_visitor<std::shared_ptr<himan::info<
} // namespace

template <typename T>
shared_ptr<himan::info<T>> cache_pool::GetInfo(const string& uniqueName)
shared_ptr<himan::info<T>> cache_pool::GetInfo(const string& uniqueName, bool strict)
{
std::map<std::string, cache_item>::iterator it;

Expand All @@ -337,12 +346,18 @@ shared_ptr<himan::info<T>> cache_pool::GetInfo(const string& uniqueName)

try
{
// if data is directly found from cache with correct data type,
// return that
return boost::get<std::shared_ptr<info<T>>>(it->second.info);
}
catch (boost::bad_get& e)
{
itsLogger.Info("Found data from cache with different datatype, conversion imminent");
if (strict)
{
return nullptr;
}

// convert to wanted data type
return boost::apply_visitor(cache_visitor<T>(), it->second.info);
}
catch (exception& e)
Expand All @@ -353,8 +368,8 @@ shared_ptr<himan::info<T>> cache_pool::GetInfo(const string& uniqueName)
return nullptr;
}

template shared_ptr<himan::info<double>> cache_pool::GetInfo<double>(const string&);
template shared_ptr<himan::info<float>> cache_pool::GetInfo<float>(const string&);
template shared_ptr<himan::info<double>> cache_pool::GetInfo<double>(const string&, bool);
template shared_ptr<himan::info<float>> cache_pool::GetInfo<float>(const string&, bool);

size_t cache_pool::Size() const
{
Expand Down

0 comments on commit 75cf86f

Please sign in to comment.