Skip to content

Conversation

@mergify
Copy link

@mergify mergify bot commented Jul 28, 2025

There were two problems in the codebase:

  1. timeline._update_index_cache was called per invalidated topic, which resulted in multiple reads of the whole bagfile (each topic seeks the reader to the bag start and forces it to read throughout the whole bag). There might be some caching on the way so that rqt_bag doesn't read 20x12 GB for a 12 GB file with 20 topics, but it did definitely read much more than 12 GB.
  2. Once I fixed the above to read all topics at once, another bug popped up: the bag entries were cached in memory so that they could be sorted before yielding them from BagTimeline.get_entries(). This was a smaller problem in the per-topic scenario, but a big problem in the all-topic one. But even in the per-topic scenario, rqt_bag needlessly cached gigabytes of entries (e.g. for an image topic) just to sort them.

This PR fixes both issues by utilizing a set of generators, one per bagfile, which produce messages simultaneousely and the earliest of all messages is taken.

I also got the progressbar working much more nicely (however, it only seems to work when loading the first bag).

Tests

The tests were performed on a 12 GB MCAP bag. Before each test, the bag file was evicted from FS cache.

I also wanted to test on a 70 GB bag file, but I didn't have enough hours and RAM to wait until it loads from a USB3 SSD. With this PR, it should be easy to open it (however, I can only verify that tomorrow).

Without this PR

real    1m7.152s
user    0m25.110s
sys     0m32.818s
Obrazovkove.vysilani.2025-05-08.13.49.15.mp4

With this PR

real    0m33.936s
user    0m16.651s
sys     0m7.837s
Obrazovkove.vysilani.2025-05-08.13.47.10.mp4

The problem mentioned in #166 that clicking on the timeline is super slow on large bags, still remains, however. But that's for a future PR.


This is an automatic backport of pull request #178 done by Mergify.

Signed-off-by: Martin Pecka <[email protected]>
(cherry picked from commit c7d3efd)

# Conflicts:
#	rqt_bag/src/rqt_bag/bag_timeline.py
#	rqt_bag/src/rqt_bag/rosbag2.py
#	rqt_bag/src/rqt_bag/timeline_frame.py
@mergify
Copy link
Author

mergify bot commented Jul 28, 2025

Cherry-pick of c7d3efd has failed:

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

You are currently cherry-picking commit c7d3efd.
  (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)

Changes to be committed:
	modified:   rqt_bag/src/rqt_bag/index_cache_thread.py

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   rqt_bag/src/rqt_bag/bag_timeline.py
	both modified:   rqt_bag/src/rqt_bag/rosbag2.py
	both modified:   rqt_bag/src/rqt_bag/timeline_frame.py

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

Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

@peci1 do you mind to take a look to the conflicts?

@ahcorde
Copy link
Contributor

ahcorde commented Aug 18, 2025

friendly ping @peci1

1 similar comment
@ahcorde
Copy link
Contributor

ahcorde commented Sep 16, 2025

friendly ping @peci1

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.

3 participants