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

Add module lazy loading in __init__ files #87

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

JAM-Beni
Copy link
Contributor

@JAM-Beni JAM-Beni commented Mar 7, 2024

This commit was made to avoid loading all of the tables when not required. This brings down the memory usage from ~400Mib to ~2MiB.

Flake8 was applied apart from line length which was ignored. For coding references I used PEP 562 and the werkzeug library to ensure I was using proper reference points.

I tested this using the following Python script:

from memory_profiler import profile


@profile
def import_phevaluator():
    from phevaluator import evaluator
    print(evaluator.evaluate_cards('2d', '3d', '4d', '5d', '6d', '7d', '8d'))


@profile
def import_omaha_phevaluator():
    from phevaluator import evaluator_omaha
    print(evaluator_omaha.evaluate_omaha_cards('2d', '3d', '4d', '5d', '6d', '7d', '8d', '9d', 'Td'))


if __name__ == "__main__":
    import_phevaluator()
    import_omaha_phevaluator()

With the output:

7
Filename: test_script.py

Line #    Mem usage    Increment  Occurrences   Line Contents
=============================================================
     4     22.8 MiB     22.8 MiB           1   @profile
     5                                         def import_phevaluator():
     6     25.0 MiB      2.2 MiB           1       from phevaluator import evaluator
     7     25.0 MiB      0.0 MiB           1       print(evaluator.evaluate_cards('2d', '3d', '4d', '5d', '6d', '7d', '8d'))


7
Filename: test_script.py

Line #    Mem usage    Increment  Occurrences   Line Contents
=============================================================
    10     25.1 MiB     25.1 MiB           1   @profile
    11                                         def import_omaha_phevaluator():
    12    423.1 MiB    398.1 MiB           1       from phevaluator import evaluator_omaha
    13    423.1 MiB      0.0 MiB           1       print(evaluator_omaha.evaluate_omaha_cards('2d', '3d', '4d', '5d', '6d', '7d', '8d', '9d', 'Td'))

These tests were surface level, please validate my work further as I am still not fully familiar with this library.

This commit was made to avoid loading all of the tables when not required. This brings down the memory usage from ~400Mib to ~2MiB
@JAM-Beni
Copy link
Contributor Author

JAM-Beni commented Mar 7, 2024

Side note: I left from . import hash as hash_ # FIXME: "hash" collides to built-in function as is due to the FIXME comment

Copy link
Owner

@HenryRLee HenryRLee left a comment

Choose a reason for hiding this comment

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

Thanks you for your work!

The tests have all passed. The code looks good to me.

Let me ask @azriel1rf to see if there are any additional thoughts.

@HenryRLee
Copy link
Owner

Hi @azriel1rf , do you also have some time to look at this PR? Any comments are welcome.

@azriel1rf
Copy link
Collaborator

@JAM-Beni Thank you for your contribution.
@HenryRLee Currently, I'm facing personal constraints that prevent me from engaging in OSS. I value your efforts and look forward to contributing again soon. Apologies for any inconvenience, and I appreciate your understanding.

@HenryRLee
Copy link
Owner

Hey @azriel1rf , no worries! All the best for your new role!

@HenryRLee
Copy link
Owner

The PR is good to merge. Thanks @JAM-Beni for your contribution!

@HenryRLee HenryRLee merged commit 2c24d77 into HenryRLee:master Mar 7, 2024
6 checks passed
@azriel1rf
Copy link
Collaborator

Hey @azriel1rf , no worries! All the best for your new role!

Thanks for your kind message, @henryLee. I'm in the process of obtaining the necessary permissions, and once that's settled, I'm eager to return to contributing to the OSS. It might take a bit of time, but I'm looking forward to rejoining the community.

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.

4 participants