Skip to content

Commit

Permalink
GH-454: Fix activating topology takes long with assets (GH-454)
Browse files Browse the repository at this point in the history
dds-topology: Fix activating topology takes too long when task assets are used. (GH-454)
  • Loading branch information
AnarManafov committed Jun 29, 2022
1 parent 061474b commit 29a8cb7
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 9 deletions.
1 change: 1 addition & 0 deletions ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Added: The command learned a new argument --inline-config. Content of this strin
### dds-topology
Fixed: Stability improvements.
Fixed: A bug which caused dds::topology_api::CTopoCreator to ignore task assets. (GH-452)
Fixed: Activating topology takes too long when task assets are used. (GH-454)
Added: A new groupName requirement. It can be used on task and collection. (GH-407)
Added: Open API to read/update/add topology variable. The CTopoVars class.
Added: Support for Task Assets. (GH-406)
Expand Down
34 changes: 25 additions & 9 deletions dds-topology-lib/src/TopoAsset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ using namespace boost::property_tree;
using namespace dds;
using namespace topology_api;

CTopoAsset::CTopoAsset(const std::string& _name)
CTopoAsset::CTopoAsset(const string& _name)
: CTopoBase(_name)
{
setType(CTopoBase::EType::ASSET);
Expand All @@ -30,9 +30,14 @@ void CTopoAsset::initFromPropertyTree(const boost::property_tree::ptree& _pt)
try
{
const ptree& assetPT = FindElementInPropertyTree(CTopoBase::EType::ASSET, getName(), _pt.get_child("topology"));
setAssetType(TagToAssetType(assetPT.get<std::string>("<xmlattr>.type", "")));
setAssetVisibility(TagToAssetVisibility(assetPT.get<std::string>("<xmlattr>.visibility", "")));
setValue(assetPT.get<std::string>("<xmlattr>.value", ""));
setAssetType(TagToAssetType(assetPT.get<string>("<xmlattr>.type", "")));
setAssetVisibility(TagToAssetVisibility(assetPT.get<string>("<xmlattr>.visibility", "")));

// Add asset value if the asset container doesn't have it.
if (getNameToValueCache() && !getNameToValueCache()->exists(getName()))
{
getNameToValueCache()->insertValue(getName(), assetPT.get<string>("<xmlattr>.value", ""));
}
}
catch (exception& error) // ptree_error, runtime_error
{
Expand All @@ -44,7 +49,7 @@ void CTopoAsset::saveToPropertyTree(boost::property_tree::ptree& _pt)
{
try
{
std::string tag("topology.asset.<xmlattr>");
string tag("topology.asset.<xmlattr>");
_pt.put(tag + ".name", getName());
_pt.put(tag + ".type", AssetTypeToTag(getAssetType()));
_pt.put(tag + ".visibility", AssetVisibilityToTag(getAssetVisibility()));
Expand All @@ -56,12 +61,17 @@ void CTopoAsset::saveToPropertyTree(boost::property_tree::ptree& _pt)
}
}

std::string CTopoAsset::getValue() const
string CTopoAsset::getValue() const
{
return m_value;
CNameToValueCachePtr_t assetCache{ getNameToValueCache() };

if (!assetCache)
return string();

return (assetCache->getValue(getName()));
}

void CTopoAsset::setValue(const std::string& _val)
void CTopoAsset::setValue(const string& _val)
{
m_value = _val;
}
Expand Down Expand Up @@ -102,8 +112,14 @@ ostream& operator<<(ostream& _strm, const CTopoAsset& _requirement)

string CTopoAsset::hashString() const
{
// WORKAROUND:
// Assets value can be big.
// We therefore can't use its value in hash calculations as it significantly drops the performance.
// see GH-454
// That means, if an asset value is changed, DDS will be not able to detect changes.
// Therefore, if you need to change the asset value and request a topology update, change its name too.
stringstream ss;
ss << "|Asset|" << getName() << "|" << AssetTypeToTag(getAssetType()) << "|"
<< AssetVisibilityToTag(getAssetVisibility()) << "|" << getValue() << "|";
<< AssetVisibilityToTag(getAssetVisibility()) << "|" /*<< getValue() << "|"*/;
return ss.str();
}
10 changes: 10 additions & 0 deletions dds-topology-lib/src/TopoBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ void CTopoBase::setParent(CTopoBase* _parent)
m_parent = _parent;
}

void CTopoBase::setNameToValueCache(CNameToValueCachePtr_t _container)
{
m_nameToValueCache = _container;
}

const string& CTopoBase::getName() const
{
return m_name;
Expand All @@ -53,6 +58,11 @@ CTopoBase* CTopoBase::getParent() const
return m_parent;
}

dds::topology_api::CTopoBase::CNameToValueCachePtr_t CTopoBase::getNameToValueCache() const
{
return m_nameToValueCache;
}

string CTopoBase::getPath() const
{
if (getParent() == nullptr)
Expand Down
36 changes: 36 additions & 0 deletions dds-topology-lib/src/TopoBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,38 @@ namespace dds
{
namespace topology_api
{
class CNameToValueCache
{
using container_t = std::map<std::string, std::string>;

public:
void insertValue(const std::string& _name, const std::string _value)
{
if (exists(_name))
return;

m_container.insert(std::make_pair(_name, _value));
}

std::string getValue(const std::string& _name)
{
auto found = m_container.find(_name);
if (found == m_container.end())
return std::string();

return found->second;
}

bool exists(const std::string& _name)
{
auto found = m_container.find(_name);
return (found != m_container.end());
}

private:
container_t m_container;
};

class CTopoBase
{
public:
Expand All @@ -36,15 +68,18 @@ namespace dds

using Ptr_t = std::shared_ptr<CTopoBase>;
using PtrVector_t = std::vector<CTopoBase::Ptr_t>;
using CNameToValueCachePtr_t = std::shared_ptr<CNameToValueCache>;

/// Modifiers
void setName(const std::string& _name);
void setParent(CTopoBase* _parent);
void setNameToValueCache(CNameToValueCachePtr_t _container);

/// Accessors
const std::string& getName() const;
CTopoBase::EType getType() const;
CTopoBase* getParent() const;
CNameToValueCachePtr_t getNameToValueCache() const;

/// \brief Return full path to topo element or property.
std::string getPath() const;
Expand Down Expand Up @@ -147,6 +182,7 @@ namespace dds
std::string m_name; ///< Name of topology element
CTopoBase::EType m_type{ EType::TOPO_BASE }; ///< Type of the topology element
CTopoBase* m_parent{ nullptr }; ///< Pointer to parent element
CNameToValueCachePtr_t m_nameToValueCache;
};
} // namespace topology_api
} // namespace dds
Expand Down
4 changes: 4 additions & 0 deletions dds-topology-lib/src/TopoContainer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ using namespace topology_api;
CTopoContainer::CTopoContainer(const std::string& _name)
: CTopoElement(_name)
{
if (_name == "main")
{
setNameToValueCache(make_shared<CTopoBase::CNameToValueCachePtr_t::element_type>());
}
}

CTopoContainer::~CTopoContainer()
Expand Down
1 change: 1 addition & 0 deletions dds-topology-lib/src/TopoContainer.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ namespace dds
{
auto element{ std::make_shared<Object_t>(_name) };
element->setParent(this);
element->setNameToValueCache(getNameToValueCache());
m_elements.push_back(element);
return element;
}
Expand Down
1 change: 1 addition & 0 deletions dds-topology-lib/src/TopoTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ CTopoAsset::Ptr_t CTopoTask::addAsset(const string& _name)
{
auto asset{ make_shared<CTopoAsset>(_name) };
asset->setParent(this);
asset->setNameToValueCache(getNameToValueCache());
m_assets.push_back(asset);
return asset;
}
Expand Down

0 comments on commit 29a8cb7

Please sign in to comment.