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

fix: Address performance by lazy loading imports inside init files #8706

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

Conversation

lohit8846
Copy link

@lohit8846 lohit8846 commented Jan 10, 2025

Related Issues

Proposed Changes:

Solves slow package load times by lazy loading components using importlib. This prevents loading components defined in init file unless they are actually used and thus optimizes the performance avoiding bringing in ML libraries like torch or transformers for lightweight pipelines

How did you test it?

Ran existing tests which import these components already

Notes for the reviewer

Couple of points to consider on the approach chosen

  • Used importlib instead of from syntax because this has greater flexibility to define imports in a dictionary and use a reusable function. from syntax would require defining a separate function for each submodule
  • The from imports are used in the type_checking condition to provide support for IDE type completion. This solves the problem of importlib not helping IDEs with types
  • The dir and getatr functions in init file are defined in a common file lazy_imports.py to avoid redundancy
  • expit in utils.py file is specifically not lazy loaded because the file name and function name being the same is causing python namespace issues when using importlib

There is another MR on a related problem. So you guys should probably consolidate them together with whatever changes you see fit. I may not be able to contribute to this PR further, so please take it from there and prioritize as it is severely affecting performance without it

#8655

Checklist

  • I have read the contributors guidelines and the code of conduct
  • I have updated the related issue with new insights and changes
  • I added unit tests and updated the docstrings
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I documented my code
  • I ran pre-commit hooks and fixed any issue

@lohit8846 lohit8846 requested review from a team as code owners January 10, 2025 22:41
@lohit8846 lohit8846 requested review from dfokina and julian-risch and removed request for a team January 10, 2025 22:41
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Lohit Vankineni seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@lohit8846
Copy link
Author

I prefer not to sign the CLA, so please take over on the fix as you already have an existing PR on this anyways. I suggest using importlib approach instead of from syntax due to reasons mentioned above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:documentation Improvements on the docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Imports inside init causing extremely slow load times
2 participants