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

Add profiling instructions #96

Merged
merged 20 commits into from
Feb 8, 2023
Merged

Conversation

ivanpauno
Copy link
Collaborator

@ivanpauno ivanpauno commented Feb 2, 2023

Related to #35

Summary

Adds instructions of how to profile cpu events of a beluga node and create a flamegraph from that.
The idea of this process is to understand what's beluga use the cpu time on.

I commented a brief summary of the results in #35 (comment).
I think there's no much to look a lot into those results.
It may be a good idea to avoid using an isolated executor for tf, as currently ROS 2 executors are not greatly implemented and they waste more cpu than they should.

I'm adding the instructions, as it's not easy to set it up when using docker and it's a pretty useful tool.

Checklist

  • Read the contributing guidelines.
  • Configured pre-commit and ran colcon test locally.
  • Signed all commits for DCO.
  • Added tests (regression tests for bugs, coverage of new code for features).
  • Updated documentation (as needed).
  • Checked that CI is passing.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno added the enhancement New feature or request label Feb 2, 2023
@ivanpauno ivanpauno self-assigned this Feb 2, 2023
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docker/run.sh Show resolved Hide resolved
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
docs/dev/profiling.md Outdated Show resolved Hide resolved
Copy link
Member

@nahueespinosa nahueespinosa left a comment

Choose a reason for hiding this comment

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

@ivanpauno This is a great contribution! I'm trying it locally.

In the meantime, I left some comments and wanted to ask a few questions:

  • Can we add the dependencies (perf, flamegraph, etc.) to our existing Dockerfile?
  • If not: What's preventing us from providing a new Dockerfile with the dependencies already installed?
  • Is there a reason not to always run perf from within a container?

beluga_example/launch/example_rosbag_launch.py Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling/flamegraph.sh Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling/profile_amcl_with_bagfile.sh Outdated Show resolved Hide resolved
docs/dev/profiling/flamegraph.sh Outdated Show resolved Hide resolved
docs/dev/profiling/flamegraph.sh Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
docs/dev/profiling.md Outdated Show resolved Hide resolved
ivanpauno and others added 11 commits February 7, 2023 15:31
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Co-authored-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Co-authored-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Co-authored-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Co-authored-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
…in script.

* Fix cleanup script behavior.
* Remove "Avoiding sudo" section from troubleshooting.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Copy link
Member

@nahueespinosa nahueespinosa left a comment

Choose a reason for hiding this comment

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

@ivanpauno Left some minor documentation comments.

scripts/profiling/.gitignore Outdated Show resolved Hide resolved
docs/dev/PROFILING.md Outdated Show resolved Hide resolved
docs/dev/PROFILING.md Outdated Show resolved Hide resolved
docs/dev/PROFILING.md Outdated Show resolved Hide resolved
docs/dev/PROFILING.md Outdated Show resolved Hide resolved
docs/dev/PROFILING.md Outdated Show resolved Hide resolved
docs/dev/PROFILING.md Outdated Show resolved Hide resolved
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Copy link
Member

@nahueespinosa nahueespinosa left a comment

Choose a reason for hiding this comment

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

LGTM!

@ivanpauno ivanpauno merged commit d4751d4 into main Feb 8, 2023
@ivanpauno ivanpauno deleted the ivanpauno/profiling-instructions branch February 8, 2023 17:52
@nahueespinosa nahueespinosa added documentation Improvements or additions to documentation infra Related to infrastructure and CI labels Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request infra Related to infrastructure and CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants