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

Improve profile generation. #211

Merged
merged 7 commits into from
Jun 16, 2023
Merged

Improve profile generation. #211

merged 7 commits into from
Jun 16, 2023

Conversation

airblast-dev
Copy link
Contributor

@airblast-dev airblast-dev commented Jun 15, 2023

Benefits: Instead of using globals() and filtering the results, this imports the profiles in function and then uses locals(). This adds four benefits.

  • 1: This can allow refreshing profiles in case of profile only updates or any other situation without restarting the application. This aspect is mainly beneficial for a GUI implementation.
  • 2: This removes the need to use types.ModuleType just for a single conditional. While it isn't the main point, it resulted in roughly a 0.1-0.2 Mb decrease in RAM usage.
  • 3: Removing the conditionals resulted in a roughly 5-10% performance improvement for the function.
  • 4: In the rare case a global value has a property, method or attribute named "profile", the old version would give an error, this way it won't get the value at all let alone give an error.

Tests were measured using timeit comparing the old and new solutions running with numbers set to 1000000. So results are based on 1000000 times of the function being run.

Unit: seconds
Python version: 3.11.3
Old results:

  • 70.173
  • 71.835
  • 70.173

New results:

  • 64.133
  • 64.187
  • 64.165

*Edit: Performance numbers, after a reboot, they became much closer. Newer version is still faster even with the imports.

Copy link
Owner

@flozz flozz left a comment

Choose a reason for hiding this comment

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

Thank you for your PR, there is just some cleaning to do before I merge :)

rivalcfg/devices/__init__.py Outdated Show resolved Hide resolved
rivalcfg/devices/__init__.py Outdated Show resolved Hide resolved
airblast-dev and others added 2 commits June 15, 2023 17:34
Co-authored-by: Fabien LOISON <flozz@users.noreply.github.com>
Co-authored-by: Fabien LOISON <flozz@users.noreply.github.com>
@flozz
Copy link
Owner

flozz commented Jun 15, 2023

It looks good to me. I will try to merge it tomorrow. Thank you :)

@airblast-dev
Copy link
Contributor Author

Happy to help!

@flozz flozz merged commit 997a5cc into flozz:master Jun 16, 2023
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