Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SofaBaseTopology] Clean Topology logs and add AdvanceTimer logs #874

Merged
merged 8 commits into from
Jan 11, 2019
Merged

[SofaBaseTopology] Clean Topology logs and add AdvanceTimer logs #874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7e028fa
[SofaKernel] Add some advanceTimer logs in Point/Edge/Triangle topolo…
epernod Dec 18, 2018
921a49f
[SofaKernel] Clean TopologyEngine logs and add AdvanceTimer logs when…
epernod Dec 18, 2018
2f6a3fa
[SofaKernel] Changer several sout/serr into msg_ macro and add some A…
epernod Dec 19, 2018
65151aa
[SofaBaseTopology] change some logs to use dmsg and msg macros.
epernod Sep 18, 2018
5c80f27
[SofaKernel] Add: AdvanceTimer logs in TopologyHandler switch.
epernod Dec 24, 2018
7339c7d
[SofaBaseTopology] Change msg_error into dmsg_warning as check on rem…
epernod Jan 2, 2019
82ce024
Merge branch 'master' into topology_logs
epernod Jan 11, 2019
b6fce1d
[SofaBaseTopology] Remove duplicate error/warning keywords in PointSe…
epernod Jan 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion SofaKernel/framework/sofa/core/topology/TopologyHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* Contact information: contact@sofa-framework.org *
******************************************************************************/
#include <sofa/core/topology/TopologyHandler.h>

#include <sofa/helper/AdvancedTimer.h>

namespace sofa
{
Expand Down Expand Up @@ -48,6 +48,8 @@ void TopologyHandler::ApplyTopologyChanges(const std::list<const core::topology:
for (changeIt=_changeList.begin(); changeIt!=_changeList.end(); ++changeIt)
{
core::topology::TopologyChangeType changeType = (*changeIt)->getChangeType();
std::string topoChangeType = "DefaultTopologyHandler: " + parseTopologyChangeTypeToString(changeType);
Copy link
Contributor

@damienmarchal damienmarchal Jan 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not put string allocation + reallocation in a loop.
Wouldn't it be possible to have a static std::vector to map (with constant time access) the changeType to the timer string ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is already what is doing parseTopologyChangeTypeToString. But the first part is to know in which class we are.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not precomputing "DefaultTyopologyHandler+xxx" instead of re-computing it a loop ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not sure to understand, you mean having inside the class DefaultTopologyHandler a new map:

map<core::topology::TopologyChangeType, std::string> handlerMap;
handlerMap[changeType] = " DefaultTopologyHandler: " + parseTopologyChangeTypeToString(changeType);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it may even be a static map in the function.

sofa::helper::AdvancedTimer::stepBegin(topoChangeType);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is RAII based timers version to avoid the begin/end .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes we spoke about it wed. But I prefer the begin/end version to visually see the blocks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using {} ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the RAII version than incrementing the code :D

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what you mean :)


switch( changeType )
{
Expand Down Expand Up @@ -103,6 +105,7 @@ void TopologyHandler::ApplyTopologyChanges(const std::list<const core::topology:
break;
}; // switch( changeType )

sofa::helper::AdvancedTimer::stepEnd(topoChangeType);
//++changeIt;
}
}
Expand Down
33 changes: 29 additions & 4 deletions SofaKernel/modules/SofaBaseTopology/EdgeSetTopologyModifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#include <functional>
#include <iostream>
#include <sofa/core/ObjectFactory.h>

#include <sofa/helper/AdvancedTimer.h>

// Use BOOST GRAPH LIBRARY :

Expand Down Expand Up @@ -619,26 +619,35 @@ void EdgeSetTopologyModifier::splitEdgesProcess(sofa::helper::vector<EdgeID> &in
void EdgeSetTopologyModifier::removeEdges(const sofa::helper::vector< EdgeID >& edgeIds,
const bool removeIsolatedPoints, const bool resetTopoChange)
{
sofa::helper::AdvancedTimer::stepBegin("removeEdges");

sofa::helper::vector<EdgeID> edgeIds_filtered;
for (size_t i = 0; i < edgeIds.size(); i++)
{
if( edgeIds[i] >= m_container->getNumberOfEdges())
dmsg_error() << "Unable to remoev and edges: "<< edgeIds[i] <<" is out of bound and won't be removed." ;
dmsg_warning() << "Unable to remoev and edges: "<< edgeIds[i] <<" is out of bound and won't be removed." ;
else
edgeIds_filtered.push_back(edgeIds[i]);
}

/// add the topological changes in the queue
// add the topological changes in the queue
sofa::helper::AdvancedTimer::stepBegin("removeEdgesWarning");
removeEdgesWarning(edgeIds_filtered);

// inform other objects that the edges are going to be removed
sofa::helper::AdvancedTimer::stepNext ("removeEdgesWarning", "propagateTopologicalChanges");
if (resetTopoChange)
propagateTopologicalChanges();
else
propagateTopologicalChangesWithoutReset();

// now destroy the old edges.
sofa::helper::AdvancedTimer::stepNext ("propagateTopologicalChanges", "removeEdgesProcess");
removeEdgesProcess( edgeIds_filtered, removeIsolatedPoints );

sofa::helper::AdvancedTimer::stepEnd("removeEdgesProcess");
m_container->checkTopology();
sofa::helper::AdvancedTimer::stepEnd("removeEdges");
}

void EdgeSetTopologyModifier::removeItems(const sofa::helper::vector< EdgeID >& items)
Expand All @@ -661,9 +670,11 @@ void EdgeSetTopologyModifier::renumberPoints( const sofa::helper::vector<EdgeID>

void EdgeSetTopologyModifier::addEdges(const sofa::helper::vector< Edge >& edges)
{
sofa::helper::AdvancedTimer::stepBegin("addEdges");
size_t nEdges = m_container->getNumberOfEdges();

/// actually add edges in the topology container
// actually add edges in the topology container
sofa::helper::AdvancedTimer::stepBegin("addEdgesProcess");
addEdgesProcess(edges);

sofa::helper::vector<EdgeID> edgesIndex;
Expand All @@ -675,19 +686,26 @@ void EdgeSetTopologyModifier::addEdges(const sofa::helper::vector< Edge >& edges
}

// add topology event in the stack of topological events
sofa::helper::AdvancedTimer::stepNext ("addEdgesProcess", "addEdgesWarning");
addEdgesWarning(edges.size(), edges, edgesIndex);

// inform other objects that the edges are already added
sofa::helper::AdvancedTimer::stepNext ("addEdgesWarning", "propagateTopologicalChanges");
propagateTopologicalChanges();
sofa::helper::AdvancedTimer::stepEnd("propagateTopologicalChanges");

sofa::helper::AdvancedTimer::stepEnd("addEdges");
}

void EdgeSetTopologyModifier::addEdges(const sofa::helper::vector< Edge >& edges,
const sofa::helper::vector< sofa::helper::vector< EdgeID > > & ancestors,
const sofa::helper::vector< sofa::helper::vector< SReal > >& baryCoefs)
{
sofa::helper::AdvancedTimer::stepBegin("addEdges with ancestors");
size_t nEdges = m_container->getNumberOfEdges();

/// actually add edges in the topology container
sofa::helper::AdvancedTimer::stepBegin("addEdgesProcess");
addEdgesProcess(edges);

sofa::helper::vector<EdgeID> edgesIndex;
Expand All @@ -699,10 +717,15 @@ void EdgeSetTopologyModifier::addEdges(const sofa::helper::vector< Edge >& edges
}

// add topology event in the stack of topological events
sofa::helper::AdvancedTimer::stepNext ("addEdgesProcess", "addEdgesWarning");
addEdgesWarning(edges.size(), edges, edgesIndex, ancestors, baryCoefs);

// inform other objects that the edges are already added
sofa::helper::AdvancedTimer::stepNext ("addEdgesWarning", "propagateTopologicalChanges");
propagateTopologicalChanges();
sofa::helper::AdvancedTimer::stepEnd("propagateTopologicalChanges");

sofa::helper::AdvancedTimer::stepEnd("addEdges with ancestors");
}

void EdgeSetTopologyModifier::addEdges(const sofa::helper::vector< Edge >& edges,
Expand Down Expand Up @@ -959,6 +982,7 @@ void EdgeSetTopologyModifier::propagateTopologicalEngineChanges()
if (!m_container->isEdgeTopologyDirty()) // edge Data has not been touched
return PointSetTopologyModifier::propagateTopologicalEngineChanges();

sofa::helper::AdvancedTimer::stepBegin("EdgeSetTopologyModifier::propagateTopologicalEngineChanges");
std::list<sofa::core::topology::TopologyEngine *>::iterator it;
for ( it = m_container->m_enginesList.begin(); it!=m_container->m_enginesList.end(); ++it)
{
Expand All @@ -975,6 +999,7 @@ void EdgeSetTopologyModifier::propagateTopologicalEngineChanges()

m_container->cleanEdgeTopologyFromDirty();
PointSetTopologyModifier::propagateTopologicalEngineChanges();
sofa::helper::AdvancedTimer::stepEnd("EdgeSetTopologyModifier::propagateTopologicalEngineChanges");
}


Expand Down
4 changes: 2 additions & 2 deletions SofaKernel/modules/SofaBaseTopology/PointSetTopologyContainer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ void PointSetTopologyContainer::updateDataEngineGraph(sofa::core::objectmodel::B
sofa::core::objectmodel::BaseData* data = dynamic_cast<sofa::core::objectmodel::BaseData*>( (*it) );
if (data)
{
sout << "Warning: Data alone linked: " << data->getName() << sendl;
msg_warning() << "Data alone linked: " << data->getName();
}
}

Expand Down Expand Up @@ -297,7 +297,7 @@ void PointSetTopologyContainer::updateDataEngineGraph(sofa::core::objectmodel::B

// check good loop escape
if (cpt_security >= 1000)
serr << "Error: PointSetTopologyContainer::updateTopologyEngineGraph reach end loop security." << sendl;
msg_error() << "PointSetTopologyContainer::updateTopologyEngineGraph reach end loop security.";


// Reorder engine graph by inverting order and avoiding duplicate engines
Expand Down
28 changes: 27 additions & 1 deletion SofaKernel/modules/SofaBaseTopology/PointSetTopologyModifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#include <sofa/core/topology/TopologyChange.h>
#include <SofaBaseTopology/PointSetTopologyContainer.h>
#include <sofa/core/ObjectFactory.h>

#include <sofa/helper/AdvancedTimer.h>

namespace sofa
{
Expand Down Expand Up @@ -265,19 +265,39 @@ void PointSetTopologyModifier::addPointsWarning(const size_t nPoints,
void PointSetTopologyModifier::addPoints(const size_t nPoints,
const bool addDOF)
{
sofa::helper::AdvancedTimer::stepBegin("addPoints");

sofa::helper::AdvancedTimer::stepBegin("addPointsProcess");
addPointsProcess(nPoints);

sofa::helper::AdvancedTimer::stepNext ("addPointsProcess", "addPointsWarning");
addPointsWarning(nPoints, addDOF);

sofa::helper::AdvancedTimer::stepNext ("addPointsWarning", "propagateTopologicalChanges");
propagateTopologicalChanges();
sofa::helper::AdvancedTimer::stepEnd("propagateTopologicalChanges");

sofa::helper::AdvancedTimer::stepEnd("addPoints");
}

void PointSetTopologyModifier::addPoints(const size_t nPoints,
const sofa::helper::vector< sofa::helper::vector< PointID > >& ancestors,
const sofa::helper::vector< sofa::helper::vector< double> >& coefs,
const bool addDOF)
{
sofa::helper::AdvancedTimer::stepBegin("addPoints with ancestors");

sofa::helper::AdvancedTimer::stepBegin("addPointsProcess");
addPointsProcess(nPoints);

sofa::helper::AdvancedTimer::stepNext ("addPointsProcess", "addPointsWarning");
addPointsWarning(nPoints, ancestors, coefs, addDOF);

sofa::helper::AdvancedTimer::stepNext ("addPointsWarning", "propagateTopologicalChanges");
propagateTopologicalChanges();
sofa::helper::AdvancedTimer::stepEnd("propagateTopologicalChanges");

sofa::helper::AdvancedTimer::stepEnd("addPoints with ancestors");
}

void PointSetTopologyModifier::addPoints(const size_t nPoints,
Expand Down Expand Up @@ -316,6 +336,7 @@ void PointSetTopologyModifier::movePointsProcess (const sofa::helper::vector <Po
void PointSetTopologyModifier::removePointsWarning(sofa::helper::vector<PointID> &indices,
const bool removeDOF)
{
sofa::helper::AdvancedTimer::stepBegin("removePointsWarning");
m_container->setPointTopologyToDirty();

// sort points so that they are removed in a descending order
Expand All @@ -330,17 +351,20 @@ void PointSetTopologyModifier::removePointsWarning(sofa::helper::vector<PointID>
PointsRemoved *e2 = new PointsRemoved(indices);
addStateChange(e2);
}
sofa::helper::AdvancedTimer::stepEnd("removePointsWarning");
}


void PointSetTopologyModifier::removePointsProcess(const sofa::helper::vector<PointID> & indices,
const bool removeDOF)
{
sofa::helper::AdvancedTimer::stepBegin("removePointsProcess");
if(removeDOF)
{
propagateStateChanges();
}
m_container->removePoints(indices.size());
sofa::helper::AdvancedTimer::stepEnd("removePointsProcess");
}


Expand Down Expand Up @@ -419,6 +443,7 @@ void PointSetTopologyModifier::propagateTopologicalEngineChanges()
if (!m_container->isPointTopologyDirty()) // triangle Data has not been touched
return;

sofa::helper::AdvancedTimer::stepBegin("PointSetTopologyModifier::propagateTopologicalEngineChanges");
// get directly the list of engines created at init: case of removing.... for the moment
std::list<sofa::core::topology::TopologyEngine *>::iterator it;

Expand All @@ -433,6 +458,7 @@ void PointSetTopologyModifier::propagateTopologicalEngineChanges()
}

m_container->cleanPointTopologyFromDirty();
sofa::helper::AdvancedTimer::stepBegin("PointSetTopologyModifier::propagateTopologicalEngineChanges");
}

void PointSetTopologyModifier::propagateStateChanges()
Expand Down
2 changes: 1 addition & 1 deletion SofaKernel/modules/SofaBaseTopology/QuadSetTopologyModifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ void QuadSetTopologyModifier::removeQuads(const sofa::helper::vector< QuadID >&
for (size_t i = 0; i < quadIds.size(); i++)
{
if( quadIds[i] >= m_container->getNumberOfQuads())
msg_error() << "Quad: "<< quadIds[i] <<" is out of bound and won't be removed.";
dmsg_warning() << "Quad: "<< quadIds[i] <<" is out of bound and won't be removed.";
else
quadIds_filtered.push_back(quadIds[i]);
}
Expand Down
Loading