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

Speed up indexed iterator on negative search by topic #1333

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wkalt
Copy link
Contributor

@wkalt wkalt commented Feb 9, 2025

Prior to this commit, the indexed iterator did not rule out chunks based on topic specification, which meant it still opened chunks when no topics would match. With this change the tool does less IO when there is no matching data.

Prior to this commit, the indexed iterator did not rule out chunks based
on topic specification, which meant it still opened chunks when no
topics would match. With this change the tool does less IO when there is
no matching data.
@wkalt wkalt requested a review from james-rms as a code owner February 9, 2025 01:56
@wkalt
Copy link
Contributor Author

wkalt commented Feb 9, 2025

timings:

[~/repos/mcap/go/cli/mcap] (main) $ time mcap cat ~/data/bags/cal_loop.mcap --topics /foobar

real    0m0.969s
user    0m0.911s
sys     0m0.163s
[~/repos/mcap/go/cli/mcap] (main) $ git checkout -
Switched to branch 'indexed-iterator-negative-lookup'
[~/repos/mcap/go/cli/mcap] (indexed-iterator-negative-lookup) $ go install
[~/repos/mcap/go/cli/mcap] (indexed-iterator-negative-lookup) $ time mcap cat ~/data/bags/cal_loop.mcap --topics /foobar

real    0m0.157s
user    0m0.227s
sys     0m0.024s

Copy link
Collaborator

@james-rms james-rms left a comment

Choose a reason for hiding this comment

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

Right now this reader works with files with chunk indexes but no message indexes, which is a case I'd like to continue to support. You could check if the chunk index has any message index offsets before ruling topics out based on their absence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants