Skip to content
Snippets Groups Projects
Commit d2a875f3 authored by Maxence Naud's avatar Maxence Naud
Browse files

Merge branch 'Memory_leak' into 'main' 

[Bug] Solve several memory leaks in the code

See merge request aidge/aidge_core!2
parents f2ea4dc9 bae707f6
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
include/aidge/graph/Node.hpp
+ 40
− 26
View file @ d2a875f3
......@@ -34,13 +34,23 @@ class GraphView;
*/
class Node : public std::enable_shared_from_this<Node> {
private:
struct weakCompare {
bool operator()(const std::weak_ptr<Aidge::GraphView>& a, const std::weak_ptr<Aidge::GraphView>& b) const {
// Compare the content of the weak_ptrs
auto sharedA = a.lock();
auto sharedB = b.lock();
if (!sharedB) return false; // nothing after expired pointer
if (!sharedA) return true;
return sharedA < sharedB; // shared_ptr has a valid comparison operator
}
};
std::string mName; /** Name of the Node. Should be unique. */
std::set<std::shared_ptr<GraphView>> mViews = std::set<std::shared_ptr<GraphView>>(); /** Set of pointers to GraphView instances including this Node instance. */
std::set<std::weak_ptr<GraphView>, weakCompare> mViews; /** Set of pointers to GraphView instances including this Node instance. */
const std::shared_ptr<Operator> mOperator; // Pointer to the associated Operator
std::vector<NodePtr> mParents; /** List of parent node for each input (Parent --> Node --> Child) */
std::vector<std::vector<NodePtr>> mChildren; /** List of children nodes for each output (Parent --> Node --> Child) */
std::vector<std::vector<std::weak_ptr<Node>>> mChildren; /** List of children nodes for each output (Parent --> Node --> Child) */
std::vector<std::vector<IOIndex_t>> mIdInChildren; /** List of input index for each Node linked to each output of the Node. */
std::vector<IOIndex_t> mIdOutParents; /** index of the output linked to each input of the Node. Default: gk_IODefaultIndex. */
......@@ -70,7 +80,7 @@ public:
* @param ctors Ordered Connectors linking their associated Node to the input of the current Node with the same index.
* @return Connector
*/
Connector operator()(const std::vector<Connector> ctors);
Connector operator()(const std::vector<Connector> &ctors);
public:
///////////////////////////////////////////////////////
......@@ -131,14 +141,14 @@ public:
/**
* @brief List of pair <Parent, ID of the data intput>. When an input is not
* linked to any Parent, the pair is <nullptr, gk_IODefaultIndex>.
* @return std::vector<std::pair<NodePtr, IOIndex_t>>
* @return std::vector<std::pair<std::shared_ptr<Node>, IOIndex_t>>
*/
std::vector<std::pair<NodePtr, IOIndex_t>> dataInputs() const;
/**
* @brief List of pair <Parent, ID of the parent output>. When an input is not linked
* to any Parent, the pair is <nullptr, gk_IODefaultIndex>.
* @return std::vector<std::pair<NodePtr, IOIndex_t>>
* @return std::vector<std::pair<std::shared_ptr<Node>, IOIndex_t>>
*/
std::vector<std::pair<NodePtr, IOIndex_t>> inputs() const;
......@@ -146,7 +156,7 @@ public:
* @brief Parent and its output Tensor ID linked to the inID-th input Tensor.
* If the input is not linked to any Parent, the pair is <nullptr, gk_IODefaultIndex>.
* @param inID
* @return std::pair<NodePtr, IOIndex_t>
* @return std::pair<std::shared_ptr<Node>, IOIndex_t>
*/
inline std::pair<NodePtr, IOIndex_t> input(const IOIndex_t inID) const {
assert((inID != gk_IODefaultIndex) && (inID < nbInputs()) && "Input index out of bound.");
......@@ -178,19 +188,19 @@ public:
/**
* @brief List input ids of children liked to outputs of the node
* @return std::vector<std::vector<std::pair<NodePtr,
* @return std::vector<std::vector<std::pair<std::shared_ptr<Node>,
* IOIndex_t>>>
*/
std::vector<std::vector<std::pair<NodePtr, IOIndex_t>>> outputs() const;
/**
* @brief Children and their input Tensor ID linked to the outID-th output
* @brief Children and their input Tensor ID linked to the outId-th output
* Tensor.
* @param outID
* @return std::vector<std::pair<NodePtr, IOIndex_t>>
* @param outId
* @return std::vector<std::pair<std::shared_ptr<Node>, IOIndex_t>>
*/
std::vector<std::pair<NodePtr, IOIndex_t>>
output(IOIndex_t outID) const;
output(IOIndex_t outId) const;
/**
* @brief Number of inputs, including both data and learnable parameters.
......@@ -231,7 +241,11 @@ public:
* @return std::vector<GraphView>
*/
inline std::set<std::shared_ptr<GraphView>> views() const noexcept {
return mViews;
std::set<std::shared_ptr<GraphView>> res;
for (const auto &v : mViews) {
res.insert(v.lock());
}
return res;
}
/**
......@@ -239,14 +253,14 @@ public:
* the current Node. This feature allows transparent GraphViews.
* @param graphPtr Pointer to GraphView to add to the list.
*/
inline void addView(const std::shared_ptr<GraphView> graphPtr) {
mViews.insert(graphPtr);
inline void addView(const std::shared_ptr<GraphView> &graphPtr) {
mViews.insert(std::weak_ptr<GraphView>(graphPtr));
}
inline void removeView(const std::shared_ptr<GraphView> graphPtr) {
if (mViews.find(graphPtr) != mViews.end()) {
mViews.erase(graphPtr);
}
inline void removeView(const std::shared_ptr<GraphView> &graphPtr) {
std::set<std::weak_ptr<GraphView>, weakCompare>::const_iterator viewIt = mViews.cbegin();
for (; (viewIt != mViews.cend()) && ((*viewIt).lock() != graphPtr) ; ++viewIt) {}
mViews.erase(*viewIt);
}
/**
......@@ -280,14 +294,14 @@ public:
/**
* @brief Get the list of parent Nodes. As an input is linked to a unique Node,
* if none is linked then the parent is a nullptr.
* @return std::vector<NodePtr>
* @return std::vector<std::shared_ptr<Node>>
*/
std::vector<NodePtr> getParents() const;
/**
* @brief Get the pointer to parent of the specified input index. This pointer is nullptr if no parent is linked.
* @param inId Input index.
* @return NodePtr&
* @return std::shared_ptr<Node>&
*/
inline NodePtr &getParents(const IOIndex_t inId) {
assert(inId != gk_IODefaultIndex);
......@@ -298,7 +312,7 @@ public:
* @brief Unlink the parent Node at the specified input index and return its pointer.
* Return a nullptr is no parent was linked.
* @param inId Input index.
* @return NodePtr
* @return std::shared_ptr<Node>
*/
NodePtr popParent(const IOIndex_t inId);
......@@ -308,7 +322,7 @@ public:
* @brief Get the set of pointers to children Nodes linked to the current Node.object.
* @details The returned set does not include any nullptr as an output maybe linked to
* an undifined number of Nodes. It does not change the computation of its associated Operator.
* @return std::set<NodePtr>>
* @return std::set<std::shared_ptr<Node>>>
*/
std::set<NodePtr> getChildren() const;
......@@ -317,14 +331,14 @@ public:
/**
* @brief Get the list of children Nodes linked to the output at specified index.
* @param outId Output index.
* @return std::vector<NodePtr>
* @return std::vector<std::shared_ptr<Node>>
*/
std::vector<NodePtr> getChildren(const IOIndex_t outID) const;
std::vector<NodePtr> getChildren(const IOIndex_t outId) const;
/**
* @brief Remove registered child from children list of specified output if possible.
* If so, also remove current Node from child Node from parent.
* @param nodePtr Node to remove.
* @param std::shared_ptr<Node> Node to remove.
* @param outId Output index. Default 0.
* @return true Child found and removed for given output index.
* @return false Child not found at given index. Nothing removed.
......@@ -388,4 +402,4 @@ private:
};
} // namespace Aidge
#endif /* __AIDGE_CORE_GRAPH_NODE_H__ */
\ No newline at end of file
#endif /* __AIDGE_CORE_GRAPH_NODE_H__ */
include/aidge/utils/CParameter.hpp
+ 25
− 18
View file @ d2a875f3
......@@ -16,6 +16,7 @@
#include <map>
#include <vector>
#include <numeric>
#include <cstddef>
namespace Aidge {
......@@ -45,9 +46,9 @@ public:
{
assert(m_Params.find(i_ParamName) != m_Params.end());
assert(m_Types.find(i_ParamName) != m_Types.end());
assert(m_Params.at(i_ParamName) <= m_OffSet);
assert(m_Params.at(i_ParamName) <= m_Size);
assert(typeid(T).name() == m_Types.at(i_ParamName));
return *reinterpret_cast<T *>(m_BeginBuffer + m_Params.at(i_ParamName));
return *reinterpret_cast<T *>(m_Buffer + m_Params.at(i_ParamName));
}
///\brief Add a parameter value, identified by its name
......@@ -59,12 +60,21 @@ public:
/// internal buffer in a new location (previous value is still in memory at its previous location)
template<class T> void Add(std::string const &i_ParamName, T const &i_Value)
{
m_Buffer.resize(m_Buffer.size() + (sizeof(T) / sizeof(uint8_t)));
m_BeginBuffer = m_Buffer.data(); // Update buffer ptr in case of memory reordering
*reinterpret_cast<T *>(m_BeginBuffer + m_OffSet)
= i_Value; // Black-magic used to add anytype into the vector
m_Params[i_ParamName] = m_OffSet; // Copy pointer offset
m_OffSet += sizeof(T); // Increment offset
const std::size_t addedSize = sizeof(T) / sizeof(std::uint8_t);
std::uint8_t *tmp = m_Buffer;
std::uint8_t *m_NewBuffer = static_cast<std::uint8_t *>(std::malloc((m_Size + addedSize)*sizeof(std::uint8_t)));
for (std::size_t i = 0; i < m_Size; ++i) {
m_NewBuffer[i] = m_Buffer[i];
}
free(tmp);
for (std::size_t i = 0; i < addedSize; ++i) {
m_NewBuffer[m_Size+i] = *(reinterpret_cast<const std::uint8_t *>(&i_Value) + i);
}
m_Buffer = m_NewBuffer;
m_Params[i_ParamName] = m_Size; // Copy pointer offset
m_Size += addedSize; // Increment offset
m_Types[i_ParamName] = typeid(i_Value).name();
}
......@@ -80,10 +90,14 @@ public:
}
~CParameter() = default;
~CParameter() {
free(m_Buffer);
}
private:
// Note for Cyril: of course storing offset and not address! Good idea
/// @brief Number of elements in m_Buffer
std::size_t m_Size = 0;
std::map<std::string, std::size_t> m_Params; // { Param name : offset }
///\brief Map to check type error
......@@ -95,16 +109,9 @@ private:
std::map<std::string, std::string> m_Types;
///\brief All parameters values concatenated in raw binary form.
std::vector<uint8_t> m_Buffer = {};
///\brief Starting address of the buffer
uint8_t *m_BeginBuffer = m_Buffer.data();
///\brief Offset, in number of uint8_t, of the next parameter to write
std::size_t m_OffSet = 0;
std::uint8_t *m_Buffer = static_cast<std::uint8_t *>(std::malloc(0));
};
}
#endif /* __AIDGE_CPARAMETER_H__ */
src/graph/Node.cpp
+ 38
− 29
View file @ d2a875f3
......@@ -21,8 +21,8 @@ Aidge::Node::Node(std::shared_ptr<Operator> op, const char *name)
: mName((name == nullptr) ? std::string() : std::string(name)),
mOperator(op),
mParents(std::vector<std::shared_ptr<Node>>(static_cast<std::size_t>(op->nbInputs()), nullptr)),
mChildren(std::vector<std::vector<std::shared_ptr<Node>>>(static_cast<std::size_t>(op->nbOutputs()),
std::vector<std::shared_ptr<Node>>())),
mChildren(std::vector<std::vector<std::weak_ptr<Node>>>(static_cast<std::size_t>(op->nbOutputs()),
std::vector<std::weak_ptr<Node>>())),
mIdInChildren(
std::vector<std::vector<IOIndex_t>>(static_cast<std::size_t>(op->nbOutputs()), std::vector<IOIndex_t>())),
mIdOutParents(std::vector<IOIndex_t>(static_cast<std::size_t>(op->nbInputs()), gk_IODefaultIndex)) {
......@@ -33,7 +33,7 @@ Aidge::Node::Node(std::shared_ptr<Operator> op, const char *name)
// FUNCTIONAL DESCRIPTION
///////////////////////////////////////////////////////
Aidge::Connector Aidge::Node::operator()(const std::vector<Connector> ctors) {
Aidge::Connector Aidge::Node::operator()(const std::vector<Connector> &ctors) {
assert((ctors.size() == nbDataInputs()) && "Wrong number of arguments.\n");
for (__attribute__((unused)) std::pair<std::shared_ptr<Node>, IOIndex_t> &input : inputs()) {
assert((gk_IODefaultIndex == input.second) && "At least one input connection is not free.\n");
......@@ -134,12 +134,12 @@ Aidge::Node::outputs() const {
}
std::vector<std::pair<std::shared_ptr<Aidge::Node>, Aidge::IOIndex_t>>
Aidge::Node::output(Aidge::IOIndex_t outID) const {
Aidge::Node::output(Aidge::IOIndex_t outId) const {
std::vector<std::pair<std::shared_ptr<Node>, IOIndex_t>> listOutputs =
std::vector<std::pair<std::shared_ptr<Node>, IOIndex_t>>(mIdInChildren[outID].size());
for (std::size_t i = 0; i < mIdInChildren[outID].size(); ++i) {
std::vector<std::pair<std::shared_ptr<Node>, IOIndex_t>>(mIdInChildren[outId].size());
for (std::size_t i = 0; i < mIdInChildren[outId].size(); ++i) {
listOutputs[i] =
std::pair<std::shared_ptr<Node>, IOIndex_t>(mChildren[outID][i], mIdInChildren[outID][i]);
std::pair<std::shared_ptr<Node>, IOIndex_t>(mChildren[outId][i].lock(), mIdInChildren[outId][i]);
}
return listOutputs;
}
......@@ -161,7 +161,7 @@ Aidge::IOIndex_t Aidge::Node::nbValidOutputs() const {
return counter;
}
void Aidge::Node::setInputId(const IOIndex_t inId, const IOIndex_t newNodeOutID) {
void Aidge::Node::setInputId(const IOIndex_t inId, const IOIndex_t newNodeoutId) {
assert(inId != gk_IODefaultIndex && (inId < nbInputs()) && "Must be a valid index");
if (mIdOutParents[inId] != gk_IODefaultIndex) {
std::printf("Warning: filling a Tensor already attributed\n");
......@@ -171,7 +171,7 @@ void Aidge::Node::setInputId(const IOIndex_t inId, const IOIndex_t newNodeOutID)
// find first occurence of child in the output's children
originalParent.first->removeChild(shared_from_this(), originalParent.second);
}
mIdOutParents[inId] = newNodeOutID;
mIdOutParents[inId] = newNodeoutId;
}
///////////////////////////////////////////////////////
......@@ -179,9 +179,8 @@ void Aidge::Node::setInputId(const IOIndex_t inId, const IOIndex_t newNodeOutID)
///////////////////////////////////////////////////////
void Aidge::Node::addChildOp(std::shared_ptr<Node> otherNode, const IOIndex_t outId, const IOIndex_t otherInId) {
assert((otherInId != gk_IODefaultIndex) && (otherInId < otherNode->nbInputs()) &&
"Input index out of bound.");
assert((outId != gk_IODefaultIndex) && (outId < nbOutputs()) && "Output index out of bound.");
assert((otherInId < otherNode->nbInputs()) && "Input index out of bound.");
assert((outId < nbOutputs()) && "Output index out of bound.");
if (otherNode->input(otherInId).second != gk_IODefaultIndex) {
std::printf("Warning, the %d-th Parent of the child node already existed.\n", otherInId);
}
......@@ -189,24 +188,22 @@ void Aidge::Node::addChildOp(std::shared_ptr<Node> otherNode, const IOIndex_t ou
otherNode->setInputId(otherInId, outId);
otherNode->getOperator()->associateInput(otherInId, getOperator()->getRawOutput(outId));
// manage nodes
mChildren[outId].push_back(otherNode);
mChildren[outId].push_back(std::weak_ptr<Node>(otherNode));
mIdInChildren[outId].push_back(otherInId);
otherNode->addParent(shared_from_this(), otherInId);
}
void Aidge::Node::addChildView(std::shared_ptr<GraphView> other_graph, const IOIndex_t outID,
void Aidge::Node::addChildView(std::shared_ptr<GraphView> otherGraph, const IOIndex_t outId,
std::pair<std::shared_ptr<Node>, IOIndex_t> otherInId) {
assert((otherInId.second != gk_IODefaultIndex) &&
(otherInId.second < otherInId.first->nbInputs()) &&
"Other graph input index out of bound.");
assert((outID != gk_IODefaultIndex) && (outID < nbOutputs()) && "Output index out of bound.");
std::set<std::shared_ptr<Node>> inNodes = other_graph->inputNodes();
assert((otherInId.second < otherInId.first->nbInputs()) && "Other graph input index out of bound.");
assert((outId < nbOutputs()) && "Output index out of bound.");
std::set<std::shared_ptr<Node>> inNodes = otherGraph->inputNodes();
if (inNodes.size() == std::size_t(0)) { // no input Node
printf("Cannot add GraphView to the Node. No input node detected.\n");
} else // inNodes.size() >= 1
{
assert((inNodes.find(otherInId.first) != inNodes.end())); // assert it really is an input node
addChildOp(otherInId.first, outID, otherInId.second);
addChildOp(otherInId.first, outId, otherInId.second);
}
}
......@@ -256,24 +253,36 @@ bool Aidge::Node::removeParent(const IOIndex_t inId) {
std::set<std::shared_ptr<Aidge::Node>> Aidge::Node::getChildren() const {
std::set<std::shared_ptr<Node>> children;
for (const std::vector<std::shared_ptr<Node>> &childrenOfOneOutput : mChildren) {
children.insert(childrenOfOneOutput.begin(), childrenOfOneOutput.end());
for (const auto &childrenOfOneOutput : mChildren) {
for (const auto &oneChild : childrenOfOneOutput) {
children.insert(oneChild.lock());
}
}
return children;
}
std::vector<std::vector<std::shared_ptr<Aidge::Node>>> Aidge::Node::getOrderedChildren() const { return mChildren; }
std::vector<std::vector<std::shared_ptr<Aidge::Node>>> Aidge::Node::getOrderedChildren() const {
std::vector<std::vector<std::shared_ptr<Node>>> children = std::vector<std::vector<std::shared_ptr<Node>>>(mChildren.size());
for (std::size_t outId = 0; outId < mChildren.size(); ++outId) {
children[outId] = getChildren(outId);
}
return children;
}
std::vector<std::shared_ptr<Aidge::Node>> Aidge::Node::getChildren(const IOIndex_t outID) const {
assert((outID != gk_IODefaultIndex) && (outID < nbOutputs()) && "Output index out of bound.");
return mChildren[outID];
std::vector<std::shared_ptr<Aidge::Node>> Aidge::Node::getChildren(const IOIndex_t outId) const {
assert((outId < nbOutputs()) && "Output index out of bound.");
std::vector<std::shared_ptr<Node>> children = std::vector<std::shared_ptr<Node>>(mChildren[outId].size());
for (std::size_t i = 0; i < mChildren[outId].size(); ++i) {
children.push_back(mChildren[outId][i].lock());
}
return children;
}
bool Aidge::Node::removeChild(const std::shared_ptr<Aidge::Node> nodePtr, const Aidge::IOIndex_t outId) {
assert((outId != gk_IODefaultIndex) && (outId < nbOutputs()) && "Child index out of bound.");
assert((outId < nbOutputs()) && "Child index out of bound.");
bool removed = false;
for (std::size_t j = 0; j < mChildren[outId].size(); ++j) {
if (mChildren[outId][j] == nodePtr) {
if (mChildren[outId][j].lock() == nodePtr) {
mChildren[outId].erase(mChildren[outId].begin() + j);
mIdInChildren[outId].erase(mIdInChildren[outId].begin() + j);
removed = true;
......@@ -301,7 +310,7 @@ void Aidge::Node::resetConnections(bool includeLearnableParam) {
for (std::pair<std::shared_ptr<Node>, IOIndex_t> child : output(i)) {
child.first->removeParent(child.second);
}
mChildren[i] = std::vector<std::shared_ptr<Node>>();
mChildren[i] = std::vector<std::weak_ptr<Node>>();
mIdInChildren[i] = std::vector<IOIndex_t>();
}
// removing this Node from every GraphView it belongs to
......
src/graph/OpArgs.cpp
+ 2
− 2
View file @ d2a875f3
......@@ -22,10 +22,10 @@ std::shared_ptr<Aidge::GraphView> Aidge::Sequential(std::initializer_list<OpArgs
assert(static_cast<std::size_t>(elt.node()->getNbFreeDataInputs()) >= gv->outputNodes().size());
/*
* /!\ mn.view()->outputNodes() is a set, order of Nodes cannot be guaranted.
* Prefer a functional descrition for detailed inputs
* Prefer a functional description for detailed inputs
*/
for (const std::shared_ptr<Node>& node_ptr : gv->outputNodes()) {
node_ptr -> addChild(elt.node()); // already check that node_ptr->nbOutput == 1
node_ptr -> addChild(elt.node()); // already checks that node_ptr->nbOutput() == 1
}
gv->add(elt.node());
}
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment

Copyright © Eclipse Foundation, Inc. All Rights Reserved.     Privacy Policy | Terms of Use | Copyright Agent