-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Delay evaluation of re.compile to improve startup time. #6690
Comments
Do you have any idea how many of these compiled expressions are used during startup, and where they are defined? It seems like you could figure this out using your proxy class. [Oh, I see now from the table it’s 32, though I still wonder which modules the unused ones are defined in, and what the used ones are needed for during startup.] |
Here you go: Defined
Used
Generated with
|
Thanks! The reason I ask, by the way, is that there's another alternative, which is not to import modules / code that isn't needed for startup. Like, you could imagine pip having a minimal |
I have tried tackling that, but the most difficult part for me is having enough and the right kind of testing to avoid regressions and increased overhead on development and code review. Assuming this approach doesn't have major downsides, I don't think that they are mutually exclusive. It might be worth making a dedicated issue for each possible improvement so we can give each one focus in turn. |
I guess for me personally I'd be more interested in reviewing and seeing progress on the more general dependency / import issue. Like, why does pip's startup need to import As for tackling the dependency issue, I think it's just a matter of deciding on a target for how we want things to be and an incremental plan to get there, and then to do the PR's one at a time. The PR's should mostly just be moving things. And those kinds of changes would I think already be covered by tests since things would fail quickly if e.g. a function was imported from the wrong module. We'd probably want to settle on some kind of light-weight convention or pattern where we import the command or operation code we need inside a function once the code knows what command needs to be executed. There are also many opportunities for doing things towards this goal that can be done even without an overall goal / plan, like moving utility functions out of |
Like, it seems like we shouldn't need to actually import all the command classes here just to get their name and summary in order to parse the command name: pip/src/pip/_internal/commands/__init__.py Lines 27 to 42 in ddfa401
It looks like the command class can simply be imported here once the name is parsed from the arguments and known: pip/src/pip/_internal/__init__.py Line 76 in ddfa401
|
I'd caution against getting sucked into looking at wider issues all at once. This is a nice, focused improvement which seems like it might help. Let's not block progress just for the sake of other potential improvements that may or may not happen. It's not like we can't do both. So +1 from me on investigating this possibility further, and looking at other improvements in their own PRs. |
I filed a separate issue for what I was suggesting: #6692. It's similarly focused and gets more at the root cause IMO as many of the |
Given that cpython automatically caches the last 512 regexes used, is there really a need to pre-compile all those regexes ? |
Related to #4768. |
Interesting. The idea here however, IIUC, is to defer the compiling to later, instead of pre-compiling. |
What's the problem this feature will solve?
Speed up pip startup time, as motivated by #4768.
Describe the solution you'd like
Globally evaluate calls to
re.compile
lazily, only executing when needed.Alternative Solutions
re.compile
at global or class scope to do so as-needed.A quick grep in
pip/_internal
shows 21 calls in our code, out of the 220 total mentioned below, so neither of these are likely to have a large impact.Additional context
Profiling with
cProfile
indicates that a non-negligible part of our startup time is due to calls tore.compile
. Evaluating it lazily shows some improvement. Just runningpip
, we see:re.compile
re.compile
The times will always be machine/hardware-dependent, but it seems worth evaluating as an option just from the reduction in number of function calls.
To reproduce, run
test.sh
Example output
The important parts from the output show the numbers I included in the table above.
Run 1 (no change):
Run 2 (lazy
re.compile
):The text was updated successfully, but these errors were encountered: