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 cache for _find_maturin_project_above #7

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mrcljx
Copy link

@mrcljx mrcljx commented Nov 15, 2024

I noticed my app's launch to take forever (nearly half a minute) with most of time spent in _find_maturin_project_above which is called repeatedly for every search path whenever a module is loaded.

Adding a cache ensures that search paths aren't analyzed over and over again.

AFAIK using functools.cache (added in 3.9) should be okay as this project requires Python 3.9. If compatibility is a concern, one could use functools.lru_cache(maxsize=None) instead.

@mbway mbway self-requested a review November 15, 2024 20:28
@mbway
Copy link
Collaborator

mbway commented Nov 15, 2024

Thanks for your PR. Can you explain some more about what situation is causing such a long time to be spent inside the import hook?

  • The hook only runs for top level imports (so not import foo.bar)
  • for each top level import, it checks each directory and parent directories in sys.path
  • The _find_maturin_project_above first checks if pyproject.toml exists and only does a slightly more expensive search if it finds that.

So does your app import a huge number of different packages in a virtual environment with many editable-installed packages installed into it?

I would think there would need to be thousands of packages in the environment to take >15 seconds of searching (30 second startup with 'most of the time' inside the import hook).

Can you run with debug logging enabled in the import hook? (modified from the docs)

import logging
logging.basicConfig(format='%(name)s %(asctime)s [%(levelname)s] %(message)s', level=logging.DEBUG)
import maturin_import_hook
maturin_import_hook.reset_logger()
maturin_import_hook.install()

If you are able, can you share the log? Otherwise, can you look at the log and confirm that a significant time is being spent in the import hook.


Regarding the PR itself, I'm open to adding caching functionality but I would rather it be done in a way that can be disabled (opt-out) so it does not interfere with the importlib.reload() support, where things on the filesystem may have changed significantly between reloads.

I can provide a PR with this alternative approach. I hope that's OK with you?

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