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.

Sign up for GitHub

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

Jump to bottom

Add tests for rosbag2_performance_benchmarking pkg #1268

Merged
merged 7 commits into from
Apr 14, 2023
Merged

Add tests for rosbag2_performance_benchmarking pkg #1268

merged 7 commits into from
Apr 14, 2023

Conversation

MichaelOrlov
Copy link
Contributor

@MichaelOrlov MichaelOrlov commented Mar 23, 2023 •
edited
Loading

This is integration tests for rosbag2_performance_benchmarking. By default tests uses tmpfs folder for writing results and bags.

Note. Test will fail without #1267

Currently tests capable to run only for SQlite3 storage backend and disabled on CI.
I will create follow up PRs with adding MCAP support, ability to setup CPU affinity for testing processes and measure CPU load per processes and per each core.
Hopefully we will be able to run these performance tests on nightly CI job to check regression in performance of the rosbag2.

@MichaelOrlov MichaelOrlov force-pushed the morlov/add_tests_for_performance_benchmarking branch 2 times, most recently from 6ea3d4d to b293e71 Compare March 23, 2023 06:58
@MichaelOrlov MichaelOrlov force-pushed the morlov/add_tests_for_performance_benchmarking branch 2 times, most recently from 59e00ad to a89a699 Compare March 29, 2023 08:01
@MichaelOrlov MichaelOrlov force-pushed the morlov/add_tests_for_performance_benchmarking branch 2 times, most recently from 6d4f12f to 8adcb63 Compare March 29, 2023 19:36
@MichaelOrlov
Copy link
Contributor Author

@carlossvg Could you please review this PR?

@MichaelOrlov MichaelOrlov marked this pull request as ready for review March 29, 2023 19:47
@MichaelOrlov MichaelOrlov requested a review from a team as a code owner March 29, 2023 19:47
@MichaelOrlov MichaelOrlov requested review from emersonknapp and james-rms and removed request for a team March 29, 2023 19:47
@MichaelOrlov
Copy link
Contributor Author

@carlossvg kindly ping for review

@MichaelOrlov MichaelOrlov force-pushed the morlov/add_tests_for_performance_benchmarking branch from 02b4648 to ee2d151 Compare April 10, 2023 22:28
MichaelOrlov and others added 7 commits April 12, 2023 16:40
@MichaelOrlov @carlossvg
Add integration test for rosbag2_performance_benchmarking
52e3aff 
Co-authored-by: Carlos San Vicente <carlos.sanvicente@apex.ai>
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Add underscore for topic index name in WriterBenchmark
4c66998 
- Note. It was inconsistency with BenchmarkPublishers

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Enable rosbag2_performance_benchmarking for build and disable for test
17036d4 
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov @christophebedard
Bump actions-ros-ci to v0.3
9997dc7 
Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
@MichaelOrlov
Add missing rosbag2_examples_py to test and build job
39439a4 
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Add TODOs for code refactoring in future
cfddf26 
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Add non default msg_type for camera images and point cloud in automot…
a665e2f 
…ive.yaml

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov MichaelOrlov force-pushed the morlov/add_tests_for_performance_benchmarking branch from ee2d151 to a665e2f Compare April 13, 2023 02:44
carlossvg
carlossvg approved these changes Apr 13, 2023
View reviewed changes
Copy link
Contributor

@carlossvg carlossvg left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@carlossvg
Copy link
Contributor

@MichaelOrlov Just one last thing. Should we document how to setup the benchmark by creating tmpfs or modifying the benchmark path?

@MichaelOrlov
Copy link
Contributor Author

@MichaelOrlov Just one last thing. Should we document how to setup the benchmark by creating tmpfs or modifying the benchmark path?

tmpfs usually existed by default

@MichaelOrlov
Copy link
Contributor Author

@emersonknapp @james-rms Could you please additionally approve or review to be able to merge this PR?
@carlossvg Already reviewed this PR but his approval is not enough to be able to merge changes from this PR.

emersonknapp
emersonknapp approved these changes Apr 13, 2023
View reviewed changes
Copy link
Collaborator

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

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

This looks fine to me! 👍 go ahead

@MichaelOrlov
Copy link
Contributor Author

Gist: https://gist.githubusercontent.com/MichaelOrlov/79969f0d04f121d696cf3dba6674154f/raw/43fdc699b8cd4fa0abb2a1080b3d782969fb2b8b/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_performance_benchmarking
TEST args: --packages-above rosbag2_performance_benchmarking
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11896

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@MichaelOrlov MichaelOrlov merged commit 3455605 into rolling Apr 14, 2023
@delete-merged-branch delete-merged-branch bot deleted the morlov/add_tests_for_performance_benchmarking branch April 14, 2023 06:14
@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-4-20-2023/31087/1

@MichaelOrlov MichaelOrlov mentioned this pull request May 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christophebedard christophebedard christophebedard left review comments

@emersonknapp emersonknapp emersonknapp approved these changes

@carlossvg carlossvg carlossvg approved these changes

@james-rms james-rms Awaiting requested review from james-rms james-rms is a code owner automatically assigned from ros2/tooling-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@MichaelOrlov @carlossvg @ros-discourse @emersonknapp @christophebedard