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

feat: add lazy imports for faster startup time #160

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Erotemic
Copy link

For easier review, this is broken out feature I wrote in #150

This uses lazy imports

Issue Being Solved

The problem is that the startup time of the yolo/lazy.py script is very long because yolo/__init__.py explicitly imports a lot of heavyweight dependencies.

Previously running: time python -m yolo.lazy --help took 3 seconds on my machine. After this patch it now takes 0.3 seconds, a 10x improvement.

It works using the lazy-loader module. Instead of explicitly importing everything, it only will import them if they are used. You can also specify the EAGER_IMPORT=1 environment variable to have it import everything on startup as it used to. E.g, the following command will revert to the old 3 second startup time:

time EAGER_IMPORT=True python -m yolo.lazy --help

Additionally I had to move a few imports in lazy.py itself into the main function so it can hit the hydra code before it starts to import all of lightning.

Note on mkinit

Note, that I wrote this using mkinit autogeneration, which is why there is a __submodules__ and __autogen__ definition at the top. Technically this can be removed. But it also lets you regenerate the __init__.py file more easily if anything changes. It can even reconstruct the original explicit definition of the file with:

mkinit ./yolo/__init__.py --nomods --write

On Dependencies

This does add a dependency on lazy-loader, but that could be removed by defining the lazy_loader.attach directly in the __init__.py, but it is a bit ugly, so I tend to prefer using the lightweight package. Running mkinit with --lazy instead of --lazy-loader will generate this variant of the code.

@henrytsui000
Copy link
Member

I'm sorry for not being familiar with lazy_loader. Give me some time to review the code! The import process takes quite a long time and is indeed a bit annoying for me as well XD. I think this will be really helpful!

}


import lazy_loader

Choose a reason for hiding this comment

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

https://peps.python.org/pep-0008/#imports

Imports are always put at the top of the file, just after any module comments and docstrings, and before module globals and constants.

Copy link
Author

Choose a reason for hiding this comment

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

In this case it is done due a quirk of how mkinit autogenerates code, so if you want the lazy-loader dependency path I can fix that. Otherwise, that would be replaced with a definition of the function that does the same thing. Technically none of the constants are needed, they are just convenient to automatically update the __init__.py with mkinit.

However, it's important to note: always adhering to this rule can have significant startup-time consequences. I moved some imports after the call to setup inside of the main function here: https://github.com/MultimediaTechLab/YOLO/pull/160/files#diff-029d4708225fe34b2045fd6a94297034336187c511ca506dccd687f5b6fca3c3R16 and this is important for a faster startup time.

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