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

crba: avoid temporaries with .noalias() #2125

Closed
wants to merge 1 commit into from
Closed

crba: avoid temporaries with .noalias() #2125

wants to merge 1 commit into from

Conversation

alaurenzi
Copy link

@alaurenzi alaurenzi commented Dec 20, 2023

Dear maintainers,
for a few projects we'd like to avoid allocating memory inside function running inside a real time loop.

I have found out that a few temporaries are generated inside the crba function. Adding a .noalias() here and there was enough to keep Eigen from generating temporaries during matrix multiplications.

Let me know what you think!

Arturo

@ManifoldFR
Copy link
Member

ManifoldFR commented Dec 20, 2023

Hi,

Thanks for reaching out!
You found that a malloc happened at the lines you modified ? I thought the involved matrices had fixed Eigen sizes.

@alaurenzi
Copy link
Author

For some of them you may be right.. I belive the problematic line is

data.M.block(jmodel.idx_v(),jmodel.idx_v(),jmodel.nv(),data.nvSubtree[i]).noalias() = ... * ...

where the LHS block has dynamic size.

@ManifoldFR
Copy link
Member

You're right, the LHS is a dynamic-sized Eigen expression.

Have you run the tests/benchmarks with this change and the CHECK_RUNTIME_MALLOC flag (I think you need a Debug build type for that)?

@jcarpent
Copy link
Contributor

@alaurenzi Thanks for reaching us. Could you provide a robot model that triggers allocation?
It would be indeed required to check on our side.

@alaurenzi
Copy link
Author

Hi @jcarpent , please have a look to this minimal self contained example: pinocchio_malloc_test.zip

  • wget https://github.com/stack-of-tasks/pinocchio/files/13740137/pinocchio_malloc_test.zip
  • unzip pinocchio_malloc_test.zip
  • cd pinocchio_malloc_test
  • bash run.bash
  • The program will compile and execute under gdb. It is supposed to crash on malloc, then bt to inspect backtrace

On my machine the interesting line is

#16 0x00005555555acb3b in pinocchio::CrbaBackwardStep<double, 0, pinocchio::JointCollectionDefaultTpl>::algo<pinocchio::JointModelRevoluteUnalignedTpl<double, 0> > (jmodel=..., jdata=..., mo
del=..., data=...)
    at /home/alaurenzi/code/core_ws/install/include/pinocchio/algorithm/crba.hxx:81

which corresponds to

/* M[i,SUBTREE] = S'*F[1:6,SUBTREE] */
      data.M.block(jmodel.idx_v(),jmodel.idx_v(),jmodel.nv(),data.nvSubtree[i]) 
      = jdata.S().transpose()*data.Fcrb[i].middleCols(jmodel.idx_v(),data.nvSubtree[i]);

Adding .noalias() fixes it.

@jcarpent jcarpent self-assigned this Dec 22, 2023
@jcarpent
Copy link
Contributor

See #2126 for the final solving + test

@jcarpent jcarpent closed this Dec 22, 2023
@jcarpent
Copy link
Contributor

@alaurenzi I've provided a clean revision in #2126 with a unit test. Could you check on your side if it does what you expect?

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