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

[wip] Add cache for topic filter to avoid performance burden on discovery #1486

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

Conversation

MichaelOrlov
Copy link
Contributor

  • Relates CPU overhead when discovery is used #1485
    Rationale:
    When rosbag2 discovery is enabled we are periodically doing a lot of checks in the TopicFilter::take_topic(..) which could be avoided by using cache.

Rationale:
When rosbag2 discovery enabled we are continuously doing a lot of checks
in the TopicFilter::take_topic(..) which could be avoided by using cache

Signed-off-by: Michael Orlov <[email protected]>
@MichaelOrlov MichaelOrlov requested a review from a team as a code owner October 24, 2023 06:29
@MichaelOrlov MichaelOrlov requested review from emersonknapp, james-rms and fujitatomoya and removed request for a team October 24, 2023 06:29
@MichaelOrlov
Copy link
Contributor Author

One test is unexpectedly failing. Movin to the WIP until further investigation.

@MichaelOrlov MichaelOrlov marked this pull request as draft October 24, 2023 07:27
@MichaelOrlov MichaelOrlov self-assigned this Oct 24, 2023
filtered_topics.insert(std::make_pair(topic_name, topic_types[0]));
take_topics_cache_[topic_name] = true;
} else {
take_topics_cache_[topic_name] = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

1st discovery, lets say if the topic is bound with unknown type, and the register here with false. after 2nd discovery, even if the topic is rebound to the appropriate type, it cannot be taken since it does not reconsider the topic type every time. how about using already_warned_unknown_types_ to check if we already know that is unknown type or not to deny the topic instead?

@MichaelOrlov MichaelOrlov changed the title Add cache for topic filter to avoid performance burden on discovery [wip] Add cache for topic filter to avoid performance burden on discovery Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants