Skip to content

lazily imports more modules for performance #91

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

Merged
merged 5 commits into from
Oct 28, 2019
Merged

lazily imports more modules for performance #91

merged 5 commits into from
Oct 28, 2019

Conversation

dmalan
Copy link
Member

@dmalan dmalan commented Oct 23, 2019

Improves performance of using just get_* by lazily loading more of SQL and Flask. E.g., on a recent MBP, reduced performance of

import cs50

i = cs50.get_int()
print(i)

from ~500ms to ~300ms.

Cf. https://wiki.python.org/moin/PythonSpeed/PerformanceTips#Import_Statement_Overhead

We could alternatively break a deliberate abstraction barrier and hardcode awareness of / support for Flask inside of SQL, which might further delay expensive imports.

@@ -1,6 +1,28 @@
import os
import sys


class CustomImporter(object):
Copy link

Choose a reason for hiding this comment

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

Unless I'm missing something, all this does is alias cs50.sql to cs50.SQL. Flask indirectly import cs50.sql anyway, so does this not help in lazy loading.

# Wrap SQLAlchemy
from .sql import SQL
# Lazily load CS50.SQL
sys.meta_path.append(CustomImporter())
Copy link

Choose a reason for hiding this comment

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

At this point cs50.flask has already imported cs50.sql, so there's not much to gain

@Jelleas
Copy link

Jelleas commented Oct 23, 2019

There is indeed a significant overhead. From testing it's Flask adding most of the work, then SQLAlchemy and to a very small degree sqlparse. Lazily importing just those shrinks the import time oncs50 from 300ms to 50ms for me. Lazily importing anything else did not make a difference.

It is cs50.flask that adds the most overhead. Sadly I see no way around it, because overwriting the logger is a task that you cannot do lazily. But, is this a problem that needs a hack? I'd say the Pythonic way is to import a submodule separately if it does not need to be loaded. Such as matplotlib and matplotlib.pyplot, or even check50 and check50.c ;). You would just import cs50 as normal for get_* and such, and import cs50.flask once you are working with flask. Conceptually this makes the most sense to me, because they are actual separate things.

@kzidane
Copy link
Member

kzidane commented Oct 23, 2019

Also as an aside, I think both find_module and load_module are deprecated fwiw.

@Jelleas
Copy link

Jelleas commented Oct 23, 2019

@kzidane kzidane merged commit 49b5e36 into develop Oct 28, 2019
@kzidane kzidane deleted the lazy branch October 28, 2019 14:54
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.

3 participants