sofa icon indicating copy to clipboard operation
sofa copied to clipboard

[All] Reduce calls to getValue

Open alxbilger opened this issue 2 years ago • 5 comments

Mainly for:

  • MechanicalObject::position
  • SparseGridRamificationTopology::hexahedra
  • SparseGridRamificationTopology::position

By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).


Reviewers will merge this pull-request only if

  • it builds with SUCCESS for all platforms on the CI.
  • it does not generate new warnings.
  • it does not generate new unit test failures.
  • it does not generate new scene test failures.
  • it does not break API compatibility.
  • it is more than 1 week old (or has fast-merge label).

alxbilger avatar Sep 30 '22 10:09 alxbilger

Note: collision pipeline must be refactored if we want to reduce the ridiculous high number of calls to getValue. Every time there is a check between two primitives, there is a getValue call to MechanicalObject::position, proximity and alarmDistance.

alxbilger avatar Sep 30 '22 10:09 alxbilger

When running caduceus scene for 100 time steps:

Calls to updateIfDirty() Before After
MechanicalObject::position 2269573 515920 -77%
SparseGridRamificationTopology::hexahedra 646323 77430 -88%
SparseGridRamificationTopology::position 644268 24473 -96%
RegularGridTopology::n 19981966 10347192 -48%

alxbilger avatar Sep 30 '22 13:09 alxbilger

[ci-build][with-all-tests]

alxbilger avatar Sep 30 '22 13:09 alxbilger

Note: collision pipeline must be refactored if we want to reduce the ridiculous high number of calls to getValue. Every time there is a check between two primitives, there is a getValue call to MechanicalObject::position, proximity and alarmDistance.

Will be a very funny task to do.

EDIT: CTRL+F on 'getValue()[' shows +1000 hits.

damienmarchal avatar Oct 03 '22 14:10 damienmarchal

This PR makes me think that msg_info() << "message" is in fact something like:

if(notMuted(Base*))
      info::queue << "message"

With notMuted implemented like that:

Base::noMuted() 
{ 
     f_printLog().getValue().       
}

So either we don't use msg_info in loops (which is a good idea by the way, but not easy to track and enforce in our code base) or we reduce the cost of notMuted by using internal caching of the state.

damienmarchal avatar Oct 13 '22 11:10 damienmarchal

Conflict fixed... CI pass, I switch to ready... and let @fredroy merge it ;)

damienmarchal avatar Oct 20 '22 08:10 damienmarchal