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

Update routes pre computation strategy to pre compute less routes #1717

Merged
merged 3 commits into from
Jan 15, 2025

Conversation

OlivierHecart
Copy link
Contributor

@OlivierHecart OlivierHecart commented Jan 15, 2025

Routes pre computation strategy was a bit brutal and was computing too many unnecessary routes leading to high CPU consumption during system setup.
The routes pre computation strategy was updated to only compute

  • data routes for known keys that have no wildcard and have subscribers
  • query routes for known keys that have no wildcard and have queriables

In a 30 talkers/30 listeners zenoh_rmw test the stabilisation time was reduced from 35 seconds to 7 seconds.

The p2p throughput and latency tests have been run on a MacBook Air M2. No observed regressions.

@OlivierHecart OlivierHecart added the enhancement Existing things could work better label Jan 15, 2025
@OlivierHecart OlivierHecart marked this pull request as ready for review January 15, 2025 13:44
routes
}

pub(crate) fn update_data_routes(tables: &Tables, res: &mut Arc<Resource>) {
if res.context.is_some() {
if res.context.is_some() && !res.expr().contains('*') && res.has_subs() {
Copy link
Member

Choose a reason for hiding this comment

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

expr() returns &str, correct?
If expr() would return keyexpr you could use

pub fn is_wild(&self) -> bool {

and rely on the key-expr logic instead of using contains() everywhere. It's more a matter of concern encapsulation than anything else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fully agree. The problem is that:

  • the resource tree cannot store keyexprs for now without a big refactor.
  • doing a try_into keyexpr each time accessing the expr when not needed has a significant cost.

So until we refactor the resource tree, it's far more efficient to do it like this.

@OlivierHecart OlivierHecart merged commit 1e3a496 into main Jan 15, 2025
24 of 26 checks passed
@OlivierHecart OlivierHecart deleted the dev/routes_computation_optim branch January 15, 2025 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Existing things could work better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants