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

Don't import the world when pip starts up #6692

Closed
cjerdonek opened this issue Jul 9, 2019 · 8 comments
Closed

Don't import the world when pip starts up #6692

cjerdonek opened this issue Jul 9, 2019 · 8 comments
Labels
auto-locked Outdated issues that have been locked by automation type: enhancement Improvements to functionality type: refactor Refactoring code

Comments

@cjerdonek
Copy link
Member

Currently, when pip starts up, it basically imports its entire code base and all vendored libraries and modules it depends on. This issue is for pip only to import what it needs.

It looks like a large part of this can be done pretty simply by changing commands/__init__.py not to import all the command classes:

commands_order = [
InstallCommand,
DownloadCommand,
UninstallCommand,
FreezeCommand,
ListCommand,
ShowCommand,
CheckCommand,
ConfigurationCommand,
SearchCommand,
WheelCommand,
HashCommand,
CompletionCommand,
DebugCommand,
HelpCommand,
] # type: List[Type[Command]]

And then import only the command class it needs after parsing the command name:

command = commands_dict[cmd_name](isolated=("--isolated" in cmd_args))

This will require storing a mapping of command name to command class location and command "summary."

It looks like this would reduce the startup time by ~20% when I tried a quick test.

@cjerdonek
Copy link
Member Author

Later PR's could move the modules needed for pip startup into a startup subpackage (or similar name) to make it easier to see what code pip's startup depends on.

@cjerdonek
Copy link
Member Author

cjerdonek commented Jul 9, 2019

If in addition to the above, the following pkg_resources import is moved from misc.py

from pip._vendor import pkg_resources

to a different location (and get_installed_distributions() is imported in autocompletion.py only when needed), it looks like the improvement increases to > 30%. (This comment on issue #4768 remarked that pip._vendor.pkg_resources is a significant contributor to import time.)

@cjerdonek cjerdonek added type: enhancement Improvements to functionality type: refactor Refactoring code labels Jul 9, 2019
@triage-new-issues triage-new-issues bot removed S: needs triage Issues/PRs that need to be triaged labels Jul 9, 2019
@chrahunt
Copy link
Member

chrahunt commented Jul 9, 2019

Would we want to consider solutions like demandimport in the context of this issue? Just adding

import demandimport
demandimport.ignore("number")
demandimport.enable()

this to the top of the pip script doesn't improve startup time, but I didn't yet investigate why.

@pradyunsg
Copy link
Member

Related to #4768.

@cjerdonek
Copy link
Member Author

Would we want to consider solutions like demandimport in the context of this issue?

Personally, I think my preference would be to start out with simple, more explicit methods before relying on a third-party module like that. We haven't really tried basic things yet. As before, one of my concerns is that it could be masking / papering over issues that we could be solving more directly, and it would also maybe make the code harder to understand since the code wouldn't be behaving like people are accustomed to.

@cjerdonek
Copy link
Member Author

this to the top of the pip script doesn't improve startup time, but I didn't yet investigate why.

I think it's because demandimport requires a particular style of import to get the benefit, which we don't use. (Specifically, we import functions and classes directly instead of importing only the module and accessing the function or class as an attribute of the module.)

cjerdonek added a commit that referenced this issue Jul 28, 2019
Address #6692: Only import a Command class when needed
@pradyunsg
Copy link
Member

@cjerdonek Can we close this?

@cjerdonek
Copy link
Member Author

This was addressed by PR #6694.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Sep 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation type: enhancement Improvements to functionality type: refactor Refactoring code
Projects
None yet
Development

No branches or pull requests

3 participants