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

Speed up pipx list by removing multiprocessing code #624

Merged
merged 4 commits into from
Feb 16, 2021

Conversation

itsayellow
Copy link
Contributor

  • I have added an entry to docs/changelog.md

Summary of changes

Since we are not compute-limited in pipx list, parallel code is not appropriate, and in fact makes running pipx list noticeably slower than purely sequential code.

See #616 for some timing benchmarks

This PR removes the multiprocessing code for pipx list, and makes it always use fully sequential code.

Closes #616

Test plan

Tested by running

Unix / macOS;

> time pipx list

Windows:

> Measure-Command { pipx list }

@uranusjr
Copy link
Member

uranusjr commented Feb 8, 2021

Semi-related, it seems like multiprocessing was used because the implementation used to call pip list. This also left a superfulous python_path argument along the call stack that can be removed in a follow-up refactoring.

@uranusjr
Copy link
Member

uranusjr commented Feb 8, 2021

I’ve filed #626 to remove the unused variable.

@itsayellow
Copy link
Contributor Author

Semi-related, it seems like multiprocessing was used because the implementation used to call pip list. This also left a superfulous python_path argument along the call stack that can be removed in a follow-up refactoring.

Ahh, that makes more sense. Thanks for going back in the history and finding that out.

@itsayellow
Copy link
Contributor Author

itsayellow commented Feb 9, 2021

This seems to be the actual commit when @cs01 made list_packages() parallel: af61325

And yes, it looks like functions run from the Venv.py module involved some subprocess calls. Making them parallel might've helped more then than it does now.

@itsayellow itsayellow merged commit bbe8105 into pypa:master Feb 16, 2021
@itsayellow itsayellow deleted the seq-list branch February 16, 2021 04:35
itsayellow added a commit to itsayellow/pipx that referenced this pull request Feb 16, 2021
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.

De-parallelize pipx list
2 participants