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

[core] expmap performance improvement #1626

Merged
merged 3 commits into from
Jun 13, 2024
Merged

Conversation

rex-schilasky
Copy link
Contributor

@rex-schilasky rex-schilasky commented Jun 5, 2024

Description

For large maps the update_timestamps function is not performing well.

Changes:

  • Utilization of find Function: The find function is used to search for the key k in the _key_to_value map, rather than using at, which would throw an exception if the key is not found.
  • Splicing Instead of Erasing and Inserting: Instead of erasing and re-inserting elements into the _key_tracker list, the new version uses the splice function to move the element to the end of the list, improving efficiency.
  • Timestamp Update Improvement: The timestamp update logic has been simplified and made more explicit by directly assigning the current time to the timestamp associated with the key k.
  • Simplified Clear Functionality: Just clearing the internal map and list without iterating over their elements.

@rex-schilasky rex-schilasky added cherry-pick-to-support/v5.12 Cherry pick these changes to support/v.5.12 cherry-pick-to-support/v5.13 Cherry pick these changes to support/v5.13 labels Jun 5, 2024
monitoring_get_topics sample slightly improved to allow time updates of internal (large) expmaps in case of subsequent readings
Copy link
Contributor

@hannemn hannemn left a comment

Choose a reason for hiding this comment

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

lgtm as discussed

@rex-schilasky rex-schilasky merged commit ff6b4dd into master Jun 13, 2024
17 of 18 checks passed
@rex-schilasky rex-schilasky deleted the hotfix/expmap-performance branch June 13, 2024 14:24
rex-schilasky added a commit that referenced this pull request Jun 13, 2024
* performance improvement of update_timestamp for large maps (tested with > 25 k entries)
* monitoring_get_topics sample slightly improved to allow time updates of internal (large) expmaps in case of subsequent readings
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions


int main(int argc, char **argv)
{
int run(0), runs(1000);
int run(0), runs(10);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]

Suggested change
int run(0), runs(10);
int run(0);
int runs(10);

rex-schilasky added a commit that referenced this pull request Jun 13, 2024
* performance improvement of update_timestamp for large maps (tested with > 25 k entries)
* monitoring_get_topics sample slightly improved to allow time updates of internal (large) expmaps in case of subsequent readings
rex-schilasky added a commit that referenced this pull request Jun 14, 2024
* performance improvement of update_timestamp for large maps (tested with > 25 k entries)
* monitoring_get_topics sample slightly improved to allow time updates of internal (large) expmaps in case of subsequent readings

Co-authored-by: Rex Schilasky <49162693+rex-schilasky@users.noreply.github.com>
rex-schilasky added a commit that referenced this pull request Jun 14, 2024
* performance improvement of update_timestamp for large maps (tested with > 25 k entries)
* monitoring_get_topics sample slightly improved to allow time updates of internal (large) expmaps in case of subsequent readings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-to-support/v5.12 Cherry pick these changes to support/v.5.12 cherry-pick-to-support/v5.13 Cherry pick these changes to support/v5.13
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants