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

draft [humble] Add computation of size contribution to info verb (backport #1726) (backport #1872) #1899

Draft
wants to merge 1 commit into
base: humble
Choose a base branch
from

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Jan 21, 2025

Addresses #1601 feature request (Show size contribution of each topic with ros2 bag info). This is a test draft.

Output of ros2 bag info test_bag -v --size-contribution:

Files:             test_bag.mcap
Bag size:          2.6 GiB
Storage id:        mcap
ROS Distro:        unknown
Duration:          39.541s
Start:             Jun 19 2024 23:40:38.345 (1718833238.345)
End:               Jun 19 2024 23:41:17.886 (1718833277.886)
Messages:          2310
Topic information: Topic: /parameter_events | Type: rcl_interfaces/msg/ParameterEvent | Count: 0 | Serialization Format: cdr
                   Topic: /velodyne_points | Type: sensor_msgs/msg/PointCloud2 | Count: 765 | Size Contribution: 557.0 MiB | Serialization Format: cdr
                   Topic: /events/read_split | Type: rosbag2_interfaces/msg/ReadSplitEvent | Count: 0 | Serialization Format: cdr
                   Topic: /image_raw_1 | Type: sensor_msgs/msg/Image | Count: 763 | Size Contribution: 1.0 GiB | Serialization Format: cdr
                   Topic: /image_raw_2 | Type: sensor_msgs/msg/Image | Count: 763 | Size Contribution: 1.0 GiB | Serialization Format: cdr
                   Topic: /events/write_split | Type: rosbag2_interfaces/msg/WriteSplitEvent | Count: 0 | Serialization Format: cdr
                   Topic: /rosout | Type: rcl_interfaces/msg/Log | Count: 19 | Size Contribution: 4.9 KiB | Serialization Format: cdr
Service:           0
Service information:

The total size contribution of each topic is computed by sequentially reading every message in the rosbag. As mentioned in #1601, this approach can be slow.

In a first test A (previous output), the computation of the size contribution took ~520ms for a bag with 2310 messages, 2.6GiB large, 39.5s long, with two sensor_msgs/msg/Image topics with 763 messages each, and one sensor_msgs/msg/PointCloud2 topic with 765 messages.

In a second test B, the size computation took ~78ms for a bag with 14577 messages, 5.2MiB large, 74.4s long, with one sensor_msgs/msg/Imu topic with 14559 messages.

So to me, it seems the computation time is not simply dependent on the number of messages, but on their types and individual sizes too. As a test, I was curious about skipping the message serialization step inside the reader. I tried a brute modification of the mcap_storage interface to directly access the messageView.message.dataSize variable while skipping the message serialization step. However, this only resulted in a 15-30% improvement in computation time.

As a test, removing the actual size computation code and just reading the rosbag in an empty while loop doesn't really change the timing (only real difference was in test B, 78ms->68ms).

In rosbag2_py/_info.cpp, the new function compute_topics_size_contribution can be combined with the existing read_service_info to avoid reading the rosbag twice.

@MichaelOrlov looking forward to your feedback and suggested changes.


This is an automatic backport of pull request #1726 done by [Mergify](https://mergify.com).
This is an automatic backport of pull request #1872 done by [Mergify](https://mergify.com).

…1726) (#1872)

* Add computation of size contribution to info verb (#1726)

* Add optional computation of size contribution to info verb

Signed-off-by: Nicola Loi <[email protected]>

* Update rosbag2_cpp/src/rosbag2_cpp/info.cpp

Co-authored-by: Michael Orlov <[email protected]>
Signed-off-by: Nicola Loi <[email protected]>

* Fixes for review and failed tests

- Also update rosbag2_tests

Signed-off-by: Nicola Loi <[email protected]>

* Support services' size

- Also add new test and update design doc

Signed-off-by: Nicola Loi <[email protected]>

* Fix style divergence

Signed-off-by: Nicola Loi <[email protected]>

* Apply suggestions from code review

Co-authored-by: Michael Orlov <[email protected]>
Signed-off-by: Nicola Loi <[email protected]>

* Update timestamp check for new ros bag info test

Signed-off-by: Nicola Loi <[email protected]>

---------

Signed-off-by: Nicola Loi <[email protected]>
Co-authored-by: Michael Orlov <[email protected]>
(cherry picked from commit 22148a7)

* Make PR ABI/API compatible

- Made "compute_messages_size_contribution(..)"" as non-virtual method
- Make "format_bag_meta_data(..)" APi compatible by moving
"messages_size" argument to the end.

Signed-off-by: Michael Orlov <[email protected]>

---------

Signed-off-by: Michael Orlov <[email protected]>
Co-authored-by: Nicola Loi <[email protected]>
Co-authored-by: Michael Orlov <[email protected]>
(cherry picked from commit 9ec61ea)

# Conflicts:
#	docs/design/rosbag2_record_replay_service.md
#	ros2bag/ros2bag/verb/info.py
#	rosbag2_cpp/include/rosbag2_cpp/info.hpp
#	rosbag2_cpp/include/rosbag2_cpp/storage_options.hpp
#	rosbag2_cpp/src/rosbag2_cpp/info.cpp
#	rosbag2_py/rosbag2_py/_info.pyi
#	rosbag2_py/src/rosbag2_py/_info.cpp
#	rosbag2_py/src/rosbag2_py/format_bag_metadata.cpp
#	rosbag2_py/src/rosbag2_py/format_bag_metadata.hpp
#	rosbag2_py/src/rosbag2_py/format_service_info.cpp
#	rosbag2_tests/test/rosbag2_tests/test_rosbag2_info_end_to_end.cpp
@mergify mergify bot added the conflicts label Jan 21, 2025
@mergify mergify bot requested a review from a team as a code owner January 21, 2025 01:48
Copy link
Author

mergify bot commented Jan 21, 2025

Cherry-pick of 9ec61ea has failed:

On branch mergify/bp/humble/pr-1872
Your branch is up to date with 'origin/humble'.

You are currently cherry-picking commit 9ec61ea.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   docs/design/rosbag2_record_replay_service.md
	both modified:   ros2bag/ros2bag/verb/info.py
	both modified:   rosbag2_cpp/include/rosbag2_cpp/info.hpp
	both modified:   rosbag2_cpp/include/rosbag2_cpp/storage_options.hpp
	both modified:   rosbag2_cpp/src/rosbag2_cpp/info.cpp
	deleted by us:   rosbag2_py/rosbag2_py/_info.pyi
	both modified:   rosbag2_py/src/rosbag2_py/_info.cpp
	both modified:   rosbag2_py/src/rosbag2_py/format_bag_metadata.cpp
	both modified:   rosbag2_py/src/rosbag2_py/format_bag_metadata.hpp
	deleted by us:   rosbag2_py/src/rosbag2_py/format_service_info.cpp
	both modified:   rosbag2_tests/test/rosbag2_tests/test_rosbag2_info_end_to_end.cpp

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify bot requested review from gbiggs and emersonknapp and removed request for a team January 21, 2025 01:48
@MichaelOrlov MichaelOrlov changed the title [jazzy] Add computation of size contribution to info verb (backport #1726) (backport #1872) [humble] Add computation of size contribution to info verb (backport #1726) (backport #1872) Jan 21, 2025
@MichaelOrlov MichaelOrlov changed the title [humble] Add computation of size contribution to info verb (backport #1726) (backport #1872) draft [humble] Add computation of size contribution to info verb (backport #1726) (backport #1872) Jan 21, 2025
@MichaelOrlov MichaelOrlov marked this pull request as draft January 21, 2025 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0 participants