-
-
Notifications
You must be signed in to change notification settings - Fork 393
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
Avoid top-level imports in cli.py
#848
Comments
I'd be interested in a benchmark that shows how much time we could save here. |
For this, it is necessary to split up the package into two: one "library" part called `llm` and one "CLI" part called `lmm_cli`. The `click` interface is defined only in `llm_cli` and loads dependencies from `llm` or elsewhere only when necessary. This speeds up running `llm --help` by an order of magnitude: ``` $ time llm --help > /dev/null llm --help > /dev/null 0.80s user 0.10s system 99% cpu 0.906 total # After $ time .venv/bin/llm --help > /dev/null .venv/bin/llm --help > /dev/null 0.07s user 0.01s system 99% cpu 0.078 total ``` See simonw#848.
What a coincidence, I went ahead and implemented it and just pushed my proposed changes. I had to split up the package into two, though. Still need to run the test suite. See the commit message for a basic benchmark. I can create a benchmark for the other subcommands, but I expect the difference to be mostly negligible. |
Yeah, unfortunately this won't work. The plugin mechanism is too intertwined with the CLI. |
Outputing the help takes about one second on my system:
That's because
cli.py
is importing a lot of libraries at the top-level. I propose moving the imports in that file to the functions where they are needed. It may not look asthetic, but it will speed up the call to--help
by many orders of magnitudes, and probably somewhat speed up the warm up time when calling subcommands.Let me know if I can help out with a PR. (It's probably also something that an LLM would be good at.)
Edit: I'm seeing that
__init__.py
is also affected, and my suggestions probably doesn't apply there ...The text was updated successfully, but these errors were encountered: