ros2_control icon indicating copy to clipboard operation
ros2_control copied to clipboard

[CM] Improve memory allocation with buffer variables

Open saikishor opened this issue 1 year ago • 3 comments

This PR tries to address the issue of memory allocation in the RT loop of the CM read, write, and update cycles

saikishor avatar Oct 17 '24 08:10 saikishor

Codecov Report

Attention: Patch coverage is 97.91667% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.34%. Comparing base (254b6e8) to head (74dcce2). Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
.../include/controller_manager/controller_manager.hpp 93.33% 0 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1801      +/-   ##
==========================================
- Coverage   89.34%   89.34%   -0.01%     
==========================================
  Files         139      139              
  Lines       15000    14984      -16     
  Branches     1291     1286       -5     
==========================================
- Hits        13402    13387      -15     
- Misses       1113     1114       +1     
+ Partials      485      483       -2     
Flag Coverage Δ
unittests 89.34% <97.91%> (-0.01%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
controller_manager/src/controller_manager.cpp 75.90% <100.00%> (-0.58%) :arrow_down:
.../include/controller_manager/controller_manager.hpp 96.87% <93.33%> (-3.13%) :arrow_down:

... and 6 files with indirect coverage changes

codecov[bot] avatar Oct 17 '24 09:10 codecov[bot]

Question: would using an RT-safe memory allocator not be a more scalable approach?

gavanderhoorn avatar Oct 17 '24 11:10 gavanderhoorn

This pull request is in conflict. Could you fix it @saikishor?

mergify[bot] avatar Oct 17 '24 15:10 mergify[bot]

This seems fair. Is there a reason that you went for a quite large number to reserve memory for?

Good question, the part of the vectors I can reduce it. For the part of the string, I think we will need it, when you have a large setup and it helps when concatenating strings

saikishor avatar Oct 26 '24 17:10 saikishor

Question: would using an RT-safe memory allocator not be a more scalable approach?

@gavanderhoorn Sure, I wanted to keep it simple and isolated, so it's easier for others. No big reason apart from that. Let me know what you think

saikishor avatar Oct 26 '24 17:10 saikishor

Question: would using an RT-safe memory allocator not be a more scalable approach?

Do you have some pointers for us? (no pun intended! :D )

bmagyar avatar Oct 31 '24 12:10 bmagyar

@gavanderhoorn do you have any pointers to improve this approach?

saikishor avatar Dec 14 '24 17:12 saikishor