Skip to content

Python Plugins: Avoid pop(0) performance impact#1351

Merged
arogge merged 3 commits intomasterfrom
dev/sduehr/master/python-plugins-pop0
Jan 18, 2023
Merged

Python Plugins: Avoid pop(0) performance impact#1351
arogge merged 3 commits intomasterfrom
dev/sduehr/master/python-plugins-pop0

Conversation

@sduehr
Copy link
Member

@sduehr sduehr commented Jan 10, 2023

Python plugins should use collections.deque with popleft() instead of lists with pop(0) as it incurs O(n) memory movement costs which causes performance issues on large lists.

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Check backport line
  • Is the PR title usable as CHANGELOG entry?
  • Separate commit for CHANGELOG.md ("update CHANGELOG.md"). The PR number is correct.
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
  • bareos-check-sources --since-merge does not report any problems

Python plugins should use collections.deque with popleft() instead
of lists with pop(0) as it incurs O(n) memory movement costs which
causes performance issues on large lists.
Copy link
Member

@arogge arogge left a comment

Choose a reason for hiding this comment

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

Code changes look great. I ran the py2plug-fd-ldap test and it still passes. Not sure for the OpenVZ plugin though.

Do you think we should backport these changes to 22?

@arogge arogge merged commit fc1f94e into master Jan 18, 2023
@arogge arogge deleted the dev/sduehr/master/python-plugins-pop0 branch January 18, 2023 12:58
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