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.

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

Attempt to improve the performances of the ctrlLibRT library #143

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

GiulioRomualdi
Copy link
Member

@GiulioRomualdi GiulioRomualdi commented Mar 8, 2022

As a consequence of #141 I tried to improve the performances of the ctrlLibRT. Before explaining what I did I would recall that:
⚠️ we should test this PR on the robot to understand if the performances are improved

First of all, I tried to remove the copies from yarp to idyntree, thanks to this choice this code

iDynTree::toYarp(jointAcc,filters.bufferYarpDofs);
const yarp::sig::Vector & outputJointAcc = filters.jntAccFilter->filt(filters.bufferYarpDofs);
iDynTree::toiDynTree(outputJointAcc,jointAcc);

becomes

iDynTree::toEigen(jointAcc) = filters.jntAccFilter->filt(iDynTree::toEigen(jointAcc));

so I removed two copies every time the joint state is filtered. A similar approach has been used for joint vel, joint acc, accelerometers and gyros, ft.

To achieve that I changed the interface of the ctrLibRT to consider eigen::ref

@traversaro
Copy link
Member

⚠️ we should test this PR on the robot to understand if the performances are improved

Based on my experience, the change in performance also on a custom example that uses those classes as the wbd does may be also a good indication in the increase in the performance.

@GiulioRomualdi
Copy link
Member Author

I benchmarked the two implementations and I didn't notice any notable timing improvements

@traversaro
Copy link
Member

Let's see the bright side. If you have a simple executable it is much easier to profile the code with something like callgrind or flamegraphs

@traversaro traversaro removed their request for review May 30, 2022 08:26
@traversaro
Copy link
Member

Removing myself from the reviewers, feel free to add me back if the PR is being worked on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants